From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53997 invoked by alias); 3 May 2017 22:46:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 52513 invoked by uid 89); 3 May 2017 22:46:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Offer, again! X-HELO: gproxy6.mail.unifiedlayer.com Received: from gproxy6-pub.mail.unifiedlayer.com (HELO gproxy6.mail.unifiedlayer.com) (67.222.39.168) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 May 2017 22:46:35 +0000 Received: from CMOut01 (unknown [10.0.90.82]) by gproxy6.mail.unifiedlayer.com (Postfix) with ESMTP id 9C1191E069E for ; Wed, 3 May 2017 16:46:36 -0600 (MDT) Received: from box522.bluehost.com ([74.220.219.122]) by CMOut01 with id FymZ1v00M2f2jeq01ymcM3; Wed, 03 May 2017 16:46:36 -0600 X-Authority-Analysis: v=2.2 cv=K+5SJ2eI c=1 sm=1 tr=0 a=GsOEXm/OWkKvwdLVJsfwcA==:117 a=GsOEXm/OWkKvwdLVJsfwcA==:17 a=tJ8p9aeEuA8A:10 a=zstS-IiYAAAA:8 a=k7x0OwkvQrLILkgMW64A:9 a=4G6NA9xxw8l3yy4pmD5M:22 Received: from 75-166-63-71.hlrn.qwest.net ([75.166.63.71]:53090 helo=bapiya.Home) by box522.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.87) (envelope-from ) id 1d632W-00051x-OQ; Wed, 03 May 2017 16:46:33 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA 09/23] Remove close cleanup Date: Wed, 03 May 2017 22:46:00 -0000 Message-Id: <20170503224626.2818-10-tom@tromey.com> In-Reply-To: <20170503224626.2818-1-tom@tromey.com> References: <20170503224626.2818-1-tom@tromey.com> X-BWhitelist: no X-Exim-ID: 1d632W-00051x-OQ X-Source-Sender: 75-166-63-71.hlrn.qwest.net (bapiya.Home) [75.166.63.71]:53090 X-Source-Auth: tom+tromey.com X-Email-Count: 16 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTIyLmJsdWVob3N0LmNvbQ== X-SW-Source: 2017-05/txt/msg00082.txt.bz2 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 * 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 + * 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 + * 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