RE: [PATCH] target attributes [1/5] core and load from target function

["Abid, Hafiz" <Hafiz_Abid at mentor dot com>]

Hi,

>+  ta = xzalloc (sizeof (struct target_attribute));
>+  ta->name = xstrdup (name);
This name is not freed and will leak.

>+  if (*tap)
>+    (*tap)->next = ta;
>+  else
>+    target_attributes_list = ta;
>+  *tap = ta;
>+
>+  user_data = &ta;

So you are assigning ta to (*tap) and (*tap)->next. Then user_data is also being assigned. It did not look right to me. I wanted to bring it in your attention in case it is a typo.

>+    unsigned int agent_access;
>+    unsigned int gdb_access;
I can understand the access mode for GDB. But what agent_access means and how it will effect a user?

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Hui Zhu
> Sent: Wednesday, November 21, 2012 8:55 AM
> To: Qi, Yao
> Cc: Zhu, Hui; gdb-patches ml
> Subject: Re: [PATCH] target attributes [1/5] core and load from target
> function
> 
> On Sun, Sep 2, 2012 at 6:37 PM, Hui Zhu <teawater@gmail.com> wrote:
> > On Wed, Aug 29, 2012 at 10:43 PM, Yao Qi <yao@codesourcery.com>
> wrote:
> >> On 08/29/2012 04:11 PM, Hui Zhu wrote:
> >>> +static void
> >>> +target_attribute_address_handler (struct gdb_xml_parser *parser,
> >>> +                               const struct gdb_xml_element
> *element,
> >>> +                               void *user_data,
> >>> +                               VEC(gdb_xml_value_s) *attributes) {
> >>> +  int i, len;
> >>> +  struct gdb_xml_value *attrs = VEC_address (gdb_xml_value_s,
> >>> +attributes);
> >>> +  struct target_attribute *ta = *(struct target_attribute
> >>> +**)user_data;
> >>> +  CORE_ADDR start, end;
> >>> +  struct target_attribute_address *ta_addr;
> >>
> >> I happen to see some compilation warnings on my new Fedora 16 box,
> >> while these warnings don't appear on my Ubuntu box.
> >>
> >> ... -Werror -c -o target-memory.o -MT target-memory.o -MMD -MP -MF
> >> .deps/target-memory.Tpo ../../../git/gdb/target-memory.c
> >> ../../../git/gdb/target-attributes.c: In function
> 'target_attribute_address_handler':
> >> ../../../git/gdb/target-attributes.c:487:16: error: 'end' may be
> used
> >> uninitialized in this function [-Werror=uninitialized]
> >> ../../../git/gdb/target-attributes.c:486:18: error: 'start' may be
> >> used uninitialized in this function [-Werror=uninitialized]
> >> ../../../git/gdb/target-attributes.c: In function
> 'target_attribute_attr_handler':
> >> ../../../git/gdb/target-attributes.c:376:12: error: 'type' may be
> >> used uninitialized in this function [-Werror=uninitialized]
> >> ../../../git/gdb/target-attributes.c: In function
> 'target_attribute_type':
> >> ../../../git/gdb/target-attributes.c:632:3: error: 'ret' may be used
> >> uninitialized in this function [-Werror=uninitialized]
> >> cc1: all warnings being treated as errors
> >> make: *** [target-attributes.o] Error 1
> >>
> >> The gcc I am using is 4.6.3
> >> $ gcc --version
> >> gcc (GCC) 4.6.3 20120306 (Red Hat 4.6.3-2)
> >>
> >> We have to get these warnings fixed.
> >>
> >>> +
> >>> +  len = VEC_length (gdb_xml_value_s, attributes);  for (i = 0; i <
> >>> + len; i++)
> >>> +    {
> >>> +      if (strcmp (attrs[i].name, "start") == 0)
> >>> +        start = * (ULONGEST *) attrs[i].value;
> >>> +      else if (strcmp (attrs[i].name, "end") == 0)
> >>> +        end = * (ULONGEST *) attrs[i].value;
> >>> +      else
> >>> +        gdb_xml_error (parser, _("Unknown attribute name '%s'."),
> >>> +                    attrs[i].name);
> >>> +    }
> >>> +
> >>> +  ta_addr = xmalloc (sizeof (struct target_attribute_address));
> >>> + ta_addr->start = start;  ta_addr->end = end;
> >>> +
> >>> +  ta_addr->prev = ta->addresses;
> >>> +  ta->addresses = ta_addr;
> >>> +}
> >>
> >> --
> >> Yao
> >
> > Thanks.  I post a new version.
> >
> > Best,
> > Hui
> 
> Got some error when built with trunk.  Post a new version for this
> error.
> 
> Thanks,
> Hui