From 03e334a1c7dc8c20c38902aa039440763acc9b17 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 18 Mar 2014 19:22:43 +0100 Subject: util: replace close_nointr_nofail() by a more useful safe_close() safe_close() automatically becomes a NOP when a negative fd is passed, and returns -1 unconditionally. This makes it easy to write lines like this: fd = safe_close(fd); Which will close an fd if it is open, and reset the fd variable correctly. By making use of this new scheme we can drop a > 200 lines of code that was required to test for non-negative fds or to reset the closed fd variable afterwards. --- src/shared/ask-password-api.c | 17 ++++----- src/shared/fdset.c | 2 +- src/shared/install.c | 12 +++---- src/shared/log.c | 25 +++----------- src/shared/logs-show.c | 8 ++--- src/shared/spawn-polkit-agent.c | 4 +-- src/shared/util.c | 76 ++++++++++++++++++++--------------------- src/shared/util.h | 6 ++-- src/shared/watchdog.c | 3 +- 9 files changed, 64 insertions(+), 89 deletions(-) (limited to 'src/shared') diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index a328f145e9..117f0c6687 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -226,8 +226,7 @@ int ask_password_tty( r = 0; finish: - if (notify >= 0) - close_nointr_nofail(notify); + safe_close(notify); if (ttyfd >= 0) { @@ -236,7 +235,7 @@ finish: tcsetattr(ttyfd, TCSADRAIN, &old_termios); } - close_nointr_nofail(ttyfd); + safe_close(ttyfd); } return r; @@ -290,7 +289,7 @@ static int create_socket(char **name) { return fd; fail: - close_nointr_nofail(fd); + safe_close(fd); return r; } @@ -521,19 +520,15 @@ int ask_password_agent( r = 0; finish: - if (fd >= 0) - close_nointr_nofail(fd); + safe_close(fd); if (socket_name) { unlink(socket_name); free(socket_name); } - if (socket_fd >= 0) - close_nointr_nofail(socket_fd); - - if (signal_fd >= 0) - close_nointr_nofail(signal_fd); + safe_close(socket_fd); + safe_close(signal_fd); if (f) fclose(f); diff --git a/src/shared/fdset.c b/src/shared/fdset.c index fd27398ebb..a2c861de3f 100644 --- a/src/shared/fdset.c +++ b/src/shared/fdset.c @@ -82,7 +82,7 @@ int fdset_put_dup(FDSet *s, int fd) { r = fdset_put(s, copy); if (r < 0) { - close_nointr_nofail(copy); + safe_close(copy); return r; } diff --git a/src/shared/install.c b/src/shared/install.c index f57b94d599..74090463d9 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -204,7 +204,7 @@ static int remove_marked_symlinks_fd( d = fdopendir(fd); if (!d) { - close_nointr_nofail(fd); + safe_close(fd); return -errno; } @@ -244,7 +244,7 @@ static int remove_marked_symlinks_fd( p = path_make_absolute(de->d_name, path); if (!p) { - close_nointr_nofail(nfd); + safe_close(nfd); return -ENOMEM; } @@ -344,7 +344,7 @@ static int remove_marked_symlinks( r = q; } while (deleted); - close_nointr_nofail(fd); + safe_close(fd); return r; } @@ -367,7 +367,7 @@ static int find_symlinks_fd( d = fdopendir(fd); if (!d) { - close_nointr_nofail(fd); + safe_close(fd); return -errno; } @@ -403,7 +403,7 @@ static int find_symlinks_fd( p = path_make_absolute(de->d_name, path); if (!p) { - close_nointr_nofail(nfd); + safe_close(nfd); return -ENOMEM; } @@ -1008,7 +1008,7 @@ static int unit_file_load( f = fdopen(fd, "re"); if (!f) { - close_nointr_nofail(fd); + safe_close(fd); return -ENOMEM; } diff --git a/src/shared/log.c b/src/shared/log.c index 5ea1e3a0e8..a4b3b68ef1 100644 --- a/src/shared/log.c +++ b/src/shared/log.c @@ -62,7 +62,7 @@ void log_close_console(void) { if (getpid() == 1) { if (console_fd >= 3) - close_nointr_nofail(console_fd); + safe_close(console_fd); console_fd = -1; } @@ -84,12 +84,7 @@ static int log_open_console(void) { } void log_close_kmsg(void) { - - if (kmsg_fd < 0) - return; - - close_nointr_nofail(kmsg_fd); - kmsg_fd = -1; + kmsg_fd = safe_close(kmsg_fd); } static int log_open_kmsg(void) { @@ -105,12 +100,7 @@ static int log_open_kmsg(void) { } void log_close_syslog(void) { - - if (syslog_fd < 0) - return; - - close_nointr_nofail(syslog_fd); - syslog_fd = -1; + syslog_fd = safe_close(syslog_fd); } static int create_log_socket(int type) { @@ -152,7 +142,7 @@ static int log_open_syslog(void) { } if (connect(syslog_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) { - close_nointr_nofail(syslog_fd); + safe_close(syslog_fd); /* Some legacy syslog systems still use stream * sockets. They really shouldn't. But what can we @@ -180,12 +170,7 @@ fail: } void log_close_journal(void) { - - if (journal_fd < 0) - return; - - close_nointr_nofail(journal_fd); - journal_fd = -1; + journal_fd = safe_close(journal_fd); } static int log_open_journal(void) { diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index f7d84fc723..df49375724 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1171,8 +1171,7 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { if (child == 0) { int fd; - close_nointr_nofail(pair[0]); - pair[0] = -1; + pair[0] = safe_close(pair[0]); r = namespace_enter(pidnsfd, mntnsfd, rootfd); if (r < 0) @@ -1183,7 +1182,7 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { _exit(EXIT_FAILURE); k = loop_read(fd, buf, 36, false); - close_nointr_nofail(fd); + safe_close(fd); if (k != 36) _exit(EXIT_FAILURE); @@ -1194,8 +1193,7 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { _exit(EXIT_SUCCESS); } - close_nointr_nofail(pair[1]); - pair[1] = -1; + pair[1] = safe_close(pair[1]); r = wait_for_terminate(child, &si); if (r < 0 || si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) diff --git a/src/shared/spawn-polkit-agent.c b/src/shared/spawn-polkit-agent.c index f9e52cdcbd..fccf1e9173 100644 --- a/src/shared/spawn-polkit-agent.c +++ b/src/shared/spawn-polkit-agent.c @@ -61,7 +61,7 @@ int polkit_agent_open(void) { POLKIT_AGENT_BINARY_PATH, "--notify-fd", notify_fd, "--fallback", NULL); /* Close the writing side, because that's the one for the agent */ - close_nointr_nofail(pipe_fd[1]); + safe_close(pipe_fd[1]); if (r < 0) log_error("Failed to fork TTY ask password agent: %s", strerror(-r)); @@ -69,7 +69,7 @@ int polkit_agent_open(void) { /* Wait until the agent closes the fd */ fd_wait_for_event(pipe_fd[0], POLLHUP, (usec_t) -1); - close_nointr_nofail(pipe_fd[0]); + safe_close(pipe_fd[0]); return r; } diff --git a/src/shared/util.c b/src/shared/util.c index 75870fcbe2..a8c4523905 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -183,13 +183,22 @@ int close_nointr(int fd) { return -errno; } -void close_nointr_nofail(int fd) { - PROTECT_ERRNO; +int safe_close(int fd) { + + /* + * Like close_nointr() but cannot fail. Guarantees errno is + * unchanged. Is a NOP with negative fds passed, and returns + * -1, so that it can be used in this syntax: + * + * fd = safe_close(fd); + */ - /* like close_nointr() but cannot fail, and guarantees errno - * is unchanged */ + if (fd >= 0) { + PROTECT_ERRNO; + assert_se(close_nointr(fd) == 0); + } - assert_se(close_nointr(fd) == 0); + return -1; } void close_many(const int fds[], unsigned n_fd) { @@ -198,7 +207,7 @@ void close_many(const int fds[], unsigned n_fd) { assert(fds || n_fd <= 0); for (i = 0; i < n_fd; i++) - close_nointr_nofail(fds[i]); + safe_close(fds[i]); } int unlink_noerrno(const char *path) { @@ -1693,16 +1702,13 @@ finish: } int reset_terminal(const char *name) { - int fd, r; + _cleanup_close_ int fd = -1; fd = open_terminal(name, O_RDWR|O_NOCTTY|O_CLOEXEC); if (fd < 0) return fd; - r = reset_terminal_fd(fd, true); - close_nointr_nofail(fd); - - return r; + return reset_terminal_fd(fd, true); } int open_terminal(const char *name, int mode) { @@ -1741,12 +1747,12 @@ int open_terminal(const char *name, int mode) { r = isatty(fd); if (r < 0) { - close_nointr_nofail(fd); + safe_close(fd); return -errno; } if (!r) { - close_nointr_nofail(fd); + safe_close(fd); return -ENOTTY; } @@ -1935,11 +1941,10 @@ int acquire_terminal( * ended our handle will be dead. It's important that * we do this after sleeping, so that we don't enter * an endless loop. */ - close_nointr_nofail(fd); + safe_close(fd); } - if (notify >= 0) - close_nointr_nofail(notify); + safe_close(notify); r = reset_terminal_fd(fd, true); if (r < 0) @@ -1948,11 +1953,8 @@ int acquire_terminal( return fd; fail: - if (fd >= 0) - close_nointr_nofail(fd); - - if (notify >= 0) - close_nointr_nofail(notify); + safe_close(fd); + safe_close(notify); return r; } @@ -2262,7 +2264,7 @@ int make_stdio(int fd) { t = dup3(fd, STDERR_FILENO, 0); if (fd >= 3) - close_nointr_nofail(fd); + safe_close(fd); if (r < 0 || s < 0 || t < 0) return -errno; @@ -2640,7 +2642,7 @@ int rm_rf_children_dangerous(int fd, bool only_dirs, bool honour_sticky, struct d = fdopendir(fd); if (!d) { - close_nointr_nofail(fd); + safe_close(fd); return errno == ENOENT ? 0 : -errno; } @@ -2736,7 +2738,7 @@ int rm_rf_children(int fd, bool only_dirs, bool honour_sticky, struct stat *root assert(fd >= 0); if (fstatfs(fd, &s) < 0) { - close_nointr_nofail(fd); + safe_close(fd); return -errno; } @@ -2745,7 +2747,7 @@ int rm_rf_children(int fd, bool only_dirs, bool honour_sticky, struct stat *root * non-state data */ if (!is_temporary_fs(&s)) { log_error("Attempted to remove disk file system, and we can't allow that."); - close_nointr_nofail(fd); + safe_close(fd); return -EPERM; } @@ -2791,13 +2793,13 @@ static int rm_rf_internal(const char *path, bool only_dirs, bool delete_root, bo if (!dangerous) { if (fstatfs(fd, &s) < 0) { - close_nointr_nofail(fd); + safe_close(fd); return -errno; } if (!is_temporary_fs(&s)) { log_error("Attempted to remove disk file system, and we can't allow that."); - close_nointr_nofail(fd); + safe_close(fd); return -EPERM; } } @@ -3318,7 +3320,7 @@ char *ellipsize(const char *s, size_t length, unsigned percent) { } int touch(const char *path) { - int fd; + _cleanup_close_ int fd; assert(path); @@ -3330,7 +3332,6 @@ int touch(const char *path) { if (fd < 0) return -errno; - close_nointr_nofail(fd); return 0; } @@ -3493,7 +3494,7 @@ DIR *xopendirat(int fd, const char *name, int flags) { d = fdopendir(nfd); if (!d) { - close_nointr_nofail(nfd); + safe_close(nfd); return NULL; } @@ -3987,16 +3988,13 @@ int terminal_vhangup_fd(int fd) { } int terminal_vhangup(const char *name) { - int fd, r; + _cleanup_close_ int fd; fd = open_terminal(name, O_RDWR|O_NOCTTY|O_CLOEXEC); if (fd < 0) return fd; - r = terminal_vhangup_fd(fd); - close_nointr_nofail(fd); - - return r; + return terminal_vhangup_fd(fd); } int vt_disallocate(const char *name) { @@ -4023,7 +4021,7 @@ int vt_disallocate(const char *name) { "\033[H" /* move home */ "\033[2J", /* clear screen */ 10, false); - close_nointr_nofail(fd); + safe_close(fd); return 0; } @@ -4044,7 +4042,7 @@ int vt_disallocate(const char *name) { return fd; r = ioctl(fd, VT_DISALLOCATE, u); - close_nointr_nofail(fd); + safe_close(fd); if (r >= 0) return 0; @@ -4063,7 +4061,7 @@ int vt_disallocate(const char *name) { "\033[H" /* move home */ "\033[3J", /* clear screen including scrollback, requires Linux 2.6.40 */ 10, false); - close_nointr_nofail(fd); + safe_close(fd); return 0; } @@ -5640,7 +5638,7 @@ int on_ac_power(void) { if (n != 6 || memcmp(contents, "Mains\n", 6)) continue; - close_nointr_nofail(fd); + safe_close(fd); fd = openat(device, "online", O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fd < 0) { if (errno == ENOENT) diff --git a/src/shared/util.h b/src/shared/util.h index 7752b1e3df..70c20fdcf1 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -149,7 +149,8 @@ char *endswith(const char *s, const char *postfix) _pure_; bool first_word(const char *s, const char *word) _pure_; int close_nointr(int fd); -void close_nointr_nofail(int fd); +int safe_close(int fd); + void close_many(const int fds[], unsigned n_fd); int parse_size(const char *t, off_t base, off_t *size); @@ -620,8 +621,7 @@ static inline void freep(void *p) { struct __useless_struct_to_allow_trailing_semicolon__ static inline void closep(int *fd) { - if (*fd >= 0) - close_nointr_nofail(*fd); + safe_close(*fd); } static inline void umaskp(mode_t *u) { diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c index ddbe7afd3c..ba9ad9be97 100644 --- a/src/shared/watchdog.c +++ b/src/shared/watchdog.c @@ -164,6 +164,5 @@ void watchdog_close(bool disarm) { } } - close_nointr_nofail(watchdog_fd); - watchdog_fd = -1; + watchdog_fd = safe_close(watchdog_fd); } -- cgit v1.2.3-54-g00ecf