From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32608 invoked by alias); 25 Oct 2011 11:05:47 -0000 Received: (qmail 32599 invoked by uid 22791); 25 Oct 2011 11:05:46 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 25 Oct 2011 11:05:26 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p9PB5OEp004268 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 25 Oct 2011 07:05:24 -0400 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p9PB5Nhj025200; Tue, 25 Oct 2011 07:05:23 -0400 From: Phil Muldoon To: Kevin Pouget Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [RFC] Python Finish Breakpoints References: Reply-to: pmuldoon@redhat.com X-URL: http://www.redhat.com Date: Tue, 25 Oct 2011 11:37:00 -0000 In-Reply-To: (Kevin Pouget's message of "Tue, 25 Oct 2011 10:30:53 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 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/msg00652.txt.bz2 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 brea= kpoint *b, >>> =C2=A0 =C2=A0b->frame_id =3D null_frame_id; >>> =C2=A0 =C2=A0b->condition_not_parsed =3D 0; >>> =C2=A0 =C2=A0b->py_bp_object =3D NULL; >>> + =C2=A0b->is_py_finish_bp =3D 0; >Phil>> Is there any reason why this needs to be in the breakpoint struct?= =20 >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 chang= ed >> 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