Re: [PATCH] Make tracepoints into breakpoints

[Stan Shebs <stan@codesourcery.com>]

Joel Brobecker wrote:
> On Mon, Mar 30, 2009 at 11:23:06AM -0700, Stan Shebs wrote:
>   
>> Joel Brobecker wrote:
>>     
>>>> *************** bpstat_what (bpstat bs)
>>>> *** 3324,3329 ****
>>>> --- 3358,3367 ----
>>>>   	  bs_class = bp_silent;
>>>>   	  retval.call_dummy = 1;
>>>>   	  break;
>>>> + + 	case bp_tracepoint:
>>>> + 	  /* (should never occur, complain about it) */
>>>> + 	  continue;
>>>>   	}
>>>>         current_action = table[(int) bs_class][(int) current_action];
>>>>       }
>>>>     
>>>>         
>>> This hunk looks really strange... Out of place?
>>>       
>
> First of all, I apologize of this comment, since I realize now
> that it was incorrect. I was completely confused because of the tabs
> that made it look like you were putting a continue that would kill
> the code just right after. If I had read the hunk more carefully,
> I would have realized it was a '}', not a '{'!
>   
Heh, it was a little strange-looking!
>   
>> I was waffling on whether to make explicit errors for the impossible  
>> cases. I suppose there should be, one can imagine a buggy target  
>> mistakenly stopping and reporting back at a tracepoint.
>>     
>
> Oh... I did not realize that this could actually happen if we had
> a buggy target. Perhaps an error to help diagnose any problem, or
> a complaint if we want to recover nicely from target errors? Just
> some thoughts - I'm also fine if we prefer to recover silently.
>
>   
After looking at the execution flow some more, I decided to go with 
internal_error - bpstat_what is really a "computational" routine that 
needs to have meaningful bpstats going in - there's not a "makes no 
sense" alternative. So we'll expect higher-level infrun.c code to deal 
with tracepoints being reported back unexpectedly. (That code hasn't 
been written yet, but I'm sure I'll have a need for it soon. :-) )

Stan