This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFA: open-related cleanup handling


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;
 }
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]