From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 34145 invoked by alias); 23 Aug 2017 09:33:02 -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 34039 invoked by uid 89); 23 Aug 2017 09:33:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LOTSOFHASH,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=releasing, H*r:sk:static. X-HELO: mail-it0-f65.google.com Received: from mail-it0-f65.google.com (HELO mail-it0-f65.google.com) (209.85.214.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 23 Aug 2017 09:32:58 +0000 Received: by mail-it0-f65.google.com with SMTP id w204so1005137ita.5 for ; Wed, 23 Aug 2017 02:32:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=CZ3+k+uZh4yMBvr4LfpjYayvn1lokt4K6QtRDLdkDfs=; b=Z8j2fv6wBPHnhwv4Z5erqhVWPeepaPvn+8VlnwEi+137yotLmMVGvkT43D0F5ZhhRN pTe1C9wPnFvYt0YHC1XtDyRn1pBqnS798c2woCtnEGncZUKwtK8nhVb/2VnB9nXl8WGK 6jq+MqSKB//gcvnt9Fz+r9dnZxOQYoz03wC8964ObYMxkC5PPpWGFWSfz3e9cWTzbGIN lSx9cENB6rB8X5nTtbn0DHSjZAyR4JrZeJgf4LwXC38Irih52OSRJIyi+QBexERqNcOq qTFCfNIq01MrATFA52tRZv2K+jvE07mYg2mS8+K8q3jehnSTcAot7gJiwl8lX/SEqftu zE8A== X-Gm-Message-State: AHYfb5gtxR5g+QU7oDkToLBLXhpVMvgSdYLIIJQNJHgmeSTKt1FWN/DX iNCpeTeKgDFXP+y8 X-Received: by 10.36.43.68 with SMTP id h65mr2383787ita.41.1503480777228; Wed, 23 Aug 2017 02:32:57 -0700 (PDT) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id q68sm549630iof.79.2017.08.23.02.32.55 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 23 Aug 2017 02:32:56 -0700 (PDT) From: Yao Qi To: Alan Hayward Cc: "gdb-patches\@sourceware.org" , nd Subject: Re: [PATCH 3/7]: Regcache: Remove xmalloc/xfree methods References: <08C93960-8ED8-431A-B786-3455FF149B77@arm.com> Date: Wed, 23 Aug 2017 09:33:00 -0000 In-Reply-To: <08C93960-8ED8-431A-B786-3455FF149B77@arm.com> (Alan Hayward's message of "Thu, 17 Aug 2017 08:47:54 +0000") Message-ID: <86ziaqr5mi.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00434.txt.bz2 Alan Hayward writes: > Regcache is a class. There is no need for explicit xmalloc > and xfree methods, it is much simpler to use new and delete. > new/delete isn't simpler than xmalloc/xfree, IMO. We still need to take care of releasing resources. > > diff --git a/gdb/frame.c b/gdb/frame.c > index 30e4aeab7e2901074c289ac4d96ebda885805a29..7fd4b07a2e95f28b2eb6a18de= a4d2071f0ece4e2 100644 > --- a/gdb/frame.c > +++ b/gdb/frame.c > @@ -1021,13 +1021,12 @@ struct regcache * > frame_save_as_regcache (struct frame_info *this_frame) > { > struct address_space *aspace =3D get_frame_address_space (this_frame); > - struct regcache *regcache =3D regcache_xmalloc (get_frame_arch (this_f= rame), > - aspace); > - struct cleanup *cleanups =3D make_cleanup_regcache_xfree (regcache); > + regcache *backup =3D new regcache (get_frame_arch (this_frame), aspace= ); > + struct cleanup *cleanups =3D make_cleanup_regcache_delete (backup); > It makes no sense to change make_cleanup_regcache_xfree to make_cleanup_regcache_delete. We still have to use cleanup. > - regcache_save (regcache, do_frame_register_read, this_frame); > + regcache_save (backup, do_frame_register_read, this_frame); > discard_cleanups (cleanups); > - return regcache; > + return backup; > } > > void > @@ -1063,7 +1062,7 @@ frame_pop (struct frame_info *this_frame) > trying to extract the old values from the current regcache while > at the same time writing new values into that same cache. */ > scratch =3D frame_save_as_regcache (prev_frame); > - cleanups =3D make_cleanup_regcache_xfree (scratch); > + cleanups =3D make_cleanup_regcache_delete (scratch); scratch is only used within this function, so we can change it to a local object (instead of a pointer), and call regcache_save here. Then, we can remove the cleanups. > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 0bf587ffe702c68f538fe2877cce6114e64ee1bd..1edcf752c82230fc65669f675= e10735ac8e4f654 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -1051,7 +1051,7 @@ mi_cmd_data_list_changed_registers (const char *com= mand, char **argv, int argc) > > prev_regs =3D this_regs; > this_regs =3D frame_save_as_regcache (get_selected_frame (NULL)); > - cleanup =3D make_cleanup_regcache_xfree (prev_regs); > + cleanup =3D make_cleanup_regcache_delete (prev_regs); Why don't you do this? then, we don't need this make_cleanup_regcache_xfree. std::unique_ptr prev_regs (this_regs); > > /* Note that the test for a valid register must include checking the > gdbarch_register_name because gdbarch_num_regs may be allocated > diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c > index 324b29d90c1266a73f172da20a6015174189625f..42aff2cd1bf3834268b0b58dc= f00dac1c52add96 100644 > --- a/gdb/ppc-linux-tdep.c > +++ b/gdb/ppc-linux-tdep.c > @@ -1364,13 +1364,13 @@ ppu2spu_sniffer (const struct frame_unwind *self, > =3D FRAME_OBSTACK_CALLOC (1, struct ppu2spu_cache); > > struct address_space *aspace =3D get_frame_address_space (this_frame); > - struct regcache *regcache =3D regcache_xmalloc (data.gdbarch, aspace); > - struct cleanup *cleanups =3D make_cleanup_regcache_xfree (regcache); > - regcache_save (regcache, ppu2spu_unwind_register, &data); > + regcache *backup =3D new regcache (data.gdbarch, aspace); > + struct cleanup *cleanups =3D make_cleanup_regcache_delete (backup); > + regcache_save (backup, ppu2spu_unwind_register, &data); > discard_cleanups (cleanups); We can use std::unique_ptr too here. After call regcache_save, call .release () to return the raw pointer to cache->regcach= e. > > cache->frame_id =3D frame_id_build (base, func); > - cache->regcache =3D regcache; > + cache->regcache =3D backup; > *this_prologue_cache =3D cache; > return 1; > } > @@ -1383,7 +1383,7 @@ static void > ppu2spu_dealloc_cache (struct frame_info *self, void *this_cache) > { > struct ppu2spu_cache *cache =3D (struct ppu2spu_cache *) this_cache; > - regcache_xfree (cache->regcache); > + delete cache->regcache; > } > > static const struct frame_unwind ppu2spu_unwind =3D { > diff --git a/gdb/regcache.h b/gdb/regcache.h > index aeac54e8d7b91e2bcf6a3e1ce8e781c3f994306b..00b87db7d145205b74859c08c= a5a9d852441a4aa 100644 > --- a/gdb/regcache.h > +++ b/gdb/regcache.h > @@ -36,10 +36,7 @@ extern target_regcache *get_thread_arch_aspace_regcach= e (ptid_t, > struct gdbarch *, > struct address_space *); > > -void regcache_xfree (struct regcache *regcache); > -struct cleanup *make_cleanup_regcache_xfree (struct regcache *regcache); > -struct regcache *regcache_xmalloc (struct gdbarch *gdbarch, > - struct address_space *aspace); > +struct cleanup *make_cleanup_regcache_delete (regcache *regcache); > > /* Return REGCACHE's ptid. */ > > @@ -261,12 +258,7 @@ public: > regcache (const regcache &) =3D delete; > void operator=3D (const regcache &) =3D delete; > > - /* class regcache is only extended in unit test, so only mark it > - virtual when selftest is enabled. */ > -#if GDB_SELF_TEST > - virtual > -#endif > - ~regcache () > + virtual ~regcache () This is not related to this patch. It should be in patch #1. --=20 Yao (=E9=BD=90=E5=B0=A7)