From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15132 invoked by alias); 10 Jan 2012 15:09:38 -0000 Received: (qmail 15017 invoked by uid 22791); 10 Jan 2012 15:09:36 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW 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, 10 Jan 2012 15:09:23 +0000 Received: by vcge1 with SMTP id e1so4113456vcg.0 for ; Tue, 10 Jan 2012 07:09:22 -0800 (PST) Received: by 10.220.153.134 with SMTP id k6mr11802278vcw.23.1326208130217; Tue, 10 Jan 2012 07:08:50 -0800 (PST) MIME-Version: 1.0 Received: by 10.220.3.130 with HTTP; Tue, 10 Jan 2012 07:08:29 -0800 (PST) In-Reply-To: References: <83r4z8eqoa.fsf@gnu.org> From: Kevin Pouget Date: Tue, 10 Jan 2012 16:03:00 -0000 Message-ID: Subject: Re: [PATCH] Add bp_location to Python interface To: Eli Zaretskii Cc: gdb-patches@sourceware.org, pmuldoon@redhat.com 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: 2012-01/txt/msg00293.txt.bz2 On Tue, Jan 10, 2012 at 3:50 PM, Kevin Pouget wrot= e: > On Mon, Jan 9, 2012 at 6:23 PM, Eli Zaretskii wrote: >>> From: Kevin Pouget >>> Date: Mon, 9 Jan 2012 12:46:30 +0100 >>> Cc: pmuldoon@redhat.com >>> >>> ping >> >> Sorry for missing it the first time. > > no problem, thanks for your feedbacks, I replied inline > >>> --- a/gdb/NEWS >>> +++ b/gdb/NEWS >>> @@ -9,6 +9,12 @@ >>> =A0* The binary "gdbtui" can no longer be built or installed. >>> =A0 =A0Use "gdb -tui" instead. >>> >>> +* Python scripting >>> + >>> + =A0** A new method "gdb.Breakpoint.locations" has been added, as well= as >>> + =A0 =A0 the class gdb.BpLocation to provide further details about bre= akpoint >>> + =A0 =A0 locations. >>> + >> >> This is OK. >> >>> +@defun gdb.locations () >>> +Return a tuple containing a sequence of @code{gdb.BpLocation} objects >>> +(see below) associated with this breakpoint. =A0A breakpoint with no l= ocation >>> +is a pending breakpoint (@xref{Set Breaks, , pending breakpoints}). >> >> @pxref, not @xref, as this cross-reference is inside parentheses. >> >>> +A breakpoint location is represented with a @code{gdb.BpLocation} obje= ct, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0^^^^ >> "by" > > fixed > >>> +which offers the following attributes (all read only) and methods. >>> +Please note that breakpoint locations are very transient entities in >>> +@value{GDBN}, so one should avoid keeping references to them. >> >> I'd use "volatile" instead of "transient". > > fixed > >> Also, perhaps a sentence >> or two about _why_ the locations are volatile would help. =A0E.g., if I >> knew what actions invalidate locations, I could avoid them and leave >> the locations valid for longer. > > That's a point Phil also noted so I've try to explain it a bit > further, although my knowledge about the internal mechanisms involved > is quite limited. A maintainer will have to confirm these words. I > also mentioned explicitly that it's the *objects* related to the > locations which are volatile, because the location (ie, address) > itself actually doesn't change: > >> Please note that breakpoint location objects are very volatile entities = in >> @value{GDBN}, so one should avoid keeping references to them. =A0They ca= n be >> destructed and recreated on any breakpoint creation and deletion, shared- >> library event, inferior function call, @dots{} > > > >>> +owns this location. =A0This attribute is not writable. =A0From an impl= ementation >>> +point of view, there is a @code{1 ... n} relation between a breakpoint= and >> >> "1 ... n" here means one to many? =A0If so, I suggest to say that >> literally. >> >> In any case, "@code{1 ... n}" is not a good idea, because of the >> whitespace and because we use @dots{} instead of literal periods. =A0If >> "one to many" is not what you meant, I can suggest how to mark up >> whatever needs to be written here. > > No, "one-to-many" is what I meant, I've changed it (spelled with the > dashes, right?) > >>> +This attribute holds a reference to the @code{gdb.Inferior} inferior o= bject >> >> I'd drop the second instance of "inferior", it looks redundant. > > dropped > >>> +@defun BpLocation.is_valid () >>> +Returns @code{True} if the @code{gdb.BpLocation} object is valid, >>> +@code{False} if not. =A0A @code{gdb.BpLocation} object may be invalida= ted by >>> +GDB at any moment for internal reasons. =A0All other @code{gdb.BpLocat= ion} >>> +methods and attributes will throw an exception if the object is invali= d. >> >> @value{GDBN} instead of "GDB". >> >> In any case, the last 2 sentences sound scary: I could interpret them >> as meaning I cannot trust the locations at all. =A0If that is indeed so, >> what use are they? > > that's already discussed above, but I don't want you to be scared, so > let me explain what I meant: > it's not "at any moment", but rather "after any call to GDB's Python > interface". We may want to say that it's only breakpoint or > execution-related calls, but _I_ can't ensure that this is true, and > it 'might' change in the future: > >> A @code{gdb.BpLocation} object may be invalidated during >> any call to @{GDB}'s API for internal reasons (for instance, but not lim= ited to, >> breakpoint or execution-related mechanisms). Sorry there is a missing 'value' in @{GDB} here and in the patch, it will be fixed for the next submission. And a typo, > @defvar BpLocation.owner > ... even if two breakpoints are set on at same address. I removed the redundant 'on'. Kevin > (I'm not sure what's the right Python term here for 'mechanisms', > reading/writing an attribute may trigger internal functions, so 'call' > or 'function' seem not to suit very well) > > > Cordially, > > Kevin