Re: [patch] Add visible flag to breakpoints.

[Phil Muldoon <pmuldoon at redhat dot com>]

Pedro Alves <pedro@codesourcery.com> writes:

> On Friday 08 October 2010 13:50:34, Phil Muldoon wrote:
>> The @var{internal} argument has no effect with watchpoints.
>
> Should it be an error instead (or made to work)?

I can make it an error.  I decided not to do watchpoints.  The
consequences of setting hidden watchpoints from a script that could turn
out to be software watchpoints seemed a little troubling. 

> The non-python bits looked fine.  I'd prefer Tom or someone
> else to look and approve those.  I did notice:
>
>>  #define BPPY_REQUIRE_VALID(Breakpoint)                                 \
>>      do {                                                               \
>> -      if (! bpnum_is_valid ((Breakpoint)->number))                     \
>> +      if (Breakpoint == NULL)                                          \
>>         return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
>>                              (Breakpoint)->number);                     \

> '(Breakpoint)->number' when Breakpoint is NULL is not going to work.

Oops, thanks for catching that.  (Breakpoint)->number is left over from
the old macro.  Will fix.

> Also, missing ()s:
>
>      if ((Breakpoint) == NULL)

Thanks.

> I would also suggest using a VEC for the breakpoints instead
> of a linked list.
>
> (Also, the old code appeared to have been designed for O(1) access to
> the python breakpoint objects given a breakpoing number: I have no
> clue it if that matters but you could preserve that easily with two
> VECs or bppy_breakpoints arrays.)

Yeah the old code used vectors, and I converted it to a linked list to
cope with negative breakpoint numbers.  I did consider keeping two
vectors.  But I wasn't sure if the 0(1) design choice was conscious for
performance or whether it was just done that way.  FWIW we only track
user watchpoints and breakpoints (and now the special breakpoint we have
introduced).  Even in a potentially heavy situation I would expect the
number of user breakpoints to be < 1000?  Is it really necessary?  It's a
trade-off of for keeping two reallocated vectors over the lifetime to
maintaining a linked list.  I'm ambivalent to how we decide to do it.  I
just wanted to explain my rationale.

Cheers,

Phil