Re: [RFA]: breakpoint.c patch (prelude to pending breakpoint support)

[Daniel Jacobowitz <drow at mvista dot com>]

On Thu, Dec 11, 2003 at 08:01:58AM +0200, Eli Zaretskii wrote:
> The second comment is about this hunk of changes:
> 
> > @@ -2574,9 +2581,7 @@ bpstat_stop_status (CORE_ADDR *pc, int n
> >  
> >    ALL_BREAKPOINTS_SAFE (b, temp)
> >    {
> > -    if (b->enable_state == bp_disabled
> > -	|| b->enable_state == bp_shlib_disabled
> > -	|| b->enable_state == bp_call_disabled)
> > +    if (!breakpoint_enabled (b) && b->enable_state != bp_permanent)
> >        continue;
> 
> Bother.  Is it really wise to replace an explicit check of equality to
> several bp_* constants with "!= bp_permanent"?  Are we sure that any
> non-bp_permanent breakpoint should pass this test, even if in the
> future additional bp_* constants will be introduced that aren't there
> now?

I asked Jeff to do that, so I'll step in here :)

Right now, there are five possible enable states:
  enabled
  disabled
  permanent
  call_disabled
  shlib_disabled

I'm not convinced that permanent should even be on the list.  It's a
real oddball; and there's no reason that GDB couldn't virtually
"disable" a permanent breakpoint (step over it automatically when
hitting it; give it an always-false condition, in effect).

So the others boil down to a group of enabled breakpoint states and a
group of disabled breakpoint states.  The body of the
bpstat_stop_status loop only cares about enabled breakpoints, and for
its purposes permanent breakpoints are enabled (because they might be
the reason that we stopped).  So I think the new test is logically
correct.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer