From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103801 invoked by alias); 18 Apr 2017 20:18:35 -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 102860 invoked by uid 89); 18 Apr 2017 20:18:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=complicate, metal X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Apr 2017 20:18:33 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EBBDA3D95F; Tue, 18 Apr 2017 20:18:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EBBDA3D95F 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@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com EBBDA3D95F Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5263A60319; Tue, 18 Apr 2017 20:18:33 +0000 (UTC) Subject: Re: [PATCH 3/4] Use std::vector in solib-target lm_info To: Simon Marchi , gdb-patches@sourceware.org References: <20170416141430.2585-1-simon.marchi@polymtl.ca> <20170416141430.2585-4-simon.marchi@polymtl.ca> From: Pedro Alves Message-ID: <614e9e92-fa8b-35f3-c15c-9dde043962e8@redhat.com> Date: Tue, 18 Apr 2017 20:18:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170416141430.2585-4-simon.marchi@polymtl.ca> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-04/txt/msg00528.txt.bz2 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 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