From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16380 invoked by alias); 22 Apr 2018 14:21: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 16368 invoked by uid 89); 22 Apr 2018 14:21:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 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.2 spammy=HTo:U*tom X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 22 Apr 2018 14:21:57 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w3MELouh020223 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sun, 22 Apr 2018 10:21:55 -0400 Received: by simark.ca (Postfix, from userid 112) id BB5681E778; Sun, 22 Apr 2018 10:21:50 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id F04651E47D; Sun, 22 Apr 2018 10:21:49 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 22 Apr 2018 14:21:00 -0000 From: Simon Marchi To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [RFA] Remove a cleanup from scm-frame.c In-Reply-To: <20180421223936.31584-1-tom@tromey.com> References: <20180421223936.31584-1-tom@tromey.com> Message-ID: <43a02241b2a0b1178a6dd8dfbd2ad581@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.4 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 22 Apr 2018 14:21:50 +0000 X-IsSubscribed: yes X-SW-Source: 2018-04/txt/msg00434.txt.bz2 On 2018-04-21 18:39, Tom Tromey wrote: > This removes a cleanup from scm-frame.c, replacing it with > unique_xmalloc_ptr and a new scope. I believe this also fixes a > latent bug involving calling do_cleanups twice for a single cleanup. > > Regression tested using the gdb.guile test suite on x86-64 Fedora 26. > > ChangeLog > 2018-04-21 Tom Tromey > > * guile/scm-frame.c (gdbscm_frame_read_var): Use > gdb::unique_xmalloc_ptr. > --- > gdb/ChangeLog | 5 +++++ > gdb/guile/scm-frame.c | 54 > ++++++++++++++++++++++++--------------------------- > 2 files changed, 30 insertions(+), 29 deletions(-) > > diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c > index 4f4766aceb..7b539677ff 100644 > --- a/gdb/guile/scm-frame.c > +++ b/gdb/guile/scm-frame.c > @@ -877,7 +877,6 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, > SCM rest) > } > else if (scm_is_string (symbol_scm)) > { > - char *var_name; > const struct block *block = NULL; > struct cleanup *cleanup; > struct gdb_exception except = exception_none; > @@ -893,38 +892,35 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, > SCM rest) > gdbscm_throw (except_scm); > } > > - var_name = gdbscm_scm_to_c_string (symbol_scm); > - cleanup = make_cleanup (xfree, var_name); > - /* N.B. Between here and the call to do_cleanups, don't do > anything > - to cause a Scheme exception without performing the cleanup. */ > + { > + gdb::unique_xmalloc_ptr var_name > + (gdbscm_scm_to_c_string (symbol_scm)); > + /* N.B. Between here and the end of the scope, don't do anything > + to cause a Scheme exception. */ > + > + TRY > + { > + struct block_symbol lookup_sym; > + > + if (block == NULL) > + block = get_frame_block (frame, NULL); > + lookup_sym = lookup_symbol (var_name.get (), block, VAR_DOMAIN, > + NULL); > + var = lookup_sym.symbol; > + block = lookup_sym.block; > + } > + CATCH (ex, RETURN_MASK_ALL) > + { > + except = ex; > + } > + END_CATCH > + } > > - TRY > - { > - struct block_symbol lookup_sym; > - > - if (block == NULL) > - block = get_frame_block (frame, NULL); > - lookup_sym = lookup_symbol (var_name, block, VAR_DOMAIN, NULL); > - var = lookup_sym.symbol; > - block = lookup_sym.block; > - } > - CATCH (ex, RETURN_MASK_ALL) > - { > - except = ex; > - } > - END_CATCH > - > - do_cleanups (cleanup); > GDBSCM_HANDLE_GDB_EXCEPTION (except); > > if (var == NULL) > - { > - do_cleanups (cleanup); > - gdbscm_out_of_range_error (FUNC_NAME, 0, symbol_scm, > - _("variable not found")); > - } > - > - do_cleanups (cleanup); > + gdbscm_out_of_range_error (FUNC_NAME, 0, symbol_scm, > + _("variable not found")); > } > else > { This looks good to me at first glance. Do you know if scm exceptions (scm_throw) play well with C++, the destructors of the objects in the exited scopes will correctly be called? Simon