Re: [PATCH] Make tracepoints into breakpoints

[Stan Shebs <stan at codesourcery dot 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