This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] mi/10586
- From: Tom Tromey <tromey at redhat dot com>
- To: Keith Seitz <keiths at redhat dot com>
- Cc: "gdb-patches\ at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Tue, 13 Dec 2011 12:39:52 -0700
- Subject: Re: [RFA] mi/10586
- References: <4EBD93D9.2020006@redhat.com> <m3mxbyso6i.fsf@fleche.redhat.com> <4EC157F6.1030503@redhat.com> <m3mxbyr2ju.fsf@fleche.redhat.com> <4EC16BD8.90309@redhat.com> <m34ny6qwi9.fsf@fleche.redhat.com> <4EC29CF7.40204@redhat.com> <4ED95103.8030204@redhat.com>
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> +/* The names of varobjs representing anonymous structs or unions.
Keith> + Note that is_name_anonymous_type makes assumptions about these two
Keith> + constants. */
I didn't see 'is_name_anonymous_type' anywhere in the tree or in the patch.
I guess it needs an update.
Keith> +static int
Keith> +is_path_expr_parent (struct varobj *var)
[...]
Keith> + type = get_value_type (var);
Extra space.
Keith> + return strncmp (child->name, ANONYMOUS_STRUCT_NAME, 11) == 0;
I think this is wrong since the macros have _() in their expansion.
I think you have to use strlen.
Keith> + field_name = TYPE_FIELD_NAME (type, index);
Keith> + if (*field_name == '\0')
Can field_name == NULL?
It is not clear to me. There is some code in gdb that checks this, but
I don't know whether that is defensive programming or checking a
condition that is truly possible.
Otherwise this looks good to me.
Tom