From: Tom Tromey <tromey@redhat.com>
To: gdb-patches@sourceware.org
Subject: RFA: open-related cleanup handling
Date: Tue, 28 Oct 2008 20:21:00 -0000 [thread overview]
Message-ID: <m3fxmgmqx0.fsf@fleche.redhat.com> (raw)
Another in a series of possible file descriptor leak fixes.
This fixes most leaks associated with open, and also opendir. I say
most because there is one more patch that is a bit more odd that I
wanted to submit separately.
This one should not be too controversial. I found the problems in
question by looking at all calls to open or opendir, and then all
calls to all functions returning the result of open or opendir. I
also looked at all calls to socket, accept, and pipe, but none of
these showed bugs. (I did not look at gdbserver at all, FWIW.)
Also note the changes to symtab_to_fullname and psymtab_to_fullname.
These incorrectly test for an error return from find_and_open_source.
Built and regtested on x86-64 (compile farm), though again, this is
likely not a comprehensive test.
Ok?
Tom
2008-10-28 Tom Tromey <tromey@redhat.com>
* source.c (symtab_to_fullname): Test 'r >= 0'.
(psymtab_to_fullname): Likewise.
(get_filename_and_charpos): Make a cleanup.
(forward_search_command): Likewise.
(reverse_search_command): Likewise.
* exec.c (exec_file_attach): Close scratch_chan on failure.
* procfs.c (load_syscalls): Make a cleanup.
(open_procinfo_files): fd==0 is ok.
* nto-procfs.c (procfs_open): Make a cleanup.
(procfs_pidlist): Likewise.
(do_closedir_cleanup): New function.
diff --git a/gdb/exec.c b/gdb/exec.c
index 3b9ecfa..92450e6 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -217,8 +217,11 @@ exec_file_attach (char *filename, int from_tty)
scratch_chan);
if (!exec_bfd)
- error (_("\"%s\": could not open as an executable file: %s"),
- scratch_pathname, bfd_errmsg (bfd_get_error ()));
+ {
+ close (scratch_chan);
+ error (_("\"%s\": could not open as an executable file: %s"),
+ scratch_pathname, bfd_errmsg (bfd_get_error ()));
+ }
/* At this point, scratch_pathname and exec_bfd->name both point to the
same malloc'd string. However exec_close() will attempt to free it
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 7450edc..f69aaf2 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -125,6 +125,7 @@ procfs_open (char *arg, int from_tty)
char buffer[50];
int fd, total_size;
procfs_sysinfo *sysinfo;
+ struct cleanup *cleanups;
nto_is_nto_target = procfs_is_nto_target;
@@ -169,13 +170,13 @@ procfs_open (char *arg, int from_tty)
safe_strerror (errno));
error (_("Invalid procfs arg"));
}
+ cleanups = make_cleanup_close (fd);
sysinfo = (void *) buffer;
if (devctl (fd, DCMD_PROC_SYSINFO, sysinfo, sizeof buffer, 0) != EOK)
{
printf_filtered ("Error getting size: %d (%s)\n", errno,
safe_strerror (errno));
- close (fd);
error (_("Devctl failed."));
}
else
@@ -186,7 +187,6 @@ procfs_open (char *arg, int from_tty)
{
printf_filtered ("Memory error: %d (%s)\n", errno,
safe_strerror (errno));
- close (fd);
error (_("alloca failed."));
}
else
@@ -195,7 +195,6 @@ procfs_open (char *arg, int from_tty)
{
printf_filtered ("Error getting sysinfo: %d (%s)\n", errno,
safe_strerror (errno));
- close (fd);
error (_("Devctl failed."));
}
else
@@ -203,14 +202,11 @@ procfs_open (char *arg, int from_tty)
if (sysinfo->type !=
nto_map_arch_to_cputype (gdbarch_bfd_arch_info
(current_gdbarch)->arch_name))
- {
- close (fd);
- error (_("Invalid target CPU."));
- }
+ error (_("Invalid target CPU."));
}
}
}
- close (fd);
+ do_cleanups (cleanups);
printf_filtered ("Debugging using %s\n", nto_procfs_path);
}
@@ -259,12 +255,17 @@ procfs_find_new_threads (void)
return;
}
+static void
+do_closedir_cleanup (void *dir)
+{
+ closedir (dir);
+}
+
void
procfs_pidlist (char *args, int from_tty)
{
DIR *dp = NULL;
struct dirent *dirp = NULL;
- int fd = -1;
char buf[512];
procfs_info *pidinfo = NULL;
procfs_debuginfo *info = NULL;
@@ -272,6 +273,7 @@ procfs_pidlist (char *args, int from_tty)
pid_t num_threads = 0;
pid_t pid;
char name[512];
+ struct cleanup *cleanups;
dp = opendir (nto_procfs_path);
if (dp == NULL)
@@ -281,18 +283,23 @@ procfs_pidlist (char *args, int from_tty)
return;
}
+ cleanups = make_cleanup (do_closedir_cleanup, dp);
+
/* Start scan at first pid. */
rewinddir (dp);
do
{
+ int fd;
+ struct cleanup *inner_cleanup;
+
/* Get the right pid and procfs path for the pid. */
do
{
dirp = readdir (dp);
if (dirp == NULL)
{
- closedir (dp);
+ do_cleanups (cleanups);
return;
}
snprintf (buf, 511, "%s/%s/as", nto_procfs_path, dirp->d_name);
@@ -306,9 +313,10 @@ procfs_pidlist (char *args, int from_tty)
{
fprintf_unfiltered (gdb_stderr, "failed to open %s - %d (%s)\n",
buf, errno, safe_strerror (errno));
- closedir (dp);
+ do_cleanups (cleanups);
return;
}
+ inner_cleanup = make_cleanup_close (fd);
pidinfo = (procfs_info *) buf;
if (devctl (fd, DCMD_PROC_INFO, pidinfo, sizeof (buf), 0) != EOK)
@@ -336,12 +344,12 @@ procfs_pidlist (char *args, int from_tty)
if (status->tid != 0)
printf_filtered ("%s - %d/%d\n", name, pid, status->tid);
}
- close (fd);
+
+ do_cleanups (inner_cleanup);
}
while (dirp != NULL);
- close (fd);
- closedir (dp);
+ do_cleanups (cleanups);
return;
}
diff --git a/gdb/source.c b/gdb/source.c
index 3ef557c..c41c193 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1064,7 +1064,7 @@ symtab_to_fullname (struct symtab *s)
r = find_and_open_source (s->objfile, s->filename, s->dirname,
&s->fullname);
- if (r)
+ if (r >= 0)
{
close (r);
return s->fullname;
@@ -1093,7 +1093,7 @@ psymtab_to_fullname (struct partial_symtab *ps)
r = find_and_open_source (ps->objfile, ps->filename, ps->dirname,
&ps->fullname);
- if (r)
+ if (r >= 0)
{
close (r);
return ps->fullname;
@@ -1251,6 +1251,7 @@ static int
get_filename_and_charpos (struct symtab *s, char **fullname)
{
int desc, linenums_changed = 0;
+ struct cleanup *cleanups;
desc = open_source_file (s);
if (desc < 0)
@@ -1259,13 +1260,14 @@ get_filename_and_charpos (struct symtab *s, char **fullname)
*fullname = NULL;
return 0;
}
+ cleanups = make_cleanup_close (desc);
if (fullname)
*fullname = s->fullname;
if (s->line_charpos == 0)
linenums_changed = 1;
if (linenums_changed)
find_source_lines (s, desc);
- close (desc);
+ do_cleanups (cleanups);
return linenums_changed;
}
@@ -1540,6 +1542,7 @@ forward_search_command (char *regex, int from_tty)
FILE *stream;
int line;
char *msg;
+ struct cleanup *cleanups;
line = last_line_listed + 1;
@@ -1553,24 +1556,21 @@ forward_search_command (char *regex, int from_tty)
desc = open_source_file (current_source_symtab);
if (desc < 0)
perror_with_name (current_source_symtab->filename);
+ cleanups = make_cleanup_close (desc);
if (current_source_symtab->line_charpos == 0)
find_source_lines (current_source_symtab, desc);
if (line < 1 || line > current_source_symtab->nlines)
- {
- close (desc);
- error (_("Expression not found"));
- }
+ error (_("Expression not found"));
if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 0)
- {
- close (desc);
- perror_with_name (current_source_symtab->filename);
- }
+ perror_with_name (current_source_symtab->filename);
+ discard_cleanups (cleanups);
stream = fdopen (desc, FDOPEN_MODE);
clearerr (stream);
+ cleanups = make_cleanup_fclose (stream);
while (1)
{
static char *buf = NULL;
@@ -1622,7 +1622,7 @@ forward_search_command (char *regex, int from_tty)
}
printf_filtered (_("Expression not found\n"));
- fclose (stream);
+ do_cleanups (cleanups);
}
static void
@@ -1633,6 +1633,7 @@ reverse_search_command (char *regex, int from_tty)
FILE *stream;
int line;
char *msg;
+ struct cleanup *cleanups;
line = last_line_listed - 1;
@@ -1646,24 +1647,21 @@ reverse_search_command (char *regex, int from_tty)
desc = open_source_file (current_source_symtab);
if (desc < 0)
perror_with_name (current_source_symtab->filename);
+ cleanups = make_cleanup_close (desc);
if (current_source_symtab->line_charpos == 0)
find_source_lines (current_source_symtab, desc);
if (line < 1 || line > current_source_symtab->nlines)
- {
- close (desc);
- error (_("Expression not found"));
- }
+ error (_("Expression not found"));
if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 0)
- {
- close (desc);
- perror_with_name (current_source_symtab->filename);
- }
+ perror_with_name (current_source_symtab->filename);
+ discard_cleanups (cleanups);
stream = fdopen (desc, FDOPEN_MODE);
clearerr (stream);
+ cleanups = make_cleanup_fclose (stream);
while (line > 1)
{
/* FIXME!!! We walk right off the end of buf if we get a long line!!! */
@@ -1709,7 +1707,7 @@ reverse_search_command (char *regex, int from_tty)
}
printf_filtered (_("Expression not found\n"));
- fclose (stream);
+ do_cleanups (cleanups);
return;
}
next reply other threads:[~2008-10-28 20:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-28 20:21 Tom Tromey [this message]
2008-10-29 21:02 ` Pedro Alves
2008-10-30 21:53 ` Tom Tromey
2008-10-30 21:32 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m3fxmgmqx0.fsf@fleche.redhat.com \
--to=tromey@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox