Re: [PATCH] Make tracepoints into breakpoints

[Stan Shebs <stan@codesourcery.com>]

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?
> The thing that caught my attention too was the comment that seem
> to imply that this is a temporary comment, almost a FIXME. Not sure
> if this is the case, but the complaint is not there (elsewhere maybe?).
>   
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.
>> + #if 0 /* names of commands not visible here, not clear if worth fixing */
>> + 		if (cmd_cfunc_eq (cmd, while_stepping_pseudocommand))
>> + 		  indent = i2;
>> + 		else if (cmd_cfunc_eq (cmd, end_actions_pseudocommand))
>> + 		  indent = i1;
>> + #endif
>>     
>
> Do we really want to keep this #if 0? I'd personally rather have
> a comment.
>   
Looking at this again, it would be easy enough just to make the two 
symbols visible, I'll do that.
> I'd also like to look at your patch again sometime soon, to figure out
> whether it'd make sense or not for tracepoints to have their own
> breakpoint_ops routines... To be continued :)
>   
I started out defining a breakpoint_ops, but it didn't seem like there 
was any good reason to fill in any of the fields. :-) Four of the fields 
are for the insert/hit/remove activity, which tracepoints don't 
participate in, and two are for printing, but tracepoints print out the 
same as regular breakpoints.

Stan