RFA: fix fopen related cleanups
Tom Tromey
tromey@redhat.com
Tue Oct 28 02:04:00 GMT 2008
While looking into another issue, I noticed a number of fopen-related
buglets in gdb. In particular, there are several places in gdb where
a file is not properly closed if an error occurs.
This patch fixes all the fopen-related problems. It also fixes a
couple memory leaks I noticed while writing this. Finally, it
consolidates all the implementations of make_cleanup_fclose, explicit
or implicit, into a single place.
I think there are also some file-close-related flaws having to do with
open (procfs_pidlist -- this function is flawed in a few ways) and
also gdb_fopen (instances in dummy-frame.c, regcache.c, and
reggroups.c according to my notes). I haven't yet checked opendir,
socket, or pipe. I will deal with those in separate patches.
Built and regtested on x86-64 (compile farm).
Of course, this does not show whether the bug is fixed or not.
For that I relied on inspection.
Please review.
Tom
2008-10-27 Tom Tromey <tromey@redhat.com>
* cli/cli-script.c (do_fclose_cleanup): Remove.
(script_from_file): Use make_cleanup_fclose.
* xml-tdesc.c (do_cleanup_fclose): Remove.
(fetch_xml_from_file): Use make_cleanup_fclose.
* tracepoint.c (tracepoint_save_command): Use
make_cleanup_fclose. Always free pathname.
* source.c (print_source_lines_base): Use make_cleanup_fclose.
* remote.c (fclose_cleanup): Remove.
(remote_file_put): Use make_cleanup_fclose.
(remote_file_get): Likewise.
* linux-nat.c (linux_nat_find_memory_regions): Use
make_cleanup_fclose.
(linux_nat_info_proc_cmd): Likewise.
(linux_proc_pending_signals): Likewise.
* fbsd-nat.c (fbsd_find_memory_regions): Use make_cleanup_fclose.
Free file name.
* cli/cli-dump.c (do_fclose_cleanup): Remove.
(make_cleanup_fclose): Remove.
* defs.h (make_cleanup_fclose): Declare.
* utils.c (do_fclose_cleanup): New function.
(make_cleanup_fclose): Likewise.
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 67a9fc5..d5dbc97 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -67,19 +67,6 @@ scan_expression_with_cleanup (char **cmd, const char *def)
}
-static void
-do_fclose_cleanup (void *arg)
-{
- FILE *file = arg;
- fclose (arg);
-}
-
-static struct cleanup *
-make_cleanup_fclose (FILE *file)
-{
- return make_cleanup (do_fclose_cleanup, file);
-}
-
char *
scan_filename_with_cleanup (char **cmd, const char *defname)
{
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 6bfff40..e65c29e 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1446,12 +1446,6 @@ source_cleanup_lines (void *args)
source_file_name = p->old_file;
}
-static void
-do_fclose_cleanup (void *stream)
-{
- fclose (stream);
-}
-
struct wrapped_read_command_file_args
{
FILE *stream;
@@ -1476,7 +1470,7 @@ script_from_file (FILE *stream, char *file)
if (stream == NULL)
internal_error (__FILE__, __LINE__, _("called with NULL file pointer!"));
- old_cleanups = make_cleanup (do_fclose_cleanup, stream);
+ old_cleanups = make_cleanup_fclose (stream);
old_lines.old_line = source_line_number;
old_lines.old_file = source_file_name;
diff --git a/gdb/defs.h b/gdb/defs.h
index 42dd821..8d50f8a 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -362,6 +362,8 @@ extern struct cleanup *(make_cleanup_free_section_addr_info
extern struct cleanup *make_cleanup_close (int fd);
+extern struct cleanup *make_cleanup_fclose (FILE *file);
+
extern struct cleanup *make_cleanup_bfd_close (bfd *abfd);
extern struct cleanup *make_cleanup_restore_integer (int *variable);
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 215d2a7..9fed5bf 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -101,11 +101,14 @@ fbsd_find_memory_regions (int (*func) (CORE_ADDR, unsigned long,
unsigned long start, end, size;
char protection[4];
int read, write, exec;
+ struct cleanup *cleanup;
mapfilename = xstrprintf ("/proc/%ld/map", (long) pid);
+ cleanup = make_cleanup (xfree, mapfilename);
mapfile = fopen (mapfilename, "r");
if (mapfile == NULL)
error (_("Couldn't open %s."), mapfilename);
+ make_cleanup_fclose (mapfile);
if (info_verbose)
fprintf_filtered (gdb_stdout,
@@ -134,7 +137,7 @@ fbsd_find_memory_regions (int (*func) (CORE_ADDR, unsigned long,
func (start, size, read, write, exec, obfd);
}
- fclose (mapfile);
+ do_cleanups (cleanup);
return 0;
}
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d2116e1..56ec9cb 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3334,11 +3334,13 @@ linux_nat_find_memory_regions (int (*func) (CORE_ADDR,
char permissions[8], device[8], filename[MAXPATHLEN];
int read, write, exec;
int ret;
+ struct cleanup *cleanup;
/* Compose the filename for the /proc memory map, and open it. */
sprintf (mapsfilename, "/proc/%lld/maps", pid);
if ((mapsfile = fopen (mapsfilename, "r")) == NULL)
error (_("Could not open %s."), mapsfilename);
+ cleanup = make_cleanup_fclose (mapsfile);
if (info_verbose)
fprintf_filtered (gdb_stdout,
@@ -3371,7 +3373,7 @@ linux_nat_find_memory_regions (int (*func) (CORE_ADDR,
segment. */
func (addr, size, read, write, exec, obfd);
}
- fclose (mapsfile);
+ do_cleanups (cleanup);
return 0;
}
@@ -3662,9 +3664,10 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
sprintf (fname1, "/proc/%lld/cmdline", pid);
if ((procfile = fopen (fname1, "r")) != NULL)
{
+ struct cleanup *cleanup = make_cleanup_fclose (procfile);
fgets (buffer, sizeof (buffer), procfile);
printf_filtered ("cmdline = '%s'\n", buffer);
- fclose (procfile);
+ do_cleanups (cleanup);
}
else
warning (_("unable to open /proc file '%s'"), fname1);
@@ -3694,7 +3697,9 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
{
long long addr, endaddr, size, offset, inode;
char permissions[8], device[8], filename[MAXPATHLEN];
+ struct cleanup *cleanup;
+ cleanup = make_cleanup_fclose (procfile);
printf_filtered (_("Mapped address spaces:\n\n"));
if (gdbarch_addr_bit (current_gdbarch) == 32)
{
@@ -3742,7 +3747,7 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
}
}
- fclose (procfile);
+ do_cleanups (cleanup);
}
else
warning (_("unable to open /proc file '%s'"), fname1);
@@ -3752,9 +3757,10 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
sprintf (fname1, "/proc/%lld/status", pid);
if ((procfile = fopen (fname1, "r")) != NULL)
{
+ struct cleanup *cleanup = make_cleanup_fclose (procfile);
while (fgets (buffer, sizeof (buffer), procfile) != NULL)
puts_filtered (buffer);
- fclose (procfile);
+ do_cleanups (cleanup);
}
else
warning (_("unable to open /proc file '%s'"), fname1);
@@ -3767,6 +3773,7 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
int itmp;
char ctmp;
long ltmp;
+ struct cleanup *cleanup = make_cleanup_fclose (procfile);
if (fscanf (procfile, "%d ", &itmp) > 0)
printf_filtered (_("Process: %d\n"), itmp);
@@ -3850,7 +3857,7 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
if (fscanf (procfile, "%lu ", <mp) > 0) /* FIXME arch? */
printf_filtered (_("wchan (system call): 0x%lx\n"), ltmp);
#endif
- fclose (procfile);
+ do_cleanups (cleanup);
}
else
warning (_("unable to open /proc file '%s'"), fname1);
@@ -3952,6 +3959,7 @@ linux_proc_pending_signals (int pid, sigset_t *pending, sigset_t *blocked, sigse
FILE *procfile;
char buffer[MAXPATHLEN], fname[MAXPATHLEN];
int signum;
+ struct cleanup *cleanup;
sigemptyset (pending);
sigemptyset (blocked);
@@ -3960,6 +3968,7 @@ linux_proc_pending_signals (int pid, sigset_t *pending, sigset_t *blocked, sigse
procfile = fopen (fname, "r");
if (procfile == NULL)
error (_("Could not open %s"), fname);
+ cleanup = make_cleanup_fclose (procfile);
while (fgets (buffer, MAXPATHLEN, procfile) != NULL)
{
@@ -3981,7 +3990,7 @@ linux_proc_pending_signals (int pid, sigset_t *pending, sigset_t *blocked, sigse
add_line_to_sigset (buffer + 8, ignored);
}
- fclose (procfile);
+ do_cleanups (cleanup);
}
static LONGEST
diff --git a/gdb/remote.c b/gdb/remote.c
index f334c0b..a40ca3a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7322,12 +7322,6 @@ remote_hostio_error (int errnum)
}
static void
-fclose_cleanup (void *file)
-{
- fclose (file);
-}
-
-static void
remote_hostio_close_cleanup (void *opaque)
{
int fd = *(int *) opaque;
@@ -7444,7 +7438,7 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
file = fopen (local_file, "rb");
if (file == NULL)
perror_with_name (local_file);
- back_to = make_cleanup (fclose_cleanup, file);
+ back_to = make_cleanup_fclose (file);
fd = remote_hostio_open (remote_file, (FILEIO_O_WRONLY | FILEIO_O_CREAT
| FILEIO_O_TRUNC),
@@ -7534,7 +7528,7 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty)
file = fopen (local_file, "wb");
if (file == NULL)
perror_with_name (local_file);
- back_to = make_cleanup (fclose_cleanup, file);
+ back_to = make_cleanup_fclose (file);
/* Send up to this many bytes at once. They won't all fit in the
remote packet limit, so we'll transfer slightly fewer. */
diff --git a/gdb/source.c b/gdb/source.c
index a5f434f..3ef557c 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1312,6 +1312,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
int desc;
FILE *stream;
int nlines = stopline - line;
+ struct cleanup *cleanup;
/* Regardless of whether we can open the file, set current_source_symtab. */
current_source_symtab = s;
@@ -1378,6 +1379,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
stream = fdopen (desc, FDOPEN_MODE);
clearerr (stream);
+ cleanup = make_cleanup_fclose (stream);
while (nlines-- > 0)
{
@@ -1417,7 +1419,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
while (c != '\n' && (c = fgetc (stream)) >= 0);
}
- fclose (stream);
+ do_cleanups (cleanup);
}
/* Show source lines from the file of symtab S, starting with line
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 4f5c56a..2d2c3bd 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2292,6 +2292,7 @@ tracepoint_save_command (char *args, int from_tty)
char *i1 = " ", *i2 = " ";
char *indent, *actionline, *pathname;
char tmp[40];
+ struct cleanup *cleanup;
if (args == 0 || *args == 0)
error (_("Argument required (file name in which to save tracepoints)"));
@@ -2303,10 +2304,11 @@ tracepoint_save_command (char *args, int from_tty)
}
pathname = tilde_expand (args);
+ cleanup = make_cleanup (xfree, pathname);
if (!(fp = fopen (pathname, "w")))
error (_("Unable to open file '%s' for saving tracepoints (%s)"),
args, safe_strerror (errno));
- xfree (pathname);
+ make_cleanup_fclose (fp);
ALL_TRACEPOINTS (tp)
{
@@ -2348,7 +2350,7 @@ tracepoint_save_command (char *args, int from_tty)
}
}
}
- fclose (fp);
+ do_cleanups (cleanup);
if (from_tty)
printf_filtered ("Tracepoints saved to file '%s'.\n", args);
return;
diff --git a/gdb/utils.c b/gdb/utils.c
index 3d35390..f9a5f19 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -255,6 +255,23 @@ make_cleanup_close (int fd)
return make_cleanup (do_close_cleanup, saved_fd);
}
+/* Helper function which does the work for make_cleanup_fclose. */
+
+static void
+do_fclose_cleanup (void *arg)
+{
+ FILE *file = arg;
+ fclose (arg);
+}
+
+/* Return a new cleanup that closes FILE. */
+
+struct cleanup *
+make_cleanup_fclose (FILE *file)
+{
+ return make_cleanup (do_fclose_cleanup, file);
+}
+
static void
do_ui_file_delete (void *arg)
{
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 42bc4a0..2e8c1f5 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -421,14 +421,6 @@ tdesc_parse_xml (const char *document, xml_fetch_another fetcher,
#endif /* HAVE_LIBEXPAT */
-/* Close FILE. */
-
-static void
-do_cleanup_fclose (void *file)
-{
- fclose (file);
-}
-
/* Open FILENAME, read all its text into memory, close it, and return
the text. If something goes wrong, return NULL and warn. */
@@ -455,7 +447,7 @@ fetch_xml_from_file (const char *filename, void *baton)
if (file == NULL)
return NULL;
- back_to = make_cleanup (do_cleanup_fclose, file);
+ back_to = make_cleanup_fclose (file);
/* Read in the whole file, one chunk at a time. */
len = 4096;
More information about the Gdb-patches
mailing list