From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id ZKUaDtWmhGVGAycAWB0awg (envelope-from ) for ; Thu, 21 Dec 2023 15:57:57 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1703192277; bh=OVy530cH2lhe1L6vk2ovnRqQmooPUqEA5SXfvC6qZdQ=; h=Date:Subject:To:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=QQWomECHNewvd2OerxlpqardYzXfrCa+8XmUhyByz+JYpXogNlqm21QLA9UNHpISp +7vy7c9vnu+yLmVn8FiiPpzOtg50Nmhs/4wXRfsM5G8YOr+r3PBpvBYWOrtm7yC/Wm 4mn0F5oW2hLcwmT80eH7Hc5XQ8zLdXHq1IRcrMJ0= Received: by simark.ca (Postfix, from userid 112) id 2B1BC1E0C3; Thu, 21 Dec 2023 15:57:57 -0500 (EST) Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=MPRT5/eG; dkim-atps=neutral Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 160C11E0AC for ; Thu, 21 Dec 2023 15:57:55 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 50713385842C for ; Thu, 21 Dec 2023 20:57:54 +0000 (GMT) Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 3C42B385842C for ; Thu, 21 Dec 2023 20:57:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3C42B385842C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3C42B385842C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703192262; cv=none; b=GdAKFfOlsaAQK9LOJYnyq/H1alETMZx1Gnarfqis+Jc7SmT17QK+5CaUmS/xHz1ANsYHrm8vsLQhoNBy7weaRH/6X62AZL8WXMlKCPhJWq+RCOZbYgnJWAgWsYQI1LPA9uCQahZEOake3014WcHdO2/1h3P5nY/jrCgBv28sVrg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703192262; c=relaxed/simple; bh=OVy530cH2lhe1L6vk2ovnRqQmooPUqEA5SXfvC6qZdQ=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=FvSkMBxZKdxR50T+3vpHOA0uGERhFEVKsdx9ZAiCapryaQy8NpWWFnzFJ2JGUsMJMorxdliy9kwEVlZLCDDpfVUOA2OV9N4wPt9codDjWQJ/tRMGU3kehbuntQaaM7APX1CiBbOAMD4wS8qwmcyqjCOASd45l7A32gEAnfQ64pI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1703192260; bh=OVy530cH2lhe1L6vk2ovnRqQmooPUqEA5SXfvC6qZdQ=; h=Date:Subject:To:References:From:In-Reply-To:From; b=MPRT5/eGZStMrBuLfZjmcAiSFPS8UYn/JRRO/9LSIib01+UPbg7GEo8GP62kkX6nR n353kQPd/h5Pge9HkrYMj2RI8Vv4q8mZViU4JEMo38UgbhMG4yhDRTNF1BKcKvD8XP RgaCF+bU4ugjzr7dq+dSwZtcdjf50htIfSCO2GBY= Received: from [172.16.0.192] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id DE1141E0AC; Thu, 21 Dec 2023 15:57:40 -0500 (EST) Message-ID: <49a4d1af-1a22-4ea9-9bd6-e6d169b4c00d@simark.ca> Date: Thu, 21 Dec 2023 15:57:40 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 08/26] gdbserver: convert free_register_cache into a destructor of regcache Content-Language: fr To: Tankut Baris Aktemur , gdb-patches@sourceware.org References: From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org On 2/28/23 06:28, Tankut Baris Aktemur via Gdb-patches wrote: > Convert the `free_register_cache` function into a destructor of the > regcache struct. In one place, we completely remove the call to free > the regcache object by stack-allocating the object. > > We also delete the copy constructor explicitly to prevent the risk of > copying regcaches and then double-freeing buffers when they are > destructed. > --- > gdbserver/gdbthread.h | 2 +- > gdbserver/regcache.cc | 15 +++++---------- > gdbserver/regcache.h | 9 +++++---- > gdbserver/server.cc | 8 +++----- > 4 files changed, 14 insertions(+), 20 deletions(-) > > diff --git a/gdbserver/gdbthread.h b/gdbserver/gdbthread.h > index 493e1dbf6cb..cfd81870af9 100644 > --- a/gdbserver/gdbthread.h > +++ b/gdbserver/gdbthread.h > @@ -35,7 +35,7 @@ struct thread_info > > ~thread_info () > { > - free_register_cache (this->regcache_data); > + delete this->regcache_data; Perhaps in another patch, but you could make regcache_data a unique_ptr (I quickly skimmed the subjects of the rest of the series and didn't see a patch that would do that). > } > > /* The id of this thread. */ > diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc > index be01df342bb..7b6337166ad 100644 > --- a/gdbserver/regcache.cc > +++ b/gdbserver/regcache.cc > @@ -165,16 +165,11 @@ regcache::regcache (const target_desc *tdesc) > initialize (tdesc, nullptr); > } > > -void > -free_register_cache (struct regcache *regcache) > +regcache::~regcache () > { > - if (regcache) > - { > - if (regcache->registers_owned) > - free (regcache->registers); > - free (regcache->register_status); > - delete regcache; > - } > + if (registers_owned) > + free (registers); > + free (register_status); > } > > #endif > @@ -280,7 +275,7 @@ free_register_cache_thread (struct thread_info *thread) > if (regcache != NULL) > { > regcache_invalidate_thread (thread); > - free_register_cache (regcache); > + delete regcache; > set_thread_regcache_data (thread, NULL); > } > } > diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h > index 32b3a8dccfc..614d5a2561f 100644 > --- a/gdbserver/regcache.h > +++ b/gdbserver/regcache.h > @@ -51,6 +51,11 @@ struct regcache : public reg_buffer_common > /* Constructors. */ > regcache () = default; > regcache (const target_desc *tdesc); > + regcache (const regcache &rhs) = delete; > + regcache &operator= (const regcache &rhs) = delete; Use DISALLOW_COPY_AND_ASSIGN. And I think it wouldn't hurt to put it outside of the `#ifndef IN_PROCESS_AGENT`. If you follow my previous suggestions, the constructor with 2 arguments would also need to be outside the ifndef. > + > + /* Deconstructor. */ > + virtual ~regcache (); Deconstructor -> Destructor :) But really, I don't think it's necessary, anybody slightly familiar with C++ will know this is the destructor... as you wish. I guess you might need in the rest of the series, but as of now the destructor doesn't need to be virtual, so I wouldn't make it virtual in this patch. Simon