From 1276e633708c2d8b41a3d4c0a981a830ee537465 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 28 Dec 2023 18:17:52 +0800 Subject: [PATCH 1/5] fd-util: modernization --- src/basic/fd-util.c | 52 ++++++++++++++++++++++----------------------- src/basic/fd-util.h | 4 ++-- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 9904e3e484..d704c90ade 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -92,22 +92,22 @@ void safe_close_pair(int p[static 2]) { p[1] = safe_close(p[1]); } -void close_many(const int fds[], size_t n_fd) { - assert(fds || n_fd <= 0); +void close_many(const int fds[], size_t n_fds) { + assert(fds || n_fds == 0); - for (size_t i = 0; i < n_fd; i++) - safe_close(fds[i]); + FOREACH_ARRAY(fd, fds, n_fds) + safe_close(*fd); } -void close_many_unset(int fds[], size_t n_fd) { - assert(fds || n_fd <= 0); +void close_many_unset(int fds[], size_t n_fds) { + assert(fds || n_fds == 0); - for (size_t i = 0; i < n_fd; i++) - fds[i] = safe_close(fds[i]); + FOREACH_ARRAY(fd, fds, n_fds) + *fd = safe_close(*fd); } void close_many_and_free(int *fds, size_t n_fds) { - assert(fds || n_fds <= 0); + assert(fds || n_fds == 0); close_many(fds, n_fds); free(fds); @@ -189,15 +189,15 @@ int fd_cloexec(int fd, bool cloexec) { int fd_cloexec_many(const int fds[], size_t n_fds, bool cloexec) { int ret = 0, r; - assert(n_fds == 0 || fds); + assert(fds || n_fds == 0); - for (size_t i = 0; i < n_fds; i++) { - if (fds[i] < 0) /* Skip gracefully over already invalidated fds */ + FOREACH_ARRAY(fd, fds, n_fds) { + if (*fd < 0) /* Skip gracefully over already invalidated fds */ continue; - r = fd_cloexec(fds[i], cloexec); - if (r < 0 && ret >= 0) /* Continue going, but return first error */ - ret = r; + r = fd_cloexec(*fd, cloexec); + if (r < 0) /* Continue going, but return first error */ + RET_GATHER(ret, r); else ret = 1; /* report if we did anything */ } @@ -205,14 +205,15 @@ int fd_cloexec_many(const int fds[], size_t n_fds, bool cloexec) { return ret; } -static bool fd_in_set(int fd, const int fdset[], size_t n_fdset) { - assert(n_fdset == 0 || fdset); +static bool fd_in_set(int fd, const int fds[], size_t n_fds) { + assert(fd >= 0); + assert(fds || n_fds == 0); - for (size_t i = 0; i < n_fdset; i++) { - if (fdset[i] < 0) + FOREACH_ARRAY(i, fds, n_fds) { + if (*i < 0) continue; - if (fdset[i] == fd) + if (*i == fd) return true; } @@ -243,7 +244,7 @@ int get_max_fd(void) { static int close_all_fds_frugal(const int except[], size_t n_except) { int max_fd, r = 0; - assert(n_except == 0 || except); + assert(except || n_except == 0); /* This is the inner fallback core of close_all_fds(). This never calls malloc() or opendir() or so * and hence is safe to be called in signal handler context. Most users should call close_all_fds(), @@ -258,8 +259,7 @@ static int close_all_fds_frugal(const int except[], size_t n_except) { * spin the CPU for a long time. */ if (max_fd > MAX_FD_LOOP_LIMIT) return log_debug_errno(SYNTHETIC_ERRNO(EPERM), - "Refusing to loop over %d potential fds.", - max_fd); + "Refusing to loop over %d potential fds.", max_fd); for (int fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -EBADF) { int q; @@ -268,8 +268,8 @@ static int close_all_fds_frugal(const int except[], size_t n_except) { continue; q = close_nointr(fd); - if (q < 0 && q != -EBADF && r >= 0) - r = q; + if (q != -EBADF) + RET_GATHER(r, q); } return r; @@ -598,7 +598,7 @@ int move_fd(int from, int to, int cloexec) { if (fl < 0) return -errno; - cloexec = !!(fl & FD_CLOEXEC); + cloexec = FLAGS_SET(fl, FD_CLOEXEC); } r = dup3(from, to, cloexec ? O_CLOEXEC : 0); diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 64918a4861..4bdd61fe54 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -32,8 +32,8 @@ static inline int safe_close_above_stdio(int fd) { return safe_close(fd); } -void close_many(const int fds[], size_t n_fd); -void close_many_unset(int fds[], size_t n_fd); +void close_many(const int fds[], size_t n_fds); +void close_many_unset(int fds[], size_t n_fds); void close_many_and_free(int *fds, size_t n_fds); int fclose_nointr(FILE *f); From 1cbd441b09c5880783795a5d0d192deb145a25f1 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 28 Dec 2023 18:07:12 +0800 Subject: [PATCH 2/5] fdset: use FOREACH_ARRAY at one more place --- src/shared/fdset.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/fdset.c b/src/shared/fdset.c index b408a38c37..4e86122fa5 100644 --- a/src/shared/fdset.c +++ b/src/shared/fdset.c @@ -40,8 +40,8 @@ int fdset_new_array(FDSet **ret, const int fds[], size_t n_fds) { if (!s) return -ENOMEM; - for (size_t i = 0; i < n_fds; i++) { - r = fdset_put(s, fds[i]); + FOREACH_ARRAY(fd, fds, n_fds) { + r = fdset_put(s, *fd); if (r < 0) return r; } From 3760416ee8055b252103e6ff5e823b37616fda33 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 28 Dec 2023 18:05:33 +0800 Subject: [PATCH 3/5] shared/async: use safe_close where appropriate --- src/shared/async.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/async.c b/src/shared/async.c index 41f6b97e02..bbb8b81011 100644 --- a/src/shared/async.c +++ b/src/shared/async.c @@ -94,7 +94,7 @@ int asynchronous_close(int fd) { pid = clone_with_nested_stack(close_func, CLONE_FILES | ((v & NEED_DOUBLE_FORK) ? 0 : SIGCHLD), UINT_TO_PTR(v)); if (pid < 0) - assert_se(close_nointr(fd) != -EBADF); /* local fallback */ + safe_close(fd); /* local fallback */ else if (v & NEED_DOUBLE_FORK) { /* Reap the intermediate child. Key here is that we specify __WCLONE, since we didn't ask for From 1189815a6b90c2cbdacf98b0ab75058a49951582 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 28 Dec 2023 20:43:12 +0800 Subject: [PATCH 4/5] logind-session-device: use _cleanup_close_ --- src/login/logind-session-device.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index 44d8d525ee..fc0b2fb86e 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -114,7 +114,8 @@ static int sd_drmdropmaster(int fd) { } static int session_device_open(SessionDevice *sd, bool active) { - int fd, r; + _cleanup_close_ int fd = -EBADF; + int r; assert(sd); assert(sd->type != DEVICE_TYPE_UNKNOWN); @@ -132,10 +133,8 @@ static int session_device_open(SessionDevice *sd, bool active) { /* Weird legacy DRM semantics might return an error even though we're master. No way to detect * that so fail at all times and let caller retry in inactive state. */ r = sd_drmsetmaster(fd); - if (r < 0) { - (void) close_nointr(fd); + if (r < 0) return r; - } } else /* DRM-Master is granted to the first user who opens a device automatically (ughh, * racy!). Hence, we just drop DRM-Master in case we were the first. */ @@ -153,7 +152,7 @@ static int session_device_open(SessionDevice *sd, bool active) { break; } - return fd; + return TAKE_FD(fd); } static int session_device_start(SessionDevice *sd) { From da6c52c57c949efb9c526d7c2dbef75b2a0d7440 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 28 Dec 2023 18:13:37 +0800 Subject: [PATCH 5/5] various: don't use close_nointr if retval is not checked anyway --- src/libsystemd/sd-login/sd-login.c | 2 +- src/libsystemd/sd-network/sd-network.c | 2 +- src/shared/fdset.c | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index 4d09b15653..3e88d349e1 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -1275,7 +1275,7 @@ _public_ int sd_login_monitor_new(const char *category, sd_login_monitor **m) { _public_ sd_login_monitor* sd_login_monitor_unref(sd_login_monitor *m) { if (m) - (void) close_nointr(MONITOR_TO_FD(m)); + (void) close(MONITOR_TO_FD(m)); return NULL; } diff --git a/src/libsystemd/sd-network/sd-network.c b/src/libsystemd/sd-network/sd-network.c index cf3c400dbc..9d11dce2e8 100644 --- a/src/libsystemd/sd-network/sd-network.c +++ b/src/libsystemd/sd-network/sd-network.c @@ -394,7 +394,7 @@ int sd_network_monitor_new(sd_network_monitor **m, const char *category) { sd_network_monitor* sd_network_monitor_unref(sd_network_monitor *m) { if (m) - (void) close_nointr(MONITOR_TO_FD(m)); + (void) close(MONITOR_TO_FD(m)); return NULL; } diff --git a/src/shared/fdset.c b/src/shared/fdset.c index 4e86122fa5..cb5a69ef22 100644 --- a/src/shared/fdset.c +++ b/src/shared/fdset.c @@ -3,6 +3,7 @@ #include #include #include +#include #include "sd-daemon.h" @@ -71,7 +72,7 @@ void fdset_close(FDSet *s) { log_debug("Closing set fd %i (%s)", fd, strna(path)); } - (void) close_nointr(fd); + (void) close(fd); } }