From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92731 invoked by alias); 4 Jul 2018 04:40:23 -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 92592 invoked by uid 89); 4 Jul 2018 04:40:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham 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, 04 Jul 2018 04:40:20 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 1BEEA1E48F; Wed, 4 Jul 2018 00:40:18 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=simark.ca; s=mail; t=1530679218; bh=AARs3t5Daykk8eGYK9rwAisjeayUofRFp2bO3jEgz/I=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=HYLi244xQ8OjHuDWQf9NjrffGT18ExajCAGKJqyFX/h6bhr+TUuhHTwVfAhjTACRQ tdnsPEfSmF+GkHlH2IpWbLMWxytWR9UAQag5f2iTXtcnOvUC02lUchP6N1AX6Pl6+2 1N5i0tQpT6v8okyOC8IRqyXOCagFI5uw1sbu79Ig= Subject: Re: [RFA] Remove VEC from breakpoint To: Andrew Burgess , Tom Tromey Cc: gdb-patches@sourceware.org References: <20180605192316.5423-1-tom@tromey.com> <20180703143301.GA2675@embecosm.com> From: Simon Marchi Message-ID: <65452ba0-5ff9-bdc6-ce43-164777e9407c@simark.ca> Date: Wed, 04 Jul 2018 04:40:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180703143301.GA2675@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-07/txt/msg00068.txt.bz2 On 2018-07-03 10:33 AM, Andrew Burgess wrote: > * Tom Tromey [2018-06-05 13:23:16 -0600]: > >> This removes a use of VEC from breakpoint.h, also removing the >> now-unnecessary breakpoint_p typedef. >> >> This patch fixes a latent memory leak in >> find_matching_tracepoint_location, which neglected to free the vector >> returned by all_tracepoints. >> >> Tested by the buildbot. >> >> gdb/ChangeLog >> 2018-06-05 Tom Tromey >> >> * tracepoint.c (process_tracepoint_on_disconnect, start_tracing) >> (stop_tracing, tstatus_command) >> (find_matching_tracepoint_location, merge_uploaded_tracepoints) >> (print_one_static_tracepoint_marker): Update. >> * breakpoint.c (static_tracepoints_here, all_tracepoints): Return >> std::vector. >> * breakpoint.h (breakpoint_p): Remove typedef. Don't declare >> VEC. >> (all_tracepoints, static_tracepoints_here): Return std::vector. > > FWIW this looks fine to me, except for one nit below... [snip] >> @@ -3094,9 +3063,8 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps) >> { >> struct uploaded_tp *utp; >> /* A set of tracepoints which are modified. */ >> - VEC(breakpoint_p) *modified_tp = NULL; >> + std::vector modified_tp; >> int ix; >> - struct breakpoint *b; > > The 'ix' is no longer used and could be deleted. > > While I was double checking this I also spotted another 'ix' in > merge_uploaded_trace_state_variables that's not used... :) Agreed, the patch LGTM in any case. Compiling tracepoint.c with -Wunused shows a few more: /home/simark/src/binutils-gdb/gdb/tracepoint.c:1546:32: error: unused variable 'tsv' [-Werror,-Wunused-variable] struct trace_state_variable *tsv; ^ /home/simark/src/binutils-gdb/gdb/tracepoint.c:2755:28: error: unused variable 'default_collect_action' [-Werror,-Wunused-variable] struct command_line *default_collect_action; ^ /home/simark/src/binutils-gdb/gdb/tracepoint.c:3067:7: error: unused variable 'ix' [-Werror,-Wunused-variable] int ix; ^ /home/simark/src/binutils-gdb/gdb/tracepoint.c:3183:7: error: unused variable 'ix' [-Werror,-Wunused-variable] int ix; ^ /home/simark/src/binutils-gdb/gdb/tracepoint.c:3686:26: error: unused variable 'b' [-Werror,-Wunused-variable] struct breakpoint *b; ^ It looks like an obvious patch to clean these up is in order, so it doesn't really matter if this patch or an eventual cleanup patch removes these ix variables. Thanks! Simon