* [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union
@ 2011-10-08 5:35 Li Yu
2011-10-18 1:48 ` Li Yu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Li Yu @ 2011-10-08 5:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Paul Koning, Tom Tromey
[-- Attachment #1: Type: text/plain, Size: 1739 bytes --]
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
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 :)
Signed-off-by: Li Yu <raise.sail@gmail.com>
gdb/python/:
2011-10-8 Li Yu <raise.sail@gmail.com>
* py-type.c: Add process for anonymous members of struct and union
py-type.c | 41 ++++++++++++++++++++++++++++++++++-------
1 file changed, 34 insertions(+), 7 deletions(-)
[-- Attachment #2: gdb.python.patch --]
[-- Type: text/x-patch, Size: 2119 bytes --]
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;
/* 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);
+ 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);
+ iter_obj->child = child_iter_obj;
+ iter_obj = child_iter_obj;
+ goto restart;
}
static void
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union
2011-10-08 5:35 [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union Li Yu
@ 2011-10-18 1:48 ` Li Yu
2011-10-18 2:46 ` Yao Qi
2011-10-18 14:03 ` Phil Muldoon
2 siblings, 0 replies; 6+ messages in thread
From: Li Yu @ 2011-10-18 1:48 UTC (permalink / raw)
To: gdb-patches; +Cc: Paul Koning, Tom Tromey
Is this ready for merge into upsteam ?
thanks
Yu
于 2011å¹´10月08æ—¥ 13:35, Li Yu 写é“:
> 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
>
> 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 :)
>
> Signed-off-by: Li Yu <raise.sail@gmail.com>
>
> gdb/python/:
> 2011-10-8 Li Yu <raise.sail@gmail.com>
>
> * py-type.c: Add process for anonymous members of struct and union
>
> py-type.c | 41 ++++++++++++++++++++++++++++++++++-------
> 1 file changed, 34 insertions(+), 7 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union
2011-10-08 5:35 [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union Li Yu
2011-10-18 1:48 ` Li Yu
@ 2011-10-18 2:46 ` Yao Qi
2011-10-18 14:03 ` Phil Muldoon
2 siblings, 0 replies; 6+ messages in thread
From: Yao Qi @ 2011-10-18 2:46 UTC (permalink / raw)
To: gdb-patches
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 <raise.sail@gmail.com>
>
> gdb/python/:
> 2011-10-8 Li Yu <raise.sail@gmail.com>
>
> * 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
<http://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs>
>
> 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 (é½å°§)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union
2011-10-08 5:35 [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union Li Yu
2011-10-18 1:48 ` Li Yu
2011-10-18 2:46 ` Yao Qi
@ 2011-10-18 14:03 ` Phil Muldoon
[not found] ` <09787EF419216C41A903FD14EE5506DD030CB90864@AUSX7MCPC103.AMER.DELL.COM>
2011-10-19 8:28 ` Li Yu
2 siblings, 2 replies; 6+ messages in thread
From: Phil Muldoon @ 2011-10-18 14:03 UTC (permalink / raw)
To: Li Yu; +Cc: gdb-patches, Paul Koning, Tom Tromey
Li Yu <raise.sail@gmail.com> writes:
> gdb.Type.fields() missed handling for anonymous members.
>
> This patch fix it, below are details:
Sorry I missed this patch. I have some questions.
Given this functionality, do you have any use-cases in mind for it? Do
we really want to include anonymous members in field () output? I ask
because I cannot decide if the additional anonymous field information
constitutes an API break. If so we may have to reconstitute this
functionality from fields() so that it takes a keyword to turn this
behavior on and off.
Also, this patch requires a GDB testcase, and probably a documentation
update. Also, a ChangeLog.
> /* A type iterator object. */
> -typedef struct {
> +typedef struct iterator_object
> +{
> PyObject_HEAD
> + /* The iterators for support fields of anonymous field */
> + struct iterator_object *child;
Comments have to be complete sentences, including punctuation. So add a
period at the end of that comment.
> + if (iter_obj->child)
> + {
> + result = typy_iterator_iternext((PyObject*)iter_obj->child);
Spacing around the ( and ) is incorrect.
> + 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 */
> {
Comments need to be complete and punctuated.
> 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);
Bracket spacing.
> + if (!child_pytype)
> + return NULL;
> + child_iter_obj =
> (typy_iterator_object*)typy_make_iter(child_pytype, iter_obj->kind);
Bracket spacing.
> + iter_obj->child = child_iter_obj;
> + iter_obj = child_iter_obj;
> + goto restart;
> }
>
> static void
Cheers,
Phil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union
[not found] ` <09787EF419216C41A903FD14EE5506DD030CB90864@AUSX7MCPC103.AMER.DELL.COM>
@ 2011-10-18 19:31 ` Paul Koning
0 siblings, 0 replies; 6+ messages in thread
From: Paul Koning @ 2011-10-18 19:31 UTC (permalink / raw)
To: gdb-patches, Phil Muldoon, Li Yu
On Oct 18, 2011, at 10:21 AM, <Paul_Koning@Dell.com> wrote:
>
>
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Phil Muldoon
> Sent: Tuesday, October 18, 2011 9:58 AM
> To: Li Yu
> Cc: gdb-patches@sourceware.org; Paul Koning; Tom Tromey
> Subject: Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union
>
> Li Yu <raise.sail@gmail.com> writes:
>
>> gdb.Type.fields() missed handling for anonymous members.
>>
>> This patch fix it, below are details:
>
> Sorry I missed this patch. I have some questions.
>
> Given this functionality, do you have any use-cases in mind for it? Do we really want to include anonymous members in field () output? I ask because I cannot decide if the additional anonymous field information constitutes an API break. If so we may have to reconstitute this functionality from fields() so that it takes a keyword to turn this behavior on and off.
I raised a related question two weeks ago: http://sourceware.org/ml/gdb-patches/2011-10/msg00118.html .
It would seem logical for things to be consistent. That's quite a lot more work than this one patch, but it can certainly be handled as a collection of independent changes.
Another possible approach is to leave gdb.Type.fields() alone, but supply a little bit of Python code for the gdb python library that implements a recursive iterator. It's quite easy to do that:
def deepitems (t):
for k, v in t.iteritems ():
if k:
yield k, v
else:
for i in deepitems (v.type):
yield i
paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union
2011-10-18 14:03 ` Phil Muldoon
[not found] ` <09787EF419216C41A903FD14EE5506DD030CB90864@AUSX7MCPC103.AMER.DELL.COM>
@ 2011-10-19 8:28 ` Li Yu
1 sibling, 0 replies; 6+ messages in thread
From: Li Yu @ 2011-10-19 8:28 UTC (permalink / raw)
To: pmuldoon; +Cc: gdb-patches, Paul Koning, Tom Tromey
于 2011å¹´10月18æ—¥ 21:57, Phil Muldoon 写é“:
> Li Yu <raise.sail@gmail.com> writes:
>
>> gdb.Type.fields() missed handling for anonymous members.
>>
>> This patch fix it, below are details:
>
> Sorry I missed this patch. I have some questions.
>
> Given this functionality, do you have any use-cases in mind for it? Do
> we really want to include anonymous members in field () output? I ask
> because I cannot decide if the additional anonymous field information
> constitutes an API break. If so we may have to reconstitute this
> functionality from fields() so that it takes a keyword to turn this
> behavior on and off.
>
> Also, this patch requires a GDB testcase, and probably a documentation
> update. Also, a ChangeLog.
>
The patch for test suite is here: http://www.cygwin.com/ml/gdb-patches/2011-10/msg00219.html
You may find the reason of this patch at http://sourceware.org/ml/gdb-patches/2011-09/msg00568.html
En, we may need a doc update, it seem that I indeed read tens of pages gnu coding standard carefully, it will spend some time.
>> /* A type iterator object. */
>> -typedef struct {
>> +typedef struct iterator_object
>> +{
>> PyObject_HEAD
>> + /* The iterators for support fields of anonymous field */
>> + struct iterator_object *child;
>
> Comments have to be complete sentences, including punctuation. So add a
> period at the end of that comment.
>
>> + if (iter_obj->child)
>> + {
>> + result = typy_iterator_iternext((PyObject*)iter_obj->child);
>
> Spacing around the ( and ) is incorrect.
>
>> + 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 */
>> {
>
> Comments need to be complete and punctuated.
>
>> 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);
>
> Bracket spacing.
>
>> + if (!child_pytype)
>> + return NULL;
>> + child_iter_obj =
>> (typy_iterator_object*)typy_make_iter(child_pytype, iter_obj->kind);
>
> Bracket spacing.
>
>> + iter_obj->child = child_iter_obj;
>> + iter_obj = child_iter_obj;
>> + goto restart;
>> }
>>
>> static void
>
> Cheers,
>
> Phil
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-19 2:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-08 5:35 [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union Li Yu
2011-10-18 1:48 ` Li Yu
2011-10-18 2:46 ` Yao Qi
2011-10-18 14:03 ` Phil Muldoon
[not found] ` <09787EF419216C41A903FD14EE5506DD030CB90864@AUSX7MCPC103.AMER.DELL.COM>
2011-10-18 19:31 ` Paul Koning
2011-10-19 8:28 ` Li Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox