On Tue, Oct 25, 2011 at 1:36 PM, Kevin Pouget wrote: > On Tue, Oct 25, 2011 at 1:05 PM, Phil Muldoon wrote: >> >> Kevin Pouget writes: >> >> > On Mon, Oct 24, 2011 at 6:06 PM, Phil Muldoon wrote: >> >> Kevin Pouget 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 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. > > yes, I totally agree with this opinion, and that's why I changed the > code arguing, > "what's for Python stays in Python" ! > >> 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've got time for my patches, provided that they're not "forgotten", > and that the > discussions are constructive, I'll be happy to fix and refactor accordingly :) > > I've got no idea about the cost of Python functions in general, let > see if that's > what Tom had in mind. > > > Cordially, > > Kevin Hello, I've updated this patch against the last GIT tree and fixed some typos in the code, if you want to give it another look Tom... I followed your suggestion (delete BP once the frame in gone) as well as Phil's (move is_finish_py from 'struct breakpoint' to the Py object, looks with encapsulation) Thanks, Kevin