From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6262 invoked by alias); 24 Sep 2019 02:19:59 -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 6252 invoked by uid 89); 24 Sep 2019 02:19:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 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; Tue, 24 Sep 2019 02:19:57 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id ED5511E512; Mon, 23 Sep 2019 22:19:55 -0400 (EDT) Subject: Re: [PATCH 2/2] gdb: Change a VEC to std::vector in btrace.{c,h} To: Andrew Burgess , gdb-patches References: <2754d81c341f0cbb6240e309f93d3b9efb38c3e3.1569276387.git.andrew.burgess@embecosm.com> From: Simon Marchi Message-ID: <7ad80184-704d-c939-a0e1-a85bf7ac7ea8@simark.ca> Date: Tue, 24 Sep 2019 02:19:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 MIME-Version: 1.0 In-Reply-To: <2754d81c341f0cbb6240e309f93d3b9efb38c3e3.1569276387.git.andrew.burgess@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-09/txt/msg00459.txt.bz2 On 2019-09-23 6:09 p.m., Andrew Burgess wrote: > Replace a VEC with a std::vector in btrace.h, and update btrace.c to > match. This code is currently untested, so I've been quite > conservative with the changes - where previously we used VEC_safe_push > which would allocate the vector if it isn't already allocated - I now > check and allocate the std::vector using 'new' before pushing to the > vector. > > As the new vector is inside a union I've currently used a pointer to > vector, which makes the code slightly uglier than it might otherwise > be, but again, due to lack of testing I'm reluctant to start > refactoring the code in a big way. Hi Andrew, Looks fine to me, just a few nits: > diff --git a/gdb/btrace.c b/gdb/btrace.c > index 5013b568a45..59e2a49bcd8 100644 > --- a/gdb/btrace.c > +++ b/gdb/btrace.c > @@ -1828,7 +1828,7 @@ btrace_maint_clear (struct btrace_thread_info *btinfo) > > #if defined (HAVE_LIBIPT) > case BTRACE_FORMAT_PT: > - xfree (btinfo->maint.variant.pt.packets); > + delete (btinfo->maint.variant.pt.packets); You can remove the parenthesis. > > btinfo->maint.variant.pt.packets = NULL; > btinfo->maint.variant.pt.packet_history.begin = 0; > @@ -2987,8 +2987,10 @@ btrace_maint_decode_pt (struct btrace_maint_info *maint, > if (maint_btrace_pt_skip_pad == 0 || packet.packet.type != ppt_pad) > { > packet.errcode = pt_errcode (errcode); > - VEC_safe_push (btrace_pt_packet_s, maint->variant.pt.packets, > - &packet); > + if (maint->variant.pt.packets == NULL) > + maint->variant.pt.packets > + = new std::vector ; This will fit on a single line once you remove the `_s` :). > --- a/gdb/btrace.h > +++ b/gdb/btrace.h > @@ -29,7 +29,6 @@ > #include "gdbsupport/btrace-common.h" > #include "target/waitstatus.h" /* For enum target_stop_reason. */ > #include "gdbsupport/enum-flags.h" > -#include "gdbsupport/vec.h" > > #if defined (HAVE_LIBIPT) > # include > @@ -267,7 +266,6 @@ struct btrace_pt_packet > > /* Define functions operating on a vector of packets. */ > typedef struct btrace_pt_packet btrace_pt_packet_s; > -DEF_VEC_O (btrace_pt_packet_s); Like in the previous patch, you can drop this typedef. It was only useful to define the VEC. Simon