From 13084383e8955c2ff7017ac8839301688a9ee34d Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 22 Jul 2021 11:56:33 -0400 Subject: [PATCH] gdbsupport: make gdb_open_cloexec return scoped_fd Make gdb_open_cloexec return a scoped_fd, to encourage using automatic management of the file descriptor closing. Except in the most trivial cases, I changed the callers to just release the fd, which retains their existing behavior. That will allow the transition to using scoped_fd more to go gradually, one caller at a time. Change-Id: Ife022b403f96e71d5ebb4f1056ef6251b30fe554 --- gdb/auxv.c | 14 ++++++-------- gdb/corelow.c | 2 +- gdb/darwin-nat.c | 2 +- gdb/gdb_bfd.c | 2 +- gdb/inf-child.c | 2 +- gdb/linux-nat.c | 2 +- gdb/nat/linux-namespaces.c | 13 ++++--------- gdb/remote-fileio.c | 2 +- gdb/ser-unix.c | 2 +- gdb/solib.c | 7 ++++--- gdb/source.c | 16 +++++++--------- gdb/top.c | 8 +++----- gdb/tracefile-tfile.c | 2 +- gdbsupport/filestuff.cc | 8 ++++---- gdbsupport/filestuff.h | 7 ++++--- gdbsupport/scoped_mmap.cc | 2 +- 16 files changed, 41 insertions(+), 50 deletions(-) diff --git a/gdb/auxv.c b/gdb/auxv.c index 2bcf9f452e3..120e5c7cc21 100644 --- a/gdb/auxv.c +++ b/gdb/auxv.c @@ -46,23 +46,21 @@ procfs_xfer_auxv (gdb_byte *readbuf, ULONGEST len, ULONGEST *xfered_len) { - int fd; ssize_t l; std::string pathname = string_printf ("/proc/%d/auxv", inferior_ptid.pid ()); - fd = gdb_open_cloexec (pathname, writebuf != NULL ? O_WRONLY : O_RDONLY, 0); - if (fd < 0) + scoped_fd fd + = gdb_open_cloexec (pathname, writebuf != NULL ? O_WRONLY : O_RDONLY, 0); + if (fd.get () < 0) return TARGET_XFER_E_IO; if (offset != (ULONGEST) 0 - && lseek (fd, (off_t) offset, SEEK_SET) != (off_t) offset) + && lseek (fd.get (), (off_t) offset, SEEK_SET) != (off_t) offset) l = -1; else if (readbuf != NULL) - l = read (fd, readbuf, (size_t) len); + l = read (fd.get (), readbuf, (size_t) len); else - l = write (fd, writebuf, (size_t) len); - - (void) close (fd); + l = write (fd.get (), writebuf, (size_t) len); if (l < 0) return TARGET_XFER_E_IO; diff --git a/gdb/corelow.c b/gdb/corelow.c index 711e86c4cd4..5f48d96aa12 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -437,7 +437,7 @@ core_target_open (const char *arg, int from_tty) flags |= O_RDWR; else flags |= O_RDONLY; - scratch_chan = gdb_open_cloexec (filename.get (), flags, 0); + scratch_chan = gdb_open_cloexec (filename.get (), flags, 0).release (); if (scratch_chan < 0) perror_with_name (filename.get ()); diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c index f3478a55341..525f59ae2ce 100644 --- a/gdb/darwin-nat.c +++ b/gdb/darwin-nat.c @@ -1822,7 +1822,7 @@ may_have_sip () static void copy_shell_to_cache (const char *shell, const std::string &new_name) { - scoped_fd from_fd (gdb_open_cloexec (shell, O_RDONLY, 0)); + scoped_fd from_fd = gdb_open_cloexec (shell, O_RDONLY, 0); if (from_fd.get () < 0) error (_("Could not open shell (%s) for reading: %s"), shell, safe_strerror (errno)); diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 312442a466e..c6ff409d49c 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -541,7 +541,7 @@ gdb_bfd_open (const char *name, const char *target, int fd, if (fd == -1) { - fd = gdb_open_cloexec (name, O_RDONLY | O_BINARY, 0); + fd = gdb_open_cloexec (name, O_RDONLY | O_BINARY, 0).release (); if (fd == -1) { bfd_set_error (bfd_error_system_call); diff --git a/gdb/inf-child.c b/gdb/inf-child.c index f690aa77913..5084f448c1e 100644 --- a/gdb/inf-child.c +++ b/gdb/inf-child.c @@ -261,7 +261,7 @@ inf_child_target::fileio_open (struct inferior *inf, const char *filename, return -1; } - fd = gdb_open_cloexec (filename, nat_flags, nat_mode); + fd = gdb_open_cloexec (filename, nat_flags, nat_mode).release (); if (fd == -1) *target_errno = host_to_fileio_error (errno); diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index bac383dd5e8..6f50ea39142 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -3832,7 +3832,7 @@ linux_proc_xfer_memory_partial_pid (ptid_t ptid, "/proc/%d/task/%ld/mem", ptid.pid (), ptid.lwp ()); last_proc_mem_file.fd - = gdb_open_cloexec (filename, O_RDWR | O_LARGEFILE, 0); + = gdb_open_cloexec (filename, O_RDWR | O_LARGEFILE, 0).release (); if (last_proc_mem_file.fd == -1) { diff --git a/gdb/nat/linux-namespaces.c b/gdb/nat/linux-namespaces.c index 0c4fadd49f7..c5d5eb4ea8c 100644 --- a/gdb/nat/linux-namespaces.c +++ b/gdb/nat/linux-namespaces.c @@ -520,13 +520,8 @@ static ssize_t mnsh_handle_open (int sock, const char *filename, int flags, mode_t mode) { - int fd = gdb_open_cloexec (filename, flags, mode); - ssize_t result = mnsh_return_fd (sock, fd, errno); - - if (fd >= 0) - close (fd); - - return result; + scoped_fd fd = gdb_open_cloexec (filename, flags, mode); + return mnsh_return_fd (sock, fd.get (), errno); } /* Handle a MNSH_REQ_UNLINK message. Must be async-signal-safe. */ @@ -901,7 +896,7 @@ linux_mntns_access_fs (pid_t pid) if (ns == NULL) return MNSH_FS_DIRECT; - fd = gdb_open_cloexec (linux_ns_filename (ns, pid), O_RDONLY, 0); + fd = gdb_open_cloexec (linux_ns_filename (ns, pid), O_RDONLY, 0).release (); if (fd < 0) return MNSH_FS_ERROR; @@ -968,7 +963,7 @@ linux_mntns_open_cloexec (pid_t pid, const char *filename, return -1; if (access == MNSH_FS_DIRECT) - return gdb_open_cloexec (filename, flags, mode); + return gdb_open_cloexec (filename, flags, mode).release (); gdb_assert (access == MNSH_FS_HELPER); diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c index 9765093a723..20ec0f215aa 100644 --- a/gdb/remote-fileio.c +++ b/gdb/remote-fileio.c @@ -425,7 +425,7 @@ remote_fileio_func_open (remote_target *remote, char *buf) } } - fd = gdb_open_cloexec (pathname, flags, mode); + fd = gdb_open_cloexec (pathname, flags, mode).release (); if (fd < 0) { remote_fileio_return_errno (remote, -1); diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c index 96d024eea3d..597032afe89 100644 --- a/gdb/ser-unix.c +++ b/gdb/ser-unix.c @@ -75,7 +75,7 @@ static int hardwire_setstopbits (struct serial *, int); static int hardwire_open (struct serial *scb, const char *name) { - scb->fd = gdb_open_cloexec (name, O_RDWR, 0); + scb->fd = gdb_open_cloexec (name, O_RDWR, 0).release (); if (scb->fd < 0) return -1; diff --git a/gdb/solib.c b/gdb/solib.c index e30affbb7e7..dba3d843025 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -257,7 +257,8 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib) } /* Now see if we can open it. */ - found_file = gdb_open_cloexec (temp_pathname.get (), O_RDONLY | O_BINARY, 0); + found_file = gdb_open_cloexec (temp_pathname.get (), + O_RDONLY | O_BINARY, 0).release (); /* If the search in gdb_sysroot failed, and the path name has a drive spec (e.g, c:/foo), try stripping ':' from the drive spec, @@ -278,7 +279,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib) in_pathname + 2, (char *) NULL)); found_file = gdb_open_cloexec (temp_pathname.get (), - O_RDONLY | O_BINARY, 0); + O_RDONLY | O_BINARY, 0).release (); if (found_file < 0) { /* If the search in gdb_sysroot still failed, try fully @@ -292,7 +293,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib) in_pathname + 2, (char *) NULL)); found_file = gdb_open_cloexec (temp_pathname.get (), - O_RDONLY | O_BINARY, 0); + O_RDONLY | O_BINARY, 0).release (); } } diff --git a/gdb/source.c b/gdb/source.c index eec6e77eaa3..3559eeac283 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -832,7 +832,7 @@ openp (const char *path, openp_flags opts, const char *string, { filename = (char *) alloca (strlen (string) + 1); strcpy (filename, string); - fd = gdb_open_cloexec (filename, mode, 0); + fd = gdb_open_cloexec (filename, mode, 0).release (); if (fd >= 0) goto done; last_errno = errno; @@ -924,7 +924,7 @@ openp (const char *path, openp_flags opts, const char *string, if (is_regular_file (filename, ®_file_errno)) { - fd = gdb_open_cloexec (filename, mode, 0); + fd = gdb_open_cloexec (filename, mode, 0).release (); if (fd >= 0) break; last_errno = errno; @@ -1060,7 +1060,6 @@ find_and_open_source (const char *filename, { char *path = source_path; const char *p; - int result; /* If reading of source files is disabled then return a result indicating the attempt to read this source file failed. GDB will then display @@ -1080,12 +1079,11 @@ find_and_open_source (const char *filename, if (rewritten_fullname != NULL) *fullname = std::move (rewritten_fullname); - result = gdb_open_cloexec (fullname->get (), OPEN_MODE, 0); - - if (result >= 0) + scoped_fd result = gdb_open_cloexec (fullname->get (), OPEN_MODE, 0); + if (result.get () >= 0) { *fullname = gdb_realpath (fullname->get ()); - return scoped_fd (result); + return result; } /* Didn't work -- free old one, try again. */ @@ -1129,8 +1127,8 @@ find_and_open_source (const char *filename, filename = rewritten_filename.get (); /* Try to locate file using filename. */ - result = openp (path, OPF_SEARCH_IN_PATH | OPF_RETURN_REALPATH, filename, - OPEN_MODE, fullname); + int result = openp (path, OPF_SEARCH_IN_PATH | OPF_RETURN_REALPATH, filename, + OPEN_MODE, fullname); if (result < 0 && dirname != NULL) { /* Remove characters from the start of PATH that we don't need when diff --git a/gdb/top.c b/gdb/top.c index 7e95ed3969c..bca007e38ff 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -335,13 +335,11 @@ ui::~ui () static gdb_file_up open_terminal_stream (const char *name) { - int fd; - - fd = gdb_open_cloexec (name, O_RDWR | O_NOCTTY, 0); - if (fd < 0) + scoped_fd fd = gdb_open_cloexec (name, O_RDWR | O_NOCTTY, 0); + if (fd.get () < 0) perror_with_name (_("opening terminal failed")); - return gdb_file_up (fdopen (fd, "w+")); + return fd.to_file ("w+"); } /* Implementation of the "new-ui" command. */ diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c index 33ce86bbe23..e1534826c5f 100644 --- a/gdb/tracefile-tfile.c +++ b/gdb/tracefile-tfile.c @@ -475,7 +475,7 @@ tfile_target_open (const char *arg, int from_tty) flags = O_BINARY | O_LARGEFILE; flags |= O_RDONLY; - scratch_chan = gdb_open_cloexec (filename.get (), flags, 0); + scratch_chan = gdb_open_cloexec (filename.get (), flags, 0).release (); if (scratch_chan < 0) perror_with_name (filename.get ()); diff --git a/gdbsupport/filestuff.cc b/gdbsupport/filestuff.cc index 6ea2ad842d5..2975a0e6a99 100644 --- a/gdbsupport/filestuff.cc +++ b/gdbsupport/filestuff.cc @@ -306,13 +306,13 @@ socket_mark_cloexec (int fd) /* See filestuff.h. */ -int +scoped_fd gdb_open_cloexec (const char *filename, int flags, unsigned long mode) { - int fd = open (filename, flags | O_CLOEXEC, mode); + scoped_fd fd (open (filename, flags | O_CLOEXEC, mode)); - if (fd >= 0) - maybe_mark_cloexec (fd); + if (fd.get () >= 0) + maybe_mark_cloexec (fd.get ()); return fd; } diff --git a/gdbsupport/filestuff.h b/gdbsupport/filestuff.h index aa15f89aa92..a2cb916dd15 100644 --- a/gdbsupport/filestuff.h +++ b/gdbsupport/filestuff.h @@ -22,6 +22,7 @@ #include #include #include "gdb_file.h" +#include "scoped_fd.h" /* Note all the file descriptors which are open when this is called. These file descriptors will not be closed by close_most_fds. */ @@ -47,8 +48,8 @@ extern void close_most_fds (void); /* Like 'open', but ensures that the returned file descriptor has the close-on-exec flag set. */ -extern int gdb_open_cloexec (const char *filename, int flags, - /* mode_t */ unsigned long mode); +extern scoped_fd gdb_open_cloexec (const char *filename, int flags, + /* mode_t */ unsigned long mode); /* Like mkstemp, but ensures that the file descriptor is close-on-exec. */ @@ -63,7 +64,7 @@ gdb_mkostemp_cloexec (char *name_template, int flags = 0) /* Convenience wrapper for the above, which takes the filename as an std::string. */ -static inline int +static inline scoped_fd gdb_open_cloexec (const std::string &filename, int flags, /* mode_t */ unsigned long mode) { diff --git a/gdbsupport/scoped_mmap.cc b/gdbsupport/scoped_mmap.cc index c76fb77b72d..598b3280381 100644 --- a/gdbsupport/scoped_mmap.cc +++ b/gdbsupport/scoped_mmap.cc @@ -27,7 +27,7 @@ scoped_mmap mmap_file (const char *filename) { - scoped_fd fd (gdb_open_cloexec (filename, O_RDONLY, 0)); + scoped_fd fd = gdb_open_cloexec (filename, O_RDONLY, 0); if (fd.get () < 0) perror_with_name (("open")); -- 2.30.2