[RFA 09/23] Remove close cleanup

Tom Tromey
2017-05-03T22:46:54Z

make_cleanup_close calls close on a file descriptor. This patch
replaces it with a new RAII class, gdb::fd_closer.

2017-05-02 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.
* common/filestuff.h (gdb::fd_closer): New class.
* common/filestuff.c (do_close_cleanup, make_cleanup_close):
Remove.
---
gdb/ChangeLog | 12 +++++++
gdb/common/filestuff.c | 21 ------------
gdb/common/filestuff.h | 39 ++++++++++++++++++++--
gdb/nat/linux-namespaces.c | 80 +++++++++++++++++++++-------------------------
gdb/nto-procfs.c | 9 ++----
gdb/procfs.c | 26 ++++++---------
gdb/source.c | 13 ++++----
7 files changed, 103 insertions(+), 97 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2d0d71d..50d8794 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
2017-05-02 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.
+ * common/filestuff.h (gdb::fd_closer): New class.
+ * common/filestuff.c (do_close_cleanup, make_cleanup_close):
+ Remove.
+
+2017-05-02 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..82eb5b3 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -80,8 +80,43 @@ 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. */
+
+class fd_closer
+{
+public:
+
+ explicit fd_closer (int fd)
+ : m_fd (fd)
+ {
+ }
+
+ ~fd_closer ()
+ {
+ if (m_fd != -1)
+ close (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..fcc7ba3 100644
--- a/gdb/nat/linux-namespaces.c
+++ b/gdb/nat/linux-namespaces.c
@@ -887,7 +887,6 @@ enum mnsh_fs_code
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,64 +900,59 @@ 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;
-
- make_cleanup_close (fd);
+ return MNSH_FS_ERROR;

- if (fstat (fd, &sb) != 0)
- goto error;
+ /* Introduce a new scope here so we can reset errno after
+ closing. */
+ {
+ gdb::fd_closer close_fd (fd);

- if (sb.st_ino == ns->id)
- {
- do_cleanups (old_chain);
+ if (fstat (fd, &sb) != 0)
+ goto error;

+ if (sb.st_ino == ns->id)
return MNSH_FS_DIRECT;
- }
-
- helper = linux_mntns_get_helper ();
- if (helper == NULL)
- goto error;

- if (sb.st_ino != helper->nsid)
- {
- int result, error;
+ helper = linux_mntns_get_helper ();
+ if (helper == NULL)
+ goto error;

- size = mnsh_send_setns (helper, fd, 0);
- if (size < 0)
- goto error;
+ if (sb.st_ino != helper->nsid)
+ {
+ int result, error;

- if (mnsh_recv_int (helper, &result, &error) != 0)
- goto error;
+ size = mnsh_send_setns (helper, fd, 0);
+ if (size < 0)
+ goto error;

- if (result != 0)
- {
- /* ENOSYS indicates that an entire function is unsupported
- (it's not appropriate for our versions of open/unlink/
- readlink to sometimes return with ENOSYS depending on how
- they're called) so we convert ENOSYS to ENOTSUP if setns
- fails. */
- if (error == ENOSYS)
- error = ENOTSUP;
-
- errno = error;
+ if (mnsh_recv_int (helper, &result, &error) != 0)
goto error;
- }

- helper->nsid = sb.st_ino;
- }
+ if (result != 0)
+ {
+ /* ENOSYS indicates that an entire function is unsupported
+ (it's not appropriate for our versions of open/unlink/
+ readlink to sometimes return with ENOSYS depending on how
+ they're called) so we convert ENOSYS to ENOTSUP if setns
+ fails. */
+ if (error == ENOSYS)
+ error = ENOTSUP;
+
+ errno = error;
+ goto error;
+ }

- do_cleanups (old_chain);
+ helper->nsid = sb.st_ino;
+ }

- return MNSH_FS_HELPER;
+ return MNSH_FS_HELPER;

-error:
- saved_errno = errno;
+ error:
+ saved_errno = errno;

- do_cleanups (old_chain);
+ }

errno = saved_errno;
return MNSH_FS_ERROR;
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 5d940dd..2aaee07 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;
@@ -4912,7 +4909,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
@@ -4926,7 +4922,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. */
@@ -4950,12 +4946,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..3d477ad 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)
--
2.9.3