From: Simon Marchi <simon.marchi@polymtl.ca>
To: Tom Tromey <tom@tromey.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA v2 09/24] Remove close cleanup
Date: Mon, 31 Jul 2017 19:09:00 -0000 [thread overview]
Message-ID: <0f32812a9b3d7dc71fad1d28a5593bbe@polymtl.ca> (raw)
In-Reply-To: <20170725172107.9799-10-tom@tromey.com>
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
next prev parent reply other threads:[~2017-07-31 19:09 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-25 17:24 [RFA v2 00/24] More miscellaneous C++-ification Tom Tromey
2017-07-25 17:21 ` [RFA v2 22/24] Introduce gdb_argv, a class wrapper for buildargv Tom Tromey
2017-07-31 20:22 ` Simon Marchi
2017-07-25 17:21 ` [RFA v2 19/24] Replace do_restore_instream_cleanup with scoped_restore Tom Tromey
2017-07-25 17:21 ` [RFA v2 18/24] Use a scoped_restore for command_nest_depth Tom Tromey
2017-07-25 17:21 ` [RFA v2 23/24] Use gdb_argv in Python Tom Tromey
2017-07-31 20:26 ` Simon Marchi
2017-07-25 17:22 ` [RFA v2 05/24] Use gdb_file_up in source.c Tom Tromey
2017-07-30 18:59 ` Simon Marchi
2017-07-25 17:22 ` [RFA v2 12/24] More uses of scoped_restore Tom Tromey
2017-07-25 17:22 ` [RFA v2 16/24] Remove in_user_command Tom Tromey
2017-07-25 17:24 ` [RFA v2 06/24] Change open_terminal_stream to return a gdb_file_up Tom Tromey
2017-07-30 19:04 ` Simon Marchi
2017-07-25 17:24 ` [RFA v2 08/24] Remove an unlink cleanup Tom Tromey
2017-07-31 18:47 ` Simon Marchi
2017-07-25 17:24 ` [RFA v2 09/24] Remove close cleanup Tom Tromey
2017-07-31 19:09 ` Simon Marchi [this message]
2017-07-25 17:24 ` [RFA v2 24/24] Remove make_cleanup_freeargv and gdb_buildargv Tom Tromey
2017-07-31 20:26 ` Simon Marchi
2017-07-25 17:25 ` [RFA v2 10/24] Remove make_cleanup_restore_current_language Tom Tromey
2017-07-31 19:21 ` Simon Marchi
2017-07-31 22:17 ` Tom Tromey
2017-08-01 8:44 ` Simon Marchi
2017-07-25 17:26 ` [RFA v2 01/24] Introduce and use ui_out_emit_table Tom Tromey
2017-07-29 23:10 ` Simon Marchi
2017-07-30 16:23 ` Tom Tromey
2017-07-30 18:29 ` Simon Marchi
2017-07-31 22:12 ` Tom Tromey
2017-07-25 17:26 ` [RFA v2 07/24] Remove make_cleanup_fclose Tom Tromey
2017-07-30 19:05 ` Simon Marchi
2017-07-25 17:26 ` [RFA v2 21/24] Remove a cleanup in Python Tom Tromey
2017-07-25 17:26 ` [RFA v2 04/24] Use gdb_file_up in fbsd-nat.c Tom Tromey
2017-07-29 23:56 ` Simon Marchi
2017-07-25 17:26 ` [RFA v2 02/24] Introduce and use gdb_file_up Tom Tromey
2017-07-29 23:40 ` Simon Marchi
2017-07-30 16:25 ` Tom Tromey
2017-07-30 18:31 ` Simon Marchi
2017-07-25 17:27 ` [RFA v2 14/24] Use unique_xmalloc_ptr in jit.c Tom Tromey
2017-07-31 19:25 ` Simon Marchi
2017-07-25 17:27 ` [RFA v2 15/24] Use containers to avoid cleanups Tom Tromey
2017-07-31 19:42 ` Simon Marchi
2017-07-25 17:27 ` [RFA v2 03/24] Change return type of find_and_open_script Tom Tromey
2017-07-29 23:54 ` Simon Marchi
2017-07-30 16:27 ` Tom Tromey
2017-07-25 17:27 ` [RFA v2 13/24] Replace tui_restore_gdbout with scoped_restore Tom Tromey
2017-07-25 17:51 ` [RFA v2 20/24] Avoid some manual memory management in Python Tom Tromey
2017-07-25 18:02 ` [RFA v2 17/24] Remove user_call_depth Tom Tromey
2017-07-31 19:46 ` Simon Marchi
2017-07-25 18:04 ` [RFA v2 11/24] Remove make_cleanup_free_so Tom Tromey
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=0f32812a9b3d7dc71fad1d28a5593bbe@polymtl.ca \
--to=simon.marchi@polymtl.ca \
--cc=gdb-patches@sourceware.org \
--cc=tom@tromey.com \
/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