Re: [patch] Add visible flag to breakpoints.

[Phil Muldoon <pmuldoon@redhat.com>]

Pedro Alves <pedro@codesourcery.com> writes:

> On Thursday 30 September 2010 17:18:15, Phil Muldoon wrote:
>> The original patch I wrote did use negative numbers for bp_breakpoint
>> type (in fact that patch is a commit in the archer branch:
>> archer-pmuldoon-python-breakpoints).  But normal bp_breakpoints with a
>> negative number are still displayed with 'info breakpoints'. Currently
>> the visibility of breakpoints is not decided on their number but their
>> type.
>> breakpoint_1 tests for these in user_settable_breakpoint.  
>
> I think that's just cruft and can be replaced by a b->number < 0 check?

Ah, there is the crux.  I presumed (from the code) that there was a
scenario where negative numbered breakpoints were valid to be printed.
But looks like not.  We can use negative numbers if the < 0 printing
check is ok and do away with the visible flag.


> I'm not sure what large effort you're thinking this entails.  
> You've carried the "visible" flag as argument all the way down to
> create_breakpoint_sal already.  So instead of:
>
> @@ -6961,6 +6973,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
>           b->enable_state = enabled ? bp_enabled : bp_disabled;
>           b->disposition = disposition;
>           b->pspace = sals.sals[0].pspace;
> +         b->visible = visible;
>
> You change this:
>
> 	  b = set_raw_breakpoint (gdbarch, sal, type);
> 	  set_breakpoint_count (breakpoint_count + 1);
> 	  b->number = breakpoint_count;
>
> to:
>
>  b = set_raw_breakpoint (gdbarch, sal, type);
>  if (visible /* or some other name, user? !internal? */)
>    {
>       set_breakpoint_count (breakpoint_count + 1);
>       b->number = breakpoint_count;
>    }
>   else
>    {
>      b->number = internal_breakpoint_number--;
>    }
>
> and you're golden.  What am I missing?

I have a patch in my archer branch that does just this.  The original
patch I wrote did this ;)

There is some work to be done on the Python bookkeeping side as breakpoint
references are stored via breakpoint number (in a vector), so the negative
numbers would be painful there.  Luckily in the aforementioned patch I
already changed this bookkeeping over to a single linked-list.

Looks like my first intentions were best then.  I'll reconstitute the
patch from previous commits and resubmit (happily this is super easy
with git)

Cheers

Phil