From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22756 invoked by alias); 25 Oct 2011 11:37:08 -0000 Received: (qmail 22636 invoked by uid 22791); 25 Oct 2011 11:37:07 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: sourceware.org Received: from mail-vx0-f169.google.com (HELO mail-vx0-f169.google.com) (209.85.220.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 25 Oct 2011 11:36:48 +0000 Received: by vcbfk1 with SMTP id fk1so410401vcb.0 for ; Tue, 25 Oct 2011 04:36:47 -0700 (PDT) Received: by 10.220.108.84 with SMTP id e20mr738vcp.3.1319542607042; Tue, 25 Oct 2011 04:36:47 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.180.75 with HTTP; Tue, 25 Oct 2011 04:36:26 -0700 (PDT) In-Reply-To: References: From: Kevin Pouget Date: Tue, 25 Oct 2011 12:22:00 -0000 Message-ID: Subject: Re: [RFC] Python Finish Breakpoints To: pmuldoon@redhat.com Cc: Tom Tromey , gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-10/txt/msg00653.txt.bz2 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 wro= te: > >> 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 br= eakpoint *b, > >>> =A0 =A0b->frame_id =3D null_frame_id; > >>> =A0 =A0b->condition_not_parsed =3D 0; > >>> =A0 =A0b->py_bp_object =3D NULL; > >>> + =A0b->is_py_finish_bp =3D 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 t= he > >> GIL has been acquired, which it has not. =A0I would rather it not be c= hanged > >> to acquire the GIL, however. =A0I think one of two other approaches wo= uld > >> 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? =A0There > 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. =A0Thanks for taking the time to refactor it. =A0Tom, 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 > >> 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