[RFA,v2,09/24] Remove close cleanup

Message ID 20170725172107.9799-10-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey July 25, 2017, 5:20 p.m. UTC
  make_cleanup_close calls close on a file descriptor.  This patch
replaces it with a new RAII class, gdb::fd_closer.

ChangeLog
2017-07-25  Tom Tromey  <tom@tromey.com>

	* source.c (get_filename_and_charpos, forward_search_command)
	(reverse_search_command): Use fd_closer.
	* procfs.c (load_syscalls, proc_get_LDT_entry)
	(iterate_over_mappings): Use fd_closer.
	* nto-procfs.c (procfs_open_1, procfs_pidlist): Use fd_closer.
	* nat/linux-namespaces.c (linux_mntns_access_fs): Use fd_closer.
	(close_saving_errno): New function.
	* common/filestuff.h (gdb::fd_closer): New class.
	* common/filestuff.c (do_close_cleanup, make_cleanup_close):
	Remove.
---
 gdb/ChangeLog              | 13 +++++++++++++
 gdb/common/filestuff.c     | 21 ---------------------
 gdb/common/filestuff.h     | 41 +++++++++++++++++++++++++++++++++++++++--
 gdb/nat/linux-namespaces.c | 43 ++++++++++++++++++-------------------------
 gdb/nto-procfs.c           |  9 ++-------
 gdb/procfs.c               | 26 +++++++++-----------------
 gdb/source.c               | 13 ++++++-------
 7 files changed, 87 insertions(+), 79 deletions(-)
  

Comments

Simon Marchi July 31, 2017, 7:09 p.m. UTC | #1
On 2017-07-25 19:20, Tom Tromey wrote:
> make_cleanup_close calls close on a file descriptor.  This patch
> replaces it with a new RAII class, gdb::fd_closer.
> 
> ChangeLog
> 2017-07-25  Tom Tromey  <tom@tromey.com>
> 
> 	* source.c (get_filename_and_charpos, forward_search_command)
> 	(reverse_search_command): Use fd_closer.
> 	* procfs.c (load_syscalls, proc_get_LDT_entry)
> 	(iterate_over_mappings): Use fd_closer.
> 	* nto-procfs.c (procfs_open_1, procfs_pidlist): Use fd_closer.
> 	* nat/linux-namespaces.c (linux_mntns_access_fs): Use fd_closer.
> 	(close_saving_errno): New function.
> 	* common/filestuff.h (gdb::fd_closer): New class.
> 	* common/filestuff.c (do_close_cleanup, make_cleanup_close):
> 	Remove.
> ---
>  gdb/ChangeLog              | 13 +++++++++++++
>  gdb/common/filestuff.c     | 21 ---------------------
>  gdb/common/filestuff.h     | 41 
> +++++++++++++++++++++++++++++++++++++++--
>  gdb/nat/linux-namespaces.c | 43 
> ++++++++++++++++++-------------------------
>  gdb/nto-procfs.c           |  9 ++-------
>  gdb/procfs.c               | 26 +++++++++-----------------
>  gdb/source.c               | 13 ++++++-------
>  7 files changed, 87 insertions(+), 79 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 2bf549b..871b1f0 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,18 @@
>  2017-07-25  Tom Tromey  <tom@tromey.com>
> 
> +	* source.c (get_filename_and_charpos, forward_search_command)
> +	(reverse_search_command): Use fd_closer.
> +	* procfs.c (load_syscalls, proc_get_LDT_entry)
> +	(iterate_over_mappings): Use fd_closer.
> +	* nto-procfs.c (procfs_open_1, procfs_pidlist): Use fd_closer.
> +	* nat/linux-namespaces.c (linux_mntns_access_fs): Use fd_closer.
> +	(close_saving_errno): New function.
> +	* common/filestuff.h (gdb::fd_closer): New class.
> +	* common/filestuff.c (do_close_cleanup, make_cleanup_close):
> +	Remove.
> +
> +2017-07-25  Tom Tromey  <tom@tromey.com>
> +
>  	* compile/compile.c (cleanup_unlink_file): Remove.
>  	(compile_to_object): Use gdb::unlinker.
>  	(eval_compile_command): Likewise.
> diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
> index 4b05884..5a0dd65 100644
> --- a/gdb/common/filestuff.c
> +++ b/gdb/common/filestuff.c
> @@ -404,24 +404,3 @@ gdb_pipe_cloexec (int filedes[2])
> 
>    return result;
>  }
> -
> -/* Helper function which does the work for make_cleanup_close.  */
> -
> -static void
> -do_close_cleanup (void *arg)
> -{
> -  int *fd = (int *) arg;
> -
> -  close (*fd);
> -}
> -
> -/* See filestuff.h.  */
> -
> -struct cleanup *
> -make_cleanup_close (int fd)
> -{
> -  int *saved_fd = XNEW (int);
> -
> -  *saved_fd = fd;
> -  return make_cleanup_dtor (do_close_cleanup, saved_fd, xfree);
> -}
> diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
> index 3cf2df6..ad34f27 100644
> --- a/gdb/common/filestuff.h
> +++ b/gdb/common/filestuff.h
> @@ -80,8 +80,45 @@ extern int gdb_socket_cloexec (int domain, int
> style, int protocol);
> 
>  extern int gdb_pipe_cloexec (int filedes[2]);
> 
> -/* Return a new cleanup that closes FD.  */
> +namespace gdb
> +{
> +
> +/* A class that manages a file descriptor and closes it on
> +   destruction.  This can be parameterized to use a different close
> +   function; the default is "close".  */
> +
> +template<int (*CLOSER) (int) = close>
> +class fd_closer
> +{
> +public:
> +
> +  explicit fd_closer (int fd)
> +    : m_fd (fd)
> +  {
> +  }
> +
> +  ~fd_closer ()
> +  {
> +    if (m_fd != -1)
> +      CLOSER (m_fd);
> +  }
> +
> +  fd_closer (const fd_closer &) = delete;
> +  fd_closer &operator= (const fd_closer &) = delete;
> +
> +  /* Release the file descriptor to the caller.  */
> +  int release ()
> +  {
> +    int result = m_fd;
> +    m_fd = -1;
> +    return result;
> +  }
> +
> +private:
> +
> +  int m_fd;
> +};
> 
> -extern struct cleanup *make_cleanup_close (int fd);
> +}
> 
>  #endif /* FILESTUFF_H */
> diff --git a/gdb/nat/linux-namespaces.c b/gdb/nat/linux-namespaces.c
> index 1df8f1b..fba4199 100644
> --- a/gdb/nat/linux-namespaces.c
> +++ b/gdb/nat/linux-namespaces.c
> @@ -20,6 +20,7 @@
>  #include "common-defs.h"
>  #include "nat/linux-namespaces.h"
>  #include "filestuff.h"
> +#include "scoped_restore.h"
>  #include <fcntl.h>
>  #include <sys/syscall.h>
>  #include <sys/types.h>
> @@ -881,13 +882,21 @@ enum mnsh_fs_code
>      MNSH_FS_HELPER
>    };
> 
> +/* A function like "close" that saves and restores errno.  */
> +
> +static int
> +close_saving_errno (int fd)
> +{
> +  scoped_restore save_errno = make_scoped_restore (&errno);
> +  return close (fd);
> +}
> +
>  /* Return a value indicating how the caller should access the
>     mount namespace of process PID.  */
> 
>  static enum mnsh_fs_code
>  linux_mntns_access_fs (pid_t pid)
>  {
> -  struct cleanup *old_chain;
>    struct linux_ns *ns;
>    struct stat sb;
>    struct linux_mnsh *helper;
> @@ -901,27 +910,21 @@ linux_mntns_access_fs (pid_t pid)
>    if (ns == NULL)
>      return MNSH_FS_DIRECT;
> 
> -  old_chain = make_cleanup (null_cleanup, NULL);
> -
>    fd = gdb_open_cloexec (linux_ns_filename (ns, pid), O_RDONLY, 0);
>    if (fd < 0)
> -    goto error;
> +    return MNSH_FS_ERROR;
> 
> -  make_cleanup_close (fd);
> +  gdb::fd_closer<close_saving_errno> close_fd (fd);
> 
>    if (fstat (fd, &sb) != 0)
> -    goto error;
> +    return MNSH_FS_ERROR;
> 
>    if (sb.st_ino == ns->id)
> -    {
> -      do_cleanups (old_chain);
> -
> -      return MNSH_FS_DIRECT;
> -    }
> +    return MNSH_FS_DIRECT;
> 
>    helper = linux_mntns_get_helper ();
>    if (helper == NULL)
> -    goto error;
> +    return MNSH_FS_ERROR;
> 
>    if (sb.st_ino != helper->nsid)
>      {
> @@ -929,10 +932,10 @@ linux_mntns_access_fs (pid_t pid)
> 
>        size = mnsh_send_setns (helper, fd, 0);
>        if (size < 0)
> -	goto error;
> +	return MNSH_FS_ERROR;
> 
>        if (mnsh_recv_int (helper, &result, &error) != 0)
> -	goto error;
> +	return MNSH_FS_ERROR;
> 
>        if (result != 0)
>  	{
> @@ -945,23 +948,13 @@ linux_mntns_access_fs (pid_t pid)
>  	    error = ENOTSUP;
> 
>  	  errno = error;
> -	  goto error;
> +	  return MNSH_FS_ERROR;
>  	}
> 
>        helper->nsid = sb.st_ino;
>      }
> 
> -  do_cleanups (old_chain);
> -
>    return MNSH_FS_HELPER;
> -
> -error:
> -  saved_errno = errno;
> -
> -  do_cleanups (old_chain);
> -
> -  errno = saved_errno;
> -  return MNSH_FS_ERROR;
>  }
> 
>  /* See nat/linux-namespaces.h.  */
> diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
> index 7fb7095..63ef2ec 100644
> --- a/gdb/nto-procfs.c
> +++ b/gdb/nto-procfs.c
> @@ -115,7 +115,6 @@ procfs_open_1 (struct target_ops *ops, const char
> *arg, int from_tty)
>    char buffer[50];
>    int fd, total_size;
>    procfs_sysinfo *sysinfo;
> -  struct cleanup *cleanups;
>    char nto_procfs_path[PATH_MAX];
> 
>    /* Offer to kill previous inferiors before opening this target.  */
> @@ -165,7 +164,7 @@ procfs_open_1 (struct target_ops *ops, const char
> *arg, int from_tty)
>  		       safe_strerror (errno));
>        error (_("Invalid procfs arg"));
>      }
> -  cleanups = make_cleanup_close (fd);
> +  gdb::fd_closer close_fd (fd);
> 
>    sysinfo = (void *) buffer;
>    if (devctl (fd, DCMD_PROC_SYSINFO, sysinfo, sizeof buffer, 0) != 
> EOK)
> @@ -201,7 +200,6 @@ procfs_open_1 (struct target_ops *ops, const char
> *arg, int from_tty)
>  	    }
>  	}
>      }
> -  do_cleanups (cleanups);
> 
>    inf_child_open_target (ops, arg, from_tty);
>    printf_filtered ("Debugging using %s\n", nto_procfs_path);
> @@ -392,7 +390,6 @@ procfs_pidlist (char *args, int from_tty)
>    do
>      {
>        int fd;
> -      struct cleanup *inner_cleanup;
> 
>        /* Get the right pid and procfs path for the pid.  */
>        do
> @@ -418,7 +415,7 @@ procfs_pidlist (char *args, int from_tty)
>  			      buf, errno, safe_strerror (errno));
>  	  continue;
>  	}
> -      inner_cleanup = make_cleanup_close (fd);
> +      gdb::fd_closer close_fd (fd);
> 
>        pidinfo = (procfs_info *) buf;
>        if (devctl (fd, DCMD_PROC_INFO, pidinfo, sizeof (buf), 0) != 
> EOK)
> @@ -451,8 +448,6 @@ procfs_pidlist (char *args, int from_tty)
>  	      break;
>  	    }
>  	}
> -
> -      do_cleanups (inner_cleanup);
>      }
>    while (dirp != NULL);
> 
> diff --git a/gdb/procfs.c b/gdb/procfs.c
> index b03809c..a0d2270 100644
> --- a/gdb/procfs.c
> +++ b/gdb/procfs.c
> @@ -31,6 +31,7 @@
>  #include "regcache.h"
>  #include "inf-child.h"
>  #include "filestuff.h"
> +#include "common/filestuff.h"
> 
>  #if defined (NEW_PROC_API)
>  #define _STRUCTURED_PROC 1	/* Should be done by configure script.  */
> @@ -883,7 +884,8 @@ load_syscalls (procinfo *pi)
>      {
>        error (_("load_syscalls: Can't open /proc/%d/sysent"), pi->pid);
>      }
> -  cleanups = make_cleanup_close (sysent_fd);
> +
> +  gdb::fd_closer close_sysent_fd (sysent_fd);
> 
>    size = sizeof header - sizeof (prsyscall_t);
>    if (read (sysent_fd, &header, size) != size)
> @@ -899,7 +901,7 @@ load_syscalls (procinfo *pi)
> 
>    size = header.pr_nsyscalls * sizeof (prsyscall_t);
>    syscalls = xmalloc (size);
> -  make_cleanup (free_current_contents, &syscalls);
> +  cleanups = make_cleanup (free_current_contents, &syscalls);
> 
>    if (read (sysent_fd, syscalls, size) != size)
>      error (_("load_syscalls: Error reading /proc/%d/sysent"), 
> pi->pid);
> @@ -2484,7 +2486,6 @@ proc_get_LDT_entry (procinfo *pi, int key)
>    static struct ssd *ldt_entry = NULL;
>  #ifdef NEW_PROC_API
>    char pathname[MAX_PROC_NAME_SIZE];
> -  struct cleanup *old_chain = NULL;
>    int  fd;
> 
>    /* Allocate space for one LDT entry.
> @@ -2499,8 +2500,8 @@ proc_get_LDT_entry (procinfo *pi, int key)
>        proc_warn (pi, "proc_get_LDT_entry (open)", __LINE__);
>        return NULL;
>      }
> -  /* Make sure it gets closed again!  */
> -  old_chain = make_cleanup_close (fd);
> +
> +  gdb::fd_closer close_fd (fd);
> 
>    /* Now 'read' thru the table, find a match and return it.  */
>    while (read (fd, ldt_entry, sizeof (struct ssd)) == sizeof (struct 
> ssd))
> @@ -2512,13 +2513,9 @@ proc_get_LDT_entry (procinfo *pi, int key)
>  	break;	/* end of table */
>        /* If key matches, return this entry.  */
>        if (ldt_entry->sel == key)
> -	{
> -	  do_cleanups (old_chain);
> -	  return ldt_entry;
> -	}
> +	return ldt_entry;
>      }
>    /* Loop ended, match not found.  */
> -  do_cleanups (old_chain);
>    return NULL;
>  #else
>    int nldt, i;
> @@ -4917,7 +4914,6 @@ iterate_over_mappings (procinfo *pi,
> find_memory_region_ftype child_func,
>    int funcstat;
>    int map_fd;
>    int nmap;
> -  struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
>  #ifdef NEW_PROC_API
>    struct stat sbuf;
>  #endif
> @@ -4931,7 +4927,7 @@ iterate_over_mappings (procinfo *pi,
> find_memory_region_ftype child_func,
>      proc_error (pi, "iterate_over_mappings (open)", __LINE__);
> 
>    /* Make sure it gets closed again.  */
> -  make_cleanup_close (map_fd);
> +  gdb::fd_closer close_map_fd (map_fd);
> 
>    /* Use stat to determine the file size, and compute
>       the number of prmap_t objects it contains.  */
> @@ -4955,12 +4951,8 @@ iterate_over_mappings (procinfo *pi,
> find_memory_region_ftype child_func,
> 
>    for (prmap = prmaps; nmap > 0; prmap++, nmap--)
>      if ((funcstat = (*func) (prmap, child_func, data)) != 0)
> -      {
> -	do_cleanups (cleanups);
> -        return funcstat;
> -      }
> +      return funcstat;
> 
> -  do_cleanups (cleanups);
>    return 0;
>  }
> 
> diff --git a/gdb/source.c b/gdb/source.c
> index 4cc862c..510f1c9 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1302,14 +1302,14 @@ get_filename_and_charpos (struct symtab *s,
> char **fullname)
>  	*fullname = NULL;
>        return 0;
>      }
> -  cleanups = make_cleanup_close (desc);
> +
> +  gdb::fd_closer<> close_desc (desc);
>    if (fullname)
>      *fullname = s->fullname;
>    if (s->line_charpos == 0)
>      linenums_changed = 1;
>    if (linenums_changed)
>      find_source_lines (s, desc);
> -  do_cleanups (cleanups);
>    return linenums_changed;
>  }
> 
> @@ -1627,7 +1627,6 @@ forward_search_command (char *regex, int 
> from_tty)
>    int desc;
>    int line;
>    char *msg;
> -  struct cleanup *cleanups;
> 
>    line = last_line_listed + 1;
> 
> @@ -1641,7 +1640,7 @@ forward_search_command (char *regex, int 
> from_tty)
>    desc = open_source_file (current_source_symtab);
>    if (desc < 0)
>      perror_with_name (symtab_to_filename_for_display 
> (current_source_symtab));
> -  cleanups = make_cleanup_close (desc);
> +  gdb::fd_closer<> close_desc (desc);
> 
>    if (current_source_symtab->line_charpos == 0)
>      find_source_lines (current_source_symtab, desc);
> @@ -1652,7 +1651,7 @@ forward_search_command (char *regex, int 
> from_tty)
>    if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 
> 0)
>      perror_with_name (symtab_to_filename_for_display 
> (current_source_symtab));
> 
> -  discard_cleanups (cleanups);
> +  close_desc.release ();
>    gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
>    clearerr (stream.get ());
>    while (1)
> @@ -1726,7 +1725,7 @@ reverse_search_command (char *regex, int 
> from_tty)
>    desc = open_source_file (current_source_symtab);
>    if (desc < 0)
>      perror_with_name (symtab_to_filename_for_display 
> (current_source_symtab));
> -  cleanups = make_cleanup_close (desc);
> +  gdb::fd_closer<> close_desc (desc);
> 
>    if (current_source_symtab->line_charpos == 0)
>      find_source_lines (current_source_symtab, desc);
> @@ -1737,7 +1736,7 @@ reverse_search_command (char *regex, int 
> from_tty)
>    if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 
> 0)
>      perror_with_name (symtab_to_filename_for_display 
> (current_source_symtab));
> 
> -  discard_cleanups (cleanups);
> +  close_desc.release ();
>    gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
>    clearerr (stream.get ());
>    while (line > 1)

See response here: 
https://sourceware.org/ml/gdb-patches/2017-07/msg00451.html
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2bf549b..871b1f0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@ 
 2017-07-25  Tom Tromey  <tom@tromey.com>
 
+	* source.c (get_filename_and_charpos, forward_search_command)
+	(reverse_search_command): Use fd_closer.
+	* procfs.c (load_syscalls, proc_get_LDT_entry)
+	(iterate_over_mappings): Use fd_closer.
+	* nto-procfs.c (procfs_open_1, procfs_pidlist): Use fd_closer.
+	* nat/linux-namespaces.c (linux_mntns_access_fs): Use fd_closer.
+	(close_saving_errno): New function.
+	* common/filestuff.h (gdb::fd_closer): New class.
+	* common/filestuff.c (do_close_cleanup, make_cleanup_close):
+	Remove.
+
+2017-07-25  Tom Tromey  <tom@tromey.com>
+
 	* compile/compile.c (cleanup_unlink_file): Remove.
 	(compile_to_object): Use gdb::unlinker.
 	(eval_compile_command): Likewise.
diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
index 4b05884..5a0dd65 100644
--- a/gdb/common/filestuff.c
+++ b/gdb/common/filestuff.c
@@ -404,24 +404,3 @@  gdb_pipe_cloexec (int filedes[2])
 
   return result;
 }
-
-/* Helper function which does the work for make_cleanup_close.  */
-
-static void
-do_close_cleanup (void *arg)
-{
-  int *fd = (int *) arg;
-
-  close (*fd);
-}
-
-/* See filestuff.h.  */
-
-struct cleanup *
-make_cleanup_close (int fd)
-{
-  int *saved_fd = XNEW (int);
-
-  *saved_fd = fd;
-  return make_cleanup_dtor (do_close_cleanup, saved_fd, xfree);
-}
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index 3cf2df6..ad34f27 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -80,8 +80,45 @@  extern int gdb_socket_cloexec (int domain, int style, int protocol);
 
 extern int gdb_pipe_cloexec (int filedes[2]);
 
-/* Return a new cleanup that closes FD.  */
+namespace gdb
+{
+
+/* A class that manages a file descriptor and closes it on
+   destruction.  This can be parameterized to use a different close
+   function; the default is "close".  */
+
+template<int (*CLOSER) (int) = close>
+class fd_closer
+{
+public:
+
+  explicit fd_closer (int fd)
+    : m_fd (fd)
+  {
+  }
+
+  ~fd_closer ()
+  {
+    if (m_fd != -1)
+      CLOSER (m_fd);
+  }
+
+  fd_closer (const fd_closer &) = delete;
+  fd_closer &operator= (const fd_closer &) = delete;
+
+  /* Release the file descriptor to the caller.  */
+  int release ()
+  {
+    int result = m_fd;
+    m_fd = -1;
+    return result;
+  }
+
+private:
+
+  int m_fd;
+};
 
-extern struct cleanup *make_cleanup_close (int fd);
+}
 
 #endif /* FILESTUFF_H */
diff --git a/gdb/nat/linux-namespaces.c b/gdb/nat/linux-namespaces.c
index 1df8f1b..fba4199 100644
--- a/gdb/nat/linux-namespaces.c
+++ b/gdb/nat/linux-namespaces.c
@@ -20,6 +20,7 @@ 
 #include "common-defs.h"
 #include "nat/linux-namespaces.h"
 #include "filestuff.h"
+#include "scoped_restore.h"
 #include <fcntl.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
@@ -881,13 +882,21 @@  enum mnsh_fs_code
     MNSH_FS_HELPER
   };
 
+/* A function like "close" that saves and restores errno.  */
+
+static int
+close_saving_errno (int fd)
+{
+  scoped_restore save_errno = make_scoped_restore (&errno);
+  return close (fd);
+}
+
 /* Return a value indicating how the caller should access the
    mount namespace of process PID.  */
 
 static enum mnsh_fs_code
 linux_mntns_access_fs (pid_t pid)
 {
-  struct cleanup *old_chain;
   struct linux_ns *ns;
   struct stat sb;
   struct linux_mnsh *helper;
@@ -901,27 +910,21 @@  linux_mntns_access_fs (pid_t pid)
   if (ns == NULL)
     return MNSH_FS_DIRECT;
 
-  old_chain = make_cleanup (null_cleanup, NULL);
-
   fd = gdb_open_cloexec (linux_ns_filename (ns, pid), O_RDONLY, 0);
   if (fd < 0)
-    goto error;
+    return MNSH_FS_ERROR;
 
-  make_cleanup_close (fd);
+  gdb::fd_closer<close_saving_errno> close_fd (fd);
 
   if (fstat (fd, &sb) != 0)
-    goto error;
+    return MNSH_FS_ERROR;
 
   if (sb.st_ino == ns->id)
-    {
-      do_cleanups (old_chain);
-
-      return MNSH_FS_DIRECT;
-    }
+    return MNSH_FS_DIRECT;
 
   helper = linux_mntns_get_helper ();
   if (helper == NULL)
-    goto error;
+    return MNSH_FS_ERROR;
 
   if (sb.st_ino != helper->nsid)
     {
@@ -929,10 +932,10 @@  linux_mntns_access_fs (pid_t pid)
 
       size = mnsh_send_setns (helper, fd, 0);
       if (size < 0)
-	goto error;
+	return MNSH_FS_ERROR;
 
       if (mnsh_recv_int (helper, &result, &error) != 0)
-	goto error;
+	return MNSH_FS_ERROR;
 
       if (result != 0)
 	{
@@ -945,23 +948,13 @@  linux_mntns_access_fs (pid_t pid)
 	    error = ENOTSUP;
 
 	  errno = error;
-	  goto error;
+	  return MNSH_FS_ERROR;
 	}
 
       helper->nsid = sb.st_ino;
     }
 
-  do_cleanups (old_chain);
-
   return MNSH_FS_HELPER;
-
-error:
-  saved_errno = errno;
-
-  do_cleanups (old_chain);
-
-  errno = saved_errno;
-  return MNSH_FS_ERROR;
 }
 
 /* See nat/linux-namespaces.h.  */
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 7fb7095..63ef2ec 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -115,7 +115,6 @@  procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
   char buffer[50];
   int fd, total_size;
   procfs_sysinfo *sysinfo;
-  struct cleanup *cleanups;
   char nto_procfs_path[PATH_MAX];
 
   /* Offer to kill previous inferiors before opening this target.  */
@@ -165,7 +164,7 @@  procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
 		       safe_strerror (errno));
       error (_("Invalid procfs arg"));
     }
-  cleanups = make_cleanup_close (fd);
+  gdb::fd_closer close_fd (fd);
 
   sysinfo = (void *) buffer;
   if (devctl (fd, DCMD_PROC_SYSINFO, sysinfo, sizeof buffer, 0) != EOK)
@@ -201,7 +200,6 @@  procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
 	    }
 	}
     }
-  do_cleanups (cleanups);
 
   inf_child_open_target (ops, arg, from_tty);
   printf_filtered ("Debugging using %s\n", nto_procfs_path);
@@ -392,7 +390,6 @@  procfs_pidlist (char *args, int from_tty)
   do
     {
       int fd;
-      struct cleanup *inner_cleanup;
 
       /* Get the right pid and procfs path for the pid.  */
       do
@@ -418,7 +415,7 @@  procfs_pidlist (char *args, int from_tty)
 			      buf, errno, safe_strerror (errno));
 	  continue;
 	}
-      inner_cleanup = make_cleanup_close (fd);
+      gdb::fd_closer close_fd (fd);
 
       pidinfo = (procfs_info *) buf;
       if (devctl (fd, DCMD_PROC_INFO, pidinfo, sizeof (buf), 0) != EOK)
@@ -451,8 +448,6 @@  procfs_pidlist (char *args, int from_tty)
 	      break;
 	    }
 	}
-
-      do_cleanups (inner_cleanup);
     }
   while (dirp != NULL);
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index b03809c..a0d2270 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -31,6 +31,7 @@ 
 #include "regcache.h"
 #include "inf-child.h"
 #include "filestuff.h"
+#include "common/filestuff.h"
 
 #if defined (NEW_PROC_API)
 #define _STRUCTURED_PROC 1	/* Should be done by configure script.  */
@@ -883,7 +884,8 @@  load_syscalls (procinfo *pi)
     {
       error (_("load_syscalls: Can't open /proc/%d/sysent"), pi->pid);
     }
-  cleanups = make_cleanup_close (sysent_fd);
+
+  gdb::fd_closer close_sysent_fd (sysent_fd);
 
   size = sizeof header - sizeof (prsyscall_t);
   if (read (sysent_fd, &header, size) != size)
@@ -899,7 +901,7 @@  load_syscalls (procinfo *pi)
 
   size = header.pr_nsyscalls * sizeof (prsyscall_t);
   syscalls = xmalloc (size);
-  make_cleanup (free_current_contents, &syscalls);
+  cleanups = make_cleanup (free_current_contents, &syscalls);
 
   if (read (sysent_fd, syscalls, size) != size)
     error (_("load_syscalls: Error reading /proc/%d/sysent"), pi->pid);
@@ -2484,7 +2486,6 @@  proc_get_LDT_entry (procinfo *pi, int key)
   static struct ssd *ldt_entry = NULL;
 #ifdef NEW_PROC_API
   char pathname[MAX_PROC_NAME_SIZE];
-  struct cleanup *old_chain = NULL;
   int  fd;
 
   /* Allocate space for one LDT entry.
@@ -2499,8 +2500,8 @@  proc_get_LDT_entry (procinfo *pi, int key)
       proc_warn (pi, "proc_get_LDT_entry (open)", __LINE__);
       return NULL;
     }
-  /* Make sure it gets closed again!  */
-  old_chain = make_cleanup_close (fd);
+
+  gdb::fd_closer close_fd (fd);
 
   /* Now 'read' thru the table, find a match and return it.  */
   while (read (fd, ldt_entry, sizeof (struct ssd)) == sizeof (struct ssd))
@@ -2512,13 +2513,9 @@  proc_get_LDT_entry (procinfo *pi, int key)
 	break;	/* end of table */
       /* If key matches, return this entry.  */
       if (ldt_entry->sel == key)
-	{
-	  do_cleanups (old_chain);
-	  return ldt_entry;
-	}
+	return ldt_entry;
     }
   /* Loop ended, match not found.  */
-  do_cleanups (old_chain);
   return NULL;
 #else
   int nldt, i;
@@ -4917,7 +4914,6 @@  iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
   int funcstat;
   int map_fd;
   int nmap;
-  struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
 #ifdef NEW_PROC_API
   struct stat sbuf;
 #endif
@@ -4931,7 +4927,7 @@  iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
     proc_error (pi, "iterate_over_mappings (open)", __LINE__);
 
   /* Make sure it gets closed again.  */
-  make_cleanup_close (map_fd);
+  gdb::fd_closer close_map_fd (map_fd);
 
   /* Use stat to determine the file size, and compute
      the number of prmap_t objects it contains.  */
@@ -4955,12 +4951,8 @@  iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
 
   for (prmap = prmaps; nmap > 0; prmap++, nmap--)
     if ((funcstat = (*func) (prmap, child_func, data)) != 0)
-      {
-	do_cleanups (cleanups);
-        return funcstat;
-      }
+      return funcstat;
 
-  do_cleanups (cleanups);
   return 0;
 }
 
diff --git a/gdb/source.c b/gdb/source.c
index 4cc862c..510f1c9 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1302,14 +1302,14 @@  get_filename_and_charpos (struct symtab *s, char **fullname)
 	*fullname = NULL;
       return 0;
     }
-  cleanups = make_cleanup_close (desc);
+
+  gdb::fd_closer<> close_desc (desc);
   if (fullname)
     *fullname = s->fullname;
   if (s->line_charpos == 0)
     linenums_changed = 1;
   if (linenums_changed)
     find_source_lines (s, desc);
-  do_cleanups (cleanups);
   return linenums_changed;
 }
 
@@ -1627,7 +1627,6 @@  forward_search_command (char *regex, int from_tty)
   int desc;
   int line;
   char *msg;
-  struct cleanup *cleanups;
 
   line = last_line_listed + 1;
 
@@ -1641,7 +1640,7 @@  forward_search_command (char *regex, int from_tty)
   desc = open_source_file (current_source_symtab);
   if (desc < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
-  cleanups = make_cleanup_close (desc);
+  gdb::fd_closer<> close_desc (desc);
 
   if (current_source_symtab->line_charpos == 0)
     find_source_lines (current_source_symtab, desc);
@@ -1652,7 +1651,7 @@  forward_search_command (char *regex, int from_tty)
   if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
 
-  discard_cleanups (cleanups);
+  close_desc.release ();
   gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
   clearerr (stream.get ());
   while (1)
@@ -1726,7 +1725,7 @@  reverse_search_command (char *regex, int from_tty)
   desc = open_source_file (current_source_symtab);
   if (desc < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
-  cleanups = make_cleanup_close (desc);
+  gdb::fd_closer<> close_desc (desc);
 
   if (current_source_symtab->line_charpos == 0)
     find_source_lines (current_source_symtab, desc);
@@ -1737,7 +1736,7 @@  reverse_search_command (char *regex, int from_tty)
   if (lseek (desc, current_source_symtab->line_charpos[line - 1], 0) < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
 
-  discard_cleanups (cleanups);
+  close_desc.release ();
   gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
   clearerr (stream.get ());
   while (line > 1)