[RFA] Remove do_closedir_cleanup

Message ID 20180504165009.12758-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey May 4, 2018, 4:50 p.m. UTC
  This removes both copies of do_closedir_cleanup in favor of a new
unique_ptr specialization.

Tested by the buildbot, though I'm not sure that these code paths are
exercised there.

ChangeLog
2018-05-04  Tom Tromey  <tom@tromey.com>

	* nto-procfs.c (do_closedir_cleanup): Remove.
	(procfs_pidlist): Use gdb_dir_up.
	* procfs.c (do_closedir_cleanup): Remove.
	(proc_update_threads): Use gdb_dir_up.
	* common/filestuff.h (struct gdb_dir_deleter): New.
	(gdb_dir_up): New typedef.
---
 gdb/ChangeLog          |  9 +++++++++
 gdb/common/filestuff.h | 14 ++++++++++++++
 gdb/nto-procfs.c       | 24 ++++--------------------
 gdb/procfs.c           | 16 ++++------------
 4 files changed, 31 insertions(+), 32 deletions(-)
  

Comments

Pedro Alves May 4, 2018, 5:56 p.m. UTC | #1
On 05/04/2018 05:50 PM, Tom Tromey wrote:
> This removes both copies of do_closedir_cleanup in favor of a new
> unique_ptr specialization.
> 
> Tested by the buildbot, though I'm not sure that these code paths are
> exercised there.

They're not -- we don't have Solaris or QNX build slaves.

I noticed a typo, but otherwise it looks fine to me on a quick
glance.

>  static int
>  proc_update_threads (procinfo *pi)
>  {
>    char pathname[MAX_PROC_NAME_SIZE + 16];
>    struct dirent *direntry;
> -  struct cleanup *old_chain = NULL;
>    procinfo *thread;
> -  DIR *dirp;
> +  gdb_dir_up dirp;
>    int lwpid;
>  
>    /* We should never have to apply this operation to any procinfo
> @@ -1756,11 +1749,11 @@ proc_update_threads (procinfo *pi)
>  
>    strcpy (pathname, pi->pathname);
>    strcat (pathname, "/lwp");
> -  if ((dirp = opendir (pathname)) == NULL)
> +  dir.reset (opendir (pathname));

Typo: dir -> dirp.

> +  if (dirp == NULL)
>      proc_error (pi, "update_threads, opendir", __LINE__);
>  
Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index 92a2a5f4c7..0e46eb5da0 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -19,6 +19,8 @@ 
 #ifndef FILESTUFF_H
 #define FILESTUFF_H
 
+#include <dirent.h>
+
 /* Note all the file descriptors which are open when this is called.
    These file descriptors will not be closed by close_most_fds.  */
 
@@ -84,4 +86,16 @@  extern int gdb_pipe_cloexec (int filedes[2]);
 
 extern struct cleanup *make_cleanup_close (int fd);
 
+struct gdb_dir_deleter
+{
+  void operator() (DIR *dir) const
+  {
+    closedir (dir);
+  }
+};
+
+/* A unique pointer to a DIR.  */
+
+typedef std::unique_ptr<DIR, gdb_dir_deleter> gdb_dir_up;
+
 #endif /* FILESTUFF_H */
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 51559e6c3e..f0ef9b9eb4 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -424,15 +424,8 @@  nto_procfs_target::update_thread_list ()
 }
 
 static void
-do_closedir_cleanup (void *dir)
-{
-  closedir (dir);
-}
-
-static void
 procfs_pidlist (const char *args, int from_tty)
 {
-  DIR *dp = NULL;
   struct dirent *dirp = NULL;
   char buf[PATH_MAX];
   procfs_info *pidinfo = NULL;
@@ -441,13 +434,12 @@  procfs_pidlist (const char *args, int from_tty)
   pid_t num_threads = 0;
   pid_t pid;
   char name[512];
-  struct cleanup *cleanups;
   char procfs_dir[PATH_MAX];
 
   snprintf (procfs_dir, sizeof (procfs_dir), "%s%s",
 	    (nodestr != NULL) ? nodestr : "", "/proc");
 
-  dp = opendir (procfs_dir);
+  gdb_dir_up dp (opendir (procfs_dir));
   if (dp == NULL)
     {
       fprintf_unfiltered (gdb_stderr, "failed to opendir \"%s\" - %d (%s)",
@@ -455,22 +447,17 @@  procfs_pidlist (const char *args, int from_tty)
       return;
     }
 
-  cleanups = make_cleanup (do_closedir_cleanup, dp);
-
   /* Start scan at first pid.  */
-  rewinddir (dp);
+  rewinddir (dp.get ());
 
   do
     {
       /* Get the right pid and procfs path for the pid.  */
       do
 	{
-	  dirp = readdir (dp);
+	  dirp = readdir (dp.get ());
 	  if (dirp == NULL)
-	    {
-	      do_cleanups (cleanups);
-	      return;
-	    }
+	    return;
 	  snprintf (buf, sizeof (buf), "%s%s/%s/as",
 		    (nodestr != NULL) ? nodestr : "",
 		    "/proc", dirp->d_name);
@@ -521,9 +508,6 @@  procfs_pidlist (const char *args, int from_tty)
 	}
     }
   while (dirp != NULL);
-
-  do_cleanups (cleanups);
-  return;
 }
 
 static void
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 749b2b4833..a6e06a56ee 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1722,20 +1722,13 @@  proc_delete_dead_threads (procinfo *parent, procinfo *thread, void *ignore)
   return 0;	/* keep iterating */
 }
 
-static void
-do_closedir_cleanup (void *dir)
-{
-  closedir ((DIR *) dir);
-}
-
 static int
 proc_update_threads (procinfo *pi)
 {
   char pathname[MAX_PROC_NAME_SIZE + 16];
   struct dirent *direntry;
-  struct cleanup *old_chain = NULL;
   procinfo *thread;
-  DIR *dirp;
+  gdb_dir_up dirp;
   int lwpid;
 
   /* We should never have to apply this operation to any procinfo
@@ -1756,11 +1749,11 @@  proc_update_threads (procinfo *pi)
 
   strcpy (pathname, pi->pathname);
   strcat (pathname, "/lwp");
-  if ((dirp = opendir (pathname)) == NULL)
+  dir.reset (opendir (pathname));
+  if (dirp == NULL)
     proc_error (pi, "update_threads, opendir", __LINE__);
 
-  old_chain = make_cleanup (do_closedir_cleanup, dirp);
-  while ((direntry = readdir (dirp)) != NULL)
+  while ((direntry = readdir (dirp.get ())) != NULL)
     if (direntry->d_name[0] != '.')		/* skip '.' and '..' */
       {
 	lwpid = atoi (&direntry->d_name[0]);
@@ -1768,7 +1761,6 @@  proc_update_threads (procinfo *pi)
 	  proc_error (pi, "update_threads, create_procinfo", __LINE__);
       }
   pi->threads_valid = 1;
-  do_cleanups (old_chain);
   return 1;
 }