From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24173 invoked by alias); 30 Oct 2008 21:50:47 -0000 Received: (qmail 23705 invoked by uid 22791); 30 Oct 2008 21:50:45 -0000 X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 30 Oct 2008 21:48:45 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id m9UKMEdU004341; Thu, 30 Oct 2008 16:22:14 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m9UKMDEJ027964; Thu, 30 Oct 2008 16:22:13 -0400 Received: from opsy.redhat.com (vpn-12-180.rdu.redhat.com [10.11.12.180]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id m9UKMCIM027927; Thu, 30 Oct 2008 16:22:12 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 8BD093786F5; Thu, 30 Oct 2008 14:22:06 -0600 (MDT) To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: RFA: cleanups -vs- gdb_fopen References: <200810291645.56749.pedro@codesourcery.com> From: Tom Tromey Reply-To: Tom Tromey X-Attribution: Tom Date: Thu, 30 Oct 2008 21:52:00 -0000 In-Reply-To: <200810291645.56749.pedro@codesourcery.com> (Pedro Alves's message of "Wed\, 29 Oct 2008 16\:45\:56 +0000") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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 X-SW-Source: 2008-10/txt/msg00712.txt.bz2 >>>>> "Pedro" == Pedro Alves writes: Pedro> This one had me scratch my head a bit: [...] Pedro> If an error is thrown after tee_file_new returning a valid file (it Pedro> always does currently), you'd run the cleanup on the old `output', Pedro> and leave the new `output' leaking. That can't happen with the current Pedro> code, so, your patch looks good to me. Oops, I missed that. I think it is nice to try to be safe about this sort of thing. That way people don't have to think too hard when making changes between opening a file and closing it. What do you think of this variant, which fixes the cleanup after tee_file_new? Tom 2008-10-28 Tom Tromey * cli/cli-logging.c (handle_redirections): Make a cleanup. * reggroups.c (maintenance_print_reggroups): Make a cleanup. * regcache.c (regcache_print): Make a cleanup. * maint.c (maintenance_print_architecture): Make a cleanup. * dummy-frame.c (maintenance_print_dummy_frames): Make a cleanup. diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c index 86f1bc0..1e941b1 100644 --- a/gdb/cli/cli-logging.c +++ b/gdb/cli/cli-logging.c @@ -85,6 +85,7 @@ pop_output_files (void) static void handle_redirections (int from_tty) { + struct cleanup *cleanups; struct ui_file *output; if (saved_filename != NULL) @@ -97,6 +98,7 @@ handle_redirections (int from_tty) output = gdb_fopen (logging_filename, logging_overwrite ? "w" : "a"); if (output == NULL) perror_with_name (_("set logging")); + cleanups = make_cleanup_ui_file_delete (output); /* Redirects everything to gdb_stdout while this is running. */ if (!logging_redirect) @@ -104,6 +106,8 @@ handle_redirections (int from_tty) output = tee_file_new (gdb_stdout, 0, output, 1); if (output == NULL) perror_with_name (_("set logging")); + discard_cleanups (cleanups); + cleanups = make_cleanup_ui_file_delete (output); if (from_tty) fprintf_unfiltered (gdb_stdout, "Copying output to %s.\n", logging_filename); @@ -112,6 +116,8 @@ handle_redirections (int from_tty) fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n", logging_filename); + discard_cleanups (cleanups); + saved_filename = xstrdup (logging_filename); saved_output.out = gdb_stdout; saved_output.err = gdb_stderr; diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c index a27de2e..9cc3da7 100644 --- a/gdb/dummy-frame.c +++ b/gdb/dummy-frame.c @@ -265,11 +265,13 @@ maintenance_print_dummy_frames (char *args, int from_tty) fprint_dummy_frames (gdb_stdout); else { + struct cleanup *cleanups; struct ui_file *file = gdb_fopen (args, "w"); if (file == NULL) perror_with_name (_("maintenance print dummy-frames")); + cleanups = make_cleanup_ui_file_delete (file); fprint_dummy_frames (file); - ui_file_delete (file); + do_cleanups (cleanups); } } diff --git a/gdb/maint.c b/gdb/maint.c index e64d4fe..365e374 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -414,11 +414,13 @@ maintenance_print_architecture (char *args, int from_tty) gdbarch_dump (current_gdbarch, gdb_stdout); else { + struct cleanup *cleanups; struct ui_file *file = gdb_fopen (args, "w"); if (file == NULL) perror_with_name (_("maintenance print architecture")); + cleanups = make_cleanup_ui_file_delete (file); gdbarch_dump (current_gdbarch, file); - ui_file_delete (file); + do_cleanups (cleanups); } } diff --git a/gdb/regcache.c b/gdb/regcache.c index 616a6f7..74ca6f0 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -1104,11 +1104,13 @@ regcache_print (char *args, enum regcache_dump_what what_to_dump) regcache_dump (get_current_regcache (), gdb_stdout, what_to_dump); else { + struct cleanup *cleanups; struct ui_file *file = gdb_fopen (args, "w"); if (file == NULL) perror_with_name (_("maintenance print architecture")); + cleanups = make_cleanup_ui_file_delete (file); regcache_dump (get_current_regcache (), file, what_to_dump); - ui_file_delete (file); + do_cleanups (cleanups); } } diff --git a/gdb/reggroups.c b/gdb/reggroups.c index ea2451e..a4e1d31 100644 --- a/gdb/reggroups.c +++ b/gdb/reggroups.c @@ -234,11 +234,13 @@ maintenance_print_reggroups (char *args, int from_tty) reggroups_dump (current_gdbarch, gdb_stdout); else { + struct cleanup *cleanups; struct ui_file *file = gdb_fopen (args, "w"); if (file == NULL) perror_with_name (_("maintenance print reggroups")); + cleanups = make_cleanup_ui_file_delete (file); reggroups_dump (current_gdbarch, file); - ui_file_delete (file); + do_cleanups (cleanups); } }