From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26767 invoked by alias); 17 Apr 2012 12:52:48 -0000 Received: (qmail 26755 invoked by uid 22791); 17 Apr 2012 12:52:47 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,T_RP_MATCHES_RCVD 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, 17 Apr 2012 12:52:14 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q3HCqDxZ006350 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 17 Apr 2012 08:52:13 -0400 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q3HCqAug004687; Tue, 17 Apr 2012 08:52:12 -0400 Message-ID: <4F8D6779.8090906@redhat.com> Date: Tue, 17 Apr 2012 13:00:00 -0000 From: Phil Muldoon MIME-Version: 1.0 To: Siva Chandra CC: gdb-patches@sourceware.org Subject: Re: [RFC - Python scripting] New methods Symtab.global_block and Symtab.static_block (docs included) References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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-04/txt/msg00462.txt.bz2 On 04/17/2012 08:25 AM, Siva Chandra wrote: > The attached patch is a result of discussions here: > http://sourceware.org/ml/gdb-patches/2012-04/msg00226.html > and here: > http://sourceware.org/ml/gdb-patches/2012-04/msg00302.html Thanks, a few comments. We already have gdb.Block.is_static and gdb.Block.is_global. I don't have an objection to this patch per-se, but wouldn't the user acquire the global/static blocks similarly by iterating the blocks in the Python code and doing the above tests? > Code: > 2012-04-17 Siva Chandra Reddy > > Add two new methods global_block and static_block to gdb.Symtab > objects. I don't think GNU Style ChangeLogs allow for a summary line. I don't have an issue with it, but, check with maintainers. Just a nit. > * NEWS (Python scripting): Add entry about the new methods. > * python/py-symtab.c (stpy_global_block): New function which > implements the gdb.Symtab.global_block() method. > (stpy_static_block): New function which implements the > gdb.Symtab.static_block() method. > (symtab_object_methods): Add entries for the two new methods. > > Testsuite: > 2012-04-17 Siva Chandra Reddy > > * py-symbol.exp: Add tests to test the new methods > gdb.Symtab.global_block() and gdb.Symtab.static_block(). > * py-symbol.c: Add new struct to help test > gdb.Symtab.static_block(). gdb.python/ prefix to filenames here. > Docs: > 2012-04-17 Siva Chandra Reddy > > * gdb.texinfo (Symbol Tables In Python): Add documentation about > the new methods global_block and static_block on gdb.Symtab > objects. > > Thanks, > Siva Chandra > +@defun Symtab.global_block () > +Return the global block of the underlying symbol table. Note that, > +though highly unlikely, the returned @code{gdb.Block} objects are not > +guaranteed to be identical across different @value{GDBN} releases. > +@end defun In what sense not identical? I know Doug brought this up, and it is fair point. But if I was a Python scripting author it would make me nervous and distrustful of this API. It might also break our API promise, depending on how one looks at it. What might change? I think mitigation here could be achieved through a more thorough explanation? Doug would have to expand on what he thinks might change. As it is, this patch makes me a little nervous of the whole block API in general. > +@defun Symtab.static_block () > +Return the static block of the underlying symbol table. Note that, > +though highly unlikely, the returned @code{gdb.Block} objects are not > +guaranteed to be identical across different @value{GDBN} releases. > +@end defun Ditto above. > +static PyObject * > +stpy_global_block (PyObject *self, PyObject *args) > +{ > + struct symtab *symtab = NULL; > > + struct block *block = NULL; > + > + STPY_REQUIRE_VALID (self, symtab); > + > + block = symtab->blockvector->block[GLOBAL_BLOCK]; > + return block_to_block_object (block, symtab->objfile); I don't think so, but can block ever be NULL here? Looking at the code, I'm not even sure if it would matter as set_block just stores a reference to the passed block. However, blpy_get_superblock has a NULL block check, so it might be worthwhile checking it out. > + block = symtab->blockvector->block[STATIC_BLOCK]; > + return block_to_block_object (block, symtab->objfile); Same as above. > Index: testsuite/gdb.python/py-symbol.c > =================================================================== > RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-symbol.c,v > > retrieving revision 1.5 > diff -u -p -r1.5 py-symbol.c > Index: testsuite/gdb.python/py-symtab.exp > =================================================================== > RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-symtab.exp,v > retrieving revision 1.8 > diff -u -p -r1.8 py-symtab.exp I don't want you to fix this as it is unrelated to your patch, but .exp files should have their own test inferior. Something I noted so I remember to fix it! Cheers, Phil