From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106147 invoked by alias); 19 Apr 2017 04:18:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 105297 invoked by uid 89); 19 Apr 2017 04:18:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Apr 2017 04:18:03 +0000 Received: by simark.ca (Postfix, from userid 33) id 956AF1E165; Wed, 19 Apr 2017 00:18:03 -0400 (EDT) To: Pedro Alves Subject: Re: [PATCH 3/4] Use std::vector in solib-target lm_info X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 19 Apr 2017 04:18:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <614e9e92-fa8b-35f3-c15c-9dde043962e8@redhat.com> References: <20170416141430.2585-1-simon.marchi@polymtl.ca> <20170416141430.2585-4-simon.marchi@polymtl.ca> <614e9e92-fa8b-35f3-c15c-9dde043962e8@redhat.com> Message-ID: <14d3077c75de88c8233be22e1d3a5f94@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.4 X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00547.txt.bz2 On 2017-04-18 16:18, Pedro Alves wrote: > 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. Ok, thanks for the info. >> /* Handle the start of a 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? Err you're right. I'll add default initializers in the class, e.g.: char *name {}; >> + 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. It makes sense. >> } >> 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. Ok. I'll send a v2 in not too long. Thanks, Simon