Re: [PATCH v4] gdb/python: add missing handling for anonymous members of struct and union

[Yao Qi <yao at codesourcery dot com>]

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 (éå)