From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8982 invoked by alias); 18 Oct 2011 02:45:49 -0000 Received: (qmail 8963 invoked by uid 22791); 18 Oct 2011 02:45:46 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 18 Oct 2011 02:45:30 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RFzgH-0006FV-Jx from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Mon, 17 Oct 2011 19:45:29 -0700 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Tue, 18 Oct 2011 11:45:28 +0900 Message-ID: <4E9CE812.7030607@codesourcery.com> Date: Tue, 18 Oct 2011 02:46:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20110923 Thunderbird/7.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union References: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/msg00492.txt.bz2 On 10/08/2011 01:35 PM, Li Yu wrote: > gdb.Type.fields() missed handling for anonymous members. > > This patch fix it, below are details: > > Assume that we have a c source as below: > > ///////////////////////////// > struct A > { > int a; > union { > int b0; > int b1; > union { > int bb0; > int bb1; > union { > int bbb0; > int bbb1; > }; > }; > }; > int c; > union { > union { > int dd0; > int dd1; > }; > int d2; > int d3; > }; > }; > > int main() > { > struct A a; > } > //////////////////////////// > > And have a python gdb script as below: > > ########################## > v = gdb.parse_and_eval("a") > t = v.type > fields = t.fields() > for f in fields: > print "[%s]" % f.name, v[f.name] > ########################## > > Without this patch, above script will print: > > [a] -7616 > [] {{dd0 = 0, dd1 = 0}, d2 = 0, d3 = 0} > [c] 0 > [] {{dd0 = 0, dd1 = 0}, d2 = 0, d3 = 0} > > With this patch, above script will print rightly: > > [a] -7616 > [b0] 32767 > [b1] 32767 > [bb0] 32767 > [bb1] 32767 > [bbb0] 32767 > [bbb1] 32767 > [c] 0 > [dd0] 0 > [dd1] 0 > [d2] 0 > [d3] 0 > I am thinking that we may have to convert your test above into a dejagnu-manner testcase. testsuite/gdb.python/py-type.exp looks like a good place to add. > Thanks for Paul and Tom's feedback of this patch. > > This version uses recursion implementation, as we discussed, this > patch is passed by "make check" without regression :) > I don't know gdb/python too much, so some comments on code style. Leaving Paul/Tom and other gdb/python experts to give you comments on code logic. > Signed-off-by: Li Yu > > gdb/python/: > 2011-10-8 Li Yu > > * py-type.c: Add process for anonymous members of struct and union It is a sentence, so please add a "." at the end of it. Some of functions and struct are changed, so they should be mentioned in ChangeLog, something like * py-type.c (typy_make_iter): XXX (typy_iterator_iternext): XXX. See more in > > py-type.c | 41 ++++++++++++++++++++++++++++++++++------- > 1 file changed, 34 insertions(+), 7 deletions(-) > > > > diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c > index c7fd25b..b3632ea 100644 > --- a/gdb/python/py-type.c > +++ b/gdb/python/py-type.c > @@ -56,8 +56,11 @@ typedef struct pyty_field_object > static PyTypeObject field_object_type; > > /* A type iterator object. */ > -typedef struct { > +typedef struct iterator_object > +{ > PyObject_HEAD > + /* The iterators for support fields of anonymous field */ > + struct iterator_object *child; Usually, changes on struct should be reflected on ChangeLog entry as well. You can add something like this into your changelog entry: (struct iterator_object): New field `child'. > /* The current field index. */ > int field; > /* What to return. */ > @@ -1200,6 +1203,7 @@ typy_make_iter (PyObject *self, enum gdbpy_iter_kind kind) > if (typy_iter_obj == NULL) > return NULL; > > + typy_iter_obj->child = NULL; > typy_iter_obj->field = 0; > typy_iter_obj->kind = kind; > Py_INCREF (self); > @@ -1257,11 +1261,27 @@ static PyObject * > typy_iterator_iternext (PyObject *self) > { > typy_iterator_object *iter_obj = (typy_iterator_object *) self; > - struct type *type = iter_obj->source->type; > - int i; > - PyObject *result; > - > - if (iter_obj->field < TYPE_NFIELDS (type)) > + typy_iterator_object *child_iter_obj; > + struct type *type; > + PyObject *result, *child_pytype; > + const char *name; > + > + if (iter_obj->child) > + { > + result = typy_iterator_iternext((PyObject*)iter_obj->child); ^ ^ spaces are needed. > + if (result) > + return result; > + Py_CLEAR(iter_obj->child); > + } > + > + type = iter_obj->source->type; > + > +restart: > + if (iter_obj->field >= TYPE_NFIELDS (type)) > + return NULL; > + > + name = TYPE_FIELD_NAME (type, iter_obj->field); > + if (!name || name[0]) /* array element or regular named member */ > { > result = make_fielditem (type, iter_obj->field, iter_obj->kind); > if (result != NULL) > @@ -1269,7 +1289,14 @@ typy_iterator_iternext (PyObject *self) > return result; > } > > - return NULL; > + type = TYPE_FIELD_TYPE(type, iter_obj->field++); > + child_pytype = type_to_type_object(type); > + if (!child_pytype) > + return NULL; > + child_iter_obj = (typy_iterator_object*)typy_make_iter(child_pytype, iter_obj->kind); This line is too long, exceeding the hard-limit of 80 char per line. > + iter_obj->child = child_iter_obj; > + iter_obj = child_iter_obj; > + goto restart; > } > > static void -- Yao (齐尧)