Re: [RFC] Python Finish Breakpoints

[Phil Muldoon <pmuldoon at redhat dot com>]

Kevin Pouget <kevin.pouget@gmail.com> writes:

> On Mon, Oct 24, 2011 at 6:06 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>> Kevin Pouget <kevin.pouget@gmail.com> writes:
>>
>> I have some comments regarding the Python bits.
>
> Thanks for that, I replied inline

Thanks.

>>> @@ -5700,6 +5700,7 @@ init_raw_breakpoint_without_location (struct breakpoint *b,
>>> Â Âb->frame_id = null_frame_id;
>>> Â Âb->condition_not_parsed = 0;
>>> Â Âb->py_bp_object = NULL;
>>> + Âb->is_py_finish_bp = 0;


>Phil>> Is there any reason why this needs to be in the breakpoint struct? 

>Kevin> just to put it back in context (in was back in May ...), here is the
>Kevin> rational behind the flag:


> On Thu, May 19, 2011 at 6:20 PM, Tom Tromey <tromey@redhat.com> wrote:
>> gdbpy_is_stopped_at_finish_bp is not safe to call here -- it assumes the
>> GIL has been acquired, which it has not.  I would rather it not be changed
>> to acquire the GIL, however.  I think one of two other approaches would
>> be preferable.
>>
>> One way you could handle this is to add a new constant to enum bptype.
>> This is likely to be pretty invasive.
>>
>> Another way would be to add a flag to the struct breakpoint itself.
>>
>> Yet another way would be a new breakpoint_ops method.

>Kevin> I've refactored the code according to your comment anyway, it make
>Kevin> sense, so now there are two version:

First let me apologies for not picking up on these previous comments.

My own personal opinion is to abstract the details to the GDB Python
code, instead of adding another flag to 'struct breakpoint'. That was
the original ethos of adding a pointer inside the breakpoint struct to
the Python breakpoint-object - so we can have access to the whole of the
breakpoint object without breaking out pieces of it here and there.

That being said, I don't want to delay this patch any further (and I'm
not sure why you cannot acquire the GIL in the accessor function?  There
is a performance hit involved in acquiring the GIL, maybe that). Tom
gave three options that make sense, so whatever works for you and Tom
will be fine.  Thanks for taking the time to refactor it.  Tom, what do
you think?

>> I think these comments should wrap? They wrap for me in emacs.
>
> I'm not sure about the exact meaning of 'wrap' here, but I assume it's
> about the new line between computable and NULL; I've reformatted it.

Yeah I meant, split the comments up.

Cheers,

Phil