This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/4] Use std::vector in solib-target lm_info
- From: Pedro Alves <palves at redhat dot com>
- To: Simon Marchi <simon dot marchi at polymtl dot ca>, gdb-patches at sourceware dot org
- Date: Tue, 18 Apr 2017 21:18:32 +0100
- Subject: Re: [PATCH 3/4] Use std::vector in solib-target lm_info
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com EBBDA3D95F
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EBBDA3D95F
- References: <20170416141430.2585-1-simon.marchi@polymtl.ca> <20170416141430.2585-4-simon.marchi@polymtl.ca>
On 04/16/2017 03:14 PM, Simon Marchi wrote:
> Replace the two VEC fields with std::vector.
>
> I found only one place where these lm_infos were allocated, but two
> where they are freed. It looks like solib_target_free_so missed freeing
> section_bases before.
>
> More c++ification is obviously possible, but my goal right now is to get
> rid of VEC (CORE_ADDR).
>
> I wasn't really able to test this, since the list of remote targets that use
> this method of fetching solibs is quite limited (windows, dicos and
> arm-symbian, from what I can see).
Other random "bare metal" / RTOSs that GDB doesn't need to know about
use solib-target too. It's the default solib implementation exactly
to allow for GDB to remain agnostic about them. That doesn't help
you with testing, it's just a FYI.
> /* Handle the start of a <library> element. */
> @@ -119,7 +124,7 @@ library_list_start_library (struct gdb_xml_parser *parser,
> void *user_data, VEC(gdb_xml_value_s) *attributes)
> {
> VEC(lm_info_p) **list = (VEC(lm_info_p) **) user_data;
> - struct lm_info *item = XCNEW (struct lm_info);
Note this was an XCNEW, which means that it zeroed all
memory. Did you check whether all fields are initialized
after the patch?
> + struct lm_info *item = new lm_info;
> const char *name
> = (const char *) xml_find_attribute (attributes, "name")->value;
>
> @@ -135,10 +140,8 @@ library_list_end_library (struct gdb_xml_parser *parser,
> VEC(lm_info_p) **list = (VEC(lm_info_p) **) user_data;
> struct lm_info *lm_info = VEC_last (lm_info_p, *list);
>
> - if (lm_info->segment_bases == NULL
> - && lm_info->section_bases == NULL)
> - gdb_xml_error (parser,
> - _("No segment or section bases defined"));
> + if (lm_info->segment_bases.empty () && lm_info->section_bases.empty ())
> + gdb_xml_error (parser, _("No segment or section bases defined"));
> }
>
>
> @@ -175,9 +178,7 @@ solib_target_free_library_list (void *p)
> for (ix = 0; VEC_iterate (lm_info_p, *result, ix, info); ix++)
> {
> xfree (info->name);
> - VEC_free (CORE_ADDR, info->segment_bases);
> - VEC_free (CORE_ADDR, info->section_bases);
> - xfree (info);
> + delete info;
As a general principle, I'd rather move all destruction bits to
the destructor at the same time when we C++fy a struct.
It doesn't really complicate the patch, while not doing it
makes it easier to leave these bits missed behind in a
random follow up patch that adds a dtor.
I.e., above, I'd prefer to move the xfree to a dtor in
the same patch.
> }
> VEC_free (lm_info_p, *result);
> *result = NULL;
> @@ -326,8 +327,7 @@ solib_target_free_so (struct so_list *so)
> {
> gdb_assert (so->lm_info->name == NULL);
> xfree (so->lm_info->offsets);
> - VEC_free (CORE_ADDR, so->lm_info->segment_bases);
> - xfree (so->lm_info);
> + delete so->lm_info;
Ditto.
Thanks,
Pedro Alves