Merge pull request #30649 from YHNdnzj/close-nointr-unnecessary

Some modernization/cleanup for fd-util

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) {
+void close_many(const int fds[], size_t n_fds) {
-        assert(fds || n_fd <= 0);
+        assert(fds || n_fds == 0);
 
-        for (size_t i = 0; i < n_fd; i++)
+        FOREACH_ARRAY(fd, fds, n_fds)
-                safe_close(fds[i]);
+                safe_close(*fd);
 }
 
-void close_many_unset(int fds[], size_t n_fd) {
+void close_many_unset(int fds[], size_t n_fds) {
-        assert(fds || n_fd <= 0);
+        assert(fds || n_fds == 0);
 
-        for (size_t i = 0; i < n_fd; i++)
+        FOREACH_ARRAY(fd, fds, n_fds)
-                fds[i] = safe_close(fds[i]);
+                *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++) {
+        FOREACH_ARRAY(fd, fds, n_fds) {
-                if (fds[i] < 0) /* Skip gracefully over already invalidated fds */
+                if (*fd < 0) /* Skip gracefully over already invalidated fds */
                         continue;
 
-                r = fd_cloexec(fds[i], cloexec);
+                r = fd_cloexec(*fd, cloexec);
-                if (r < 0 && ret >= 0) /* Continue going, but return first error */
+                if (r < 0) /* Continue going, but return first error */
-                        ret = r;
+                        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) {
+static bool fd_in_set(int fd, const int fds[], size_t n_fds) {
-        assert(n_fdset == 0 || fdset);
+        assert(fd >= 0);
+        assert(fds || n_fds == 0);
 
-        for (size_t i = 0; i < n_fdset; i++) {
+        FOREACH_ARRAY(i, fds, n_fds) {
-                if (fdset[i] < 0)
+                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.",
+                                       "Refusing to loop over %d potential fds.", max_fd);
-                                       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)
+                if (q != -EBADF)
-                        r = q;
+                        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);

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(const int fds[], size_t n_fds);
-void close_many_unset(int fds[], size_t n_fd);
+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);

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;
 }

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;
 }

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) {
+                        if (r < 0)
-                                (void) close_nointr(fd);
                                 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) {

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

src/shared/fdset.c

@@ -3,6 +3,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <stddef.h>
+#include <unistd.h>
 
 #include "sd-daemon.h"
 
@@ -40,8 +41,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++) {
+        FOREACH_ARRAY(fd, fds, n_fds) {
-                r = fdset_put(s, fds[i]);
+                r = fdset_put(s, *fd);
                 if (r < 0)
                         return r;
         }
@@ -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);
         }
 }