This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Ping in http://www.sourceware.org/ml/gdb-patches/2013-02/msg00500.html

Thanks,
Hui

On Wed, Feb 20, 2013 at 12:10 AM, Hui Zhu <teawater@gmail.com> wrote:
> On Mon, Feb 18, 2013 at 7:45 PM, Abid, Hafiz <hafiz_abid@mentor.com> wrote:
>> On 18/02/13 10:01:10, Hui Zhu wrote:
>>>
>>> On Fri, Nov 30, 2012 at 2:03 PM, Abid, Hafiz <Hafiz_Abid@mentor.com>
>>> wrote:
>>> > Hi,
>>> >
>>> >>+  ta = xzalloc (sizeof (struct target_attribute));
>>> >>+  ta->name = xstrdup (name);
>>> > This name is not freed and will leak.
>>>
>>> Added xfree in clear_target_attributes.
>>>
>>> >
>>> >>+  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.
>>>
>>> user_data will always point to the last one of target_attributes_list.
>>>   Could you tell me which part is wrong?
>>
>> This code may be right. It just looked a bit confusing. Also tap and
>> user_data point to samething. One of the following 2 assignment seems
>> redundant.
>> *tap = ta;
>> user_data = &ta;
>>
>>
>
> Thanks for help me on this issue.  I make clear about this part of
> code and I change this part of code to:
>   if (*tap)
>     {
>       /* Add new TA to the tail of TARGET_ATTRIBUTES_LIST.  */
>       (*tap)->next = ta;
>     }
>   else
>     {
>       /* The fist time call target_attribute_attr_handler.
>          Just get USER_DATA from gdb_xml_parse_quick.  */
>       target_attributes_list = ta;
>     }
>   /* Let USER_DATA save the address of new TA.  */
>   *tap = ta;
>
> Thanks,
> Hui
>
> 2013-02-19  Hui Zhu  <hui_zhu@mentor.com>
>
>         * Makefile.in (SFILES): Add target-attributes.c.
>         (HFILES_NO_SRCDIR): Add target-attributes.h.
>         (COMMON_OBS): Add target-attributes.o.
>         * breakpoint.h (breakpoint): Add target_only_cond_check.
>         * remote.c (target-attributes.h): New include.
>         (PACKET_qXfer_target_attributes_read): New enum.
>         (remote_start_remote): Add handler for target attributes.
>         (remote_protocol_features): Add "qXfer:target-attributes:read".
>         (remote_xfer_partial): Add handler for TARGET_OBJECT_ATTRIBUTES.
>         (_initialize_remote): Add command
>         "set remote target-attributes-packet".
>         (target-attributes.c, target-attributes.h): New files.
>         * tracepoint.c (target-attributes.h): New include.
>         (find_trace_state_variable_number,
>         trace_variable_number_check_1,
>         trace_variable_number_check): New functions.
>         (trace_variable_command): Call trace_variable_number_check_1.
>         (merge_uploaded_trace_state_variables): Call
>         trace_variable_number_check.
>         tracepoint.h (trace_variable_number_check): New extern.
>
>>>
>>> >
>>> >>+    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?
>>>
>>> There is the introduce of agent
>>> http://sourceware.org/gdb/current/onlinedocs/gdb/Agent-Expressions.html
>>>
>>> Post a new version according to your review.
>>>
>>> Thanks,
>>> Hui
>>>
>>> 2013-02-18  Hui Zhu  <hui_zhu@mentor.com>
>>>
>>>         * Makefile.in (SFILES): Add target-attributes.c.
>>>         (HFILES_NO_SRCDIR): Add target-attributes.h.
>>>         (COMMON_OBS): Add target-attributes.o.
>>>         * breakpoint.h (breakpoint): Add target_only_cond_check.
>>>         * remote.c (target-attributes.h): New include.
>>>         (PACKET_qXfer_target_attributes_read): New enum.
>>>         (remote_start_remote): Add handler for target attributes.
>>>         (remote_protocol_features): Add "qXfer:target-attributes:read".
>>>         (remote_xfer_partial): Add handler for TARGET_OBJECT_ATTRIBUTES.
>>>         (_initialize_remote): Add command
>>>         "set remote target-attributes-packet".
>>>         (target-attributes.c, target-attributes.h): New files.
>>>         * tracepoint.c (target-attributes.h): New include.
>>>         (find_trace_state_variable_number,
>>>         trace_variable_number_check_1,
>>>         trace_variable_number_check): New functions.
>>>         (trace_variable_command): Call trace_variable_number_check_1.
>>>         (merge_uploaded_trace_state_variables): Call
>>>         trace_variable_number_check.
>>>         tracepoint.h (trace_variable_number_check): New extern.
>>>
>>> >
>>> >> -----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
>>>
>>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]