* RFA: cleanups -vs- gdb_fopen
@ 2008-10-28 18:43 Tom Tromey
2008-10-29 17:39 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2008-10-28 18:43 UTC (permalink / raw)
To: gdb-patches
This patch changes a few gdb_fopen leaks to use cleanups. This is
similar to yesterday's patch for fopen. Basically, there are some
situations where an error could leave an open file descriptor.
In some of these cases, a cleanup might not strictly be required.
Some of the guarded code can only call internal_error, not error.
However, I think it is safer to use cleanups, because it means the
caller is protected against changes in any callee.
Built & regtested on x86-64 (compile farm).
Ok?
Tom
2008-10-28 Tom Tromey <tromey@redhat.com>
* 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..5f1bcb8 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)
@@ -112,6 +114,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);
}
}
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: RFA: cleanups -vs- gdb_fopen
2008-10-28 18:43 RFA: cleanups -vs- gdb_fopen Tom Tromey
@ 2008-10-29 17:39 ` Pedro Alves
2008-10-30 21:52 ` Tom Tromey
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2008-10-29 17:39 UTC (permalink / raw)
To: gdb-patches, tromey
On Tuesday 28 October 2008 18:16:24, Tom Tromey wrote:
> @@ -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)
> @@ -112,6 +114,8 @@ handle_redirections (int from_tty)
> fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n",
> logging_filename);
>
> + discard_cleanups (cleanups);
This one had me scratch my head a bit:
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)
{
output = tee_file_new (gdb_stdout, 0, output, 1);
^^^^^^ ^
if (output == NULL)
perror_with_name (_("set logging"));
if (from_tty)
fprintf_unfiltered (gdb_stdout, "Copying output to %s.\n",
logging_filename);
}
else if (from_tty)
fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n",
logging_filename);
+ discard_cleanups (cleanups);
If an error is thrown after tee_file_new returning a valid file (it
always does currently), you'd run the cleanup on the old `output',
and leave the new `output' leaking. That can't happen with the current
code, so, your patch looks good to me.
Thanks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: RFA: cleanups -vs- gdb_fopen
2008-10-29 17:39 ` Pedro Alves
@ 2008-10-30 21:52 ` Tom Tromey
2008-10-30 21:35 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2008-10-30 21:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> 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 <tromey@redhat.com>
* 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);
}
}
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-10-30 21:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-28 18:43 RFA: cleanups -vs- gdb_fopen Tom Tromey
2008-10-29 17:39 ` Pedro Alves
2008-10-30 21:52 ` Tom Tromey
2008-10-30 21:35 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox