From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 34174 invoked by alias); 31 Jul 2017 18:47: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 34080 invoked by uid 89); 31 Jul 2017 18:47:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=39,8, 6107 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; Mon, 31 Jul 2017 18:46:58 +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 v6VIkplj004892 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 31 Jul 2017 14:46:56 -0400 Received: by simark.ca (Postfix, from userid 112) id D35BD1EA01; Mon, 31 Jul 2017 14:46:51 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 0DE0E1E043; Mon, 31 Jul 2017 14:46:41 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 31 Jul 2017 18:47:00 -0000 From: Simon Marchi To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [RFA v2 08/24] Remove an unlink cleanup In-Reply-To: <20170725172107.9799-9-tom@tromey.com> References: <20170725172107.9799-1-tom@tromey.com> <20170725172107.9799-9-tom@tromey.com> Message-ID: <58baf0d74111cecc23838a9ece4ea69c@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 31 Jul 2017 18:46:52 +0000 X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00450.txt.bz2 On 2017-07-25 19:20, Tom Tromey wrote: > compile/compile.c had its own cleanup to unlink a file. This patch > replaces this cleanup with gdb::unlinker. > > ChangeLog > 2017-07-25 Tom Tromey > > * compile/compile.c (cleanup_unlink_file): Remove. > (compile_to_object): Use gdb::unlinker. > (eval_compile_command): Likewise. > --- > gdb/ChangeLog | 6 ++++++ > gdb/compile/compile.c | 38 +++++++++++++++++++------------------- > 2 files changed, 25 insertions(+), 19 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 9d46731..2bf549b 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,5 +1,11 @@ > 2017-07-25 Tom Tromey > > + * compile/compile.c (cleanup_unlink_file): Remove. > + (compile_to_object): Use gdb::unlinker. > + (eval_compile_command): Likewise. > + > +2017-07-25 Tom Tromey > + > * utils.h (make_cleanup_fclose): Remove. > * utils.c (do_fclose_cleanup, make_cleanup_fclose): Remove. > > diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c > index d8c505f..5269aaf 100644 > --- a/gdb/compile/compile.c > +++ b/gdb/compile/compile.c > @@ -39,6 +39,8 @@ > #include "osabi.h" > #include "gdb_wait.h" > #include "valprint.h" > +#include "common/gdb_optional.h" > +#include "common/gdb_unlinker.h" > > > > @@ -427,16 +429,6 @@ cleanup_compile_instance (void *arg) > inst->destroy (inst); > } > > -/* A cleanup function to unlink a file. */ > - > -static void > -cleanup_unlink_file (void *arg) > -{ > - const char *filename = (const char *) arg; > - > - unlink (filename); > -} > - > /* A helper function suitable for use as the "print_callback" in the > compiler object. */ > > @@ -455,7 +447,7 @@ compile_to_object (struct command_line *cmd, const > char *cmd_string, > enum compile_i_scope_types scope) > { > struct compile_instance *compiler; > - struct cleanup *cleanup, *inner_cleanup; > + struct cleanup *cleanup; > const struct block *expr_block; > CORE_ADDR trash_pc, expr_pc; > int argc; > @@ -547,12 +539,15 @@ compile_to_object (struct command_line *cmd, > const char *cmd_string, > > compile_file_names fnames = get_new_file_names (); > > + gdb::optional source_remover; > + > { > gdb_file_up src = gdb_fopen_cloexec (fnames.source_file (), "w"); > if (src == NULL) > perror_with_name (_("Could not open source file for writing")); > - inner_cleanup = make_cleanup (cleanup_unlink_file, > - (void *) fnames.source_file ()); > + > + source_remover.emplace (fnames.source_file ()); > + > if (fputs (code.c_str (), src.get ()) == EOF) > perror_with_name (_("Could not write to source file")); > } > @@ -572,7 +567,9 @@ compile_to_object (struct command_line *cmd, const > char *cmd_string, > fprintf_unfiltered (gdb_stdlog, "object file produced: %s\n\n", > fnames.object_file ()); > > - discard_cleanups (inner_cleanup); > + /* Keep the source file. */ > + source_remover->keep (); > + > do_cleanups (cleanup); > > return fnames; > @@ -594,14 +591,13 @@ void > eval_compile_command (struct command_line *cmd, const char > *cmd_string, > enum compile_i_scope_types scope, void *scope_data) > { > - struct cleanup *cleanup_unlink; > struct compile_module *compile_module; > > compile_file_names fnames = compile_to_object (cmd, cmd_string, > scope); > > - cleanup_unlink = make_cleanup (cleanup_unlink_file, > - (void *) fnames.object_file ()); > - make_cleanup (cleanup_unlink_file, (void *) fnames.source_file ()); > + gdb::unlinker object_remover (fnames.object_file ()); > + gdb::unlinker source_remover (fnames.source_file ()); > + > compile_module = compile_object_load (fnames, scope, scope_data); > if (compile_module == NULL) > { > @@ -610,7 +606,11 @@ eval_compile_command (struct command_line *cmd, > const char *cmd_string, > COMPILE_I_PRINT_VALUE_SCOPE, scope_data); > return; > } > - discard_cleanups (cleanup_unlink); > + > + /* Keep the files. */ > + source_remover.keep (); > + object_remover.keep (); > + > compile_object_run (compile_module); > } LGTM. For the next patches, I'll skip those that Pedro had simply OK'ed in v1, since I don't think it needs reviewing again. Simon