From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 8 Oct 2024 09:51:48 +0000 (+0200)
Subject: fd-util: introduce fd_validate() helper
X-Git-Tag: v257-rc1~294^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6056663a14863da81ef6dad3258e514382c31adc;p=thirdparty%2Fsystemd.git

fd-util: introduce fd_validate() helper

It just uses F_GETFD to validate an fd. it's a bit easier to read
though, and handles the < 0 case internally.
---

diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c
index 88b76c51d0b..3f8c5b92d32 100644
--- a/src/basic/fd-util.c
+++ b/src/basic/fd-util.c
@@ -511,6 +511,16 @@ int pack_fds(int fds[], size_t n_fds) {
         return 0;
 }
 
+int fd_validate(int fd) {
+        if (fd < 0)
+                return -EBADF;
+
+        if (fcntl(fd, F_GETFD) < 0)
+                return -errno;
+
+        return 0;
+}
+
 int same_fd(int a, int b) {
         struct stat sta, stb;
         pid_t pid;
diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h
index 3a56d2cbbf1..93b254c6805 100644
--- a/src/basic/fd-util.h
+++ b/src/basic/fd-util.h
@@ -80,6 +80,7 @@ int close_all_fds_without_malloc(const int except[], size_t n_except);
 
 int pack_fds(int fds[], size_t n);
 
+int fd_validate(int fd);
 int same_fd(int a, int b);
 
 void cmsg_close_all(struct msghdr *mh);
diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c
index b5469b5d98b..1e28a45fde2 100644
--- a/src/libsystemd/sd-daemon/sd-daemon.c
+++ b/src/libsystemd/sd-daemon/sd-daemon.c
@@ -400,9 +400,12 @@ _public_ int sd_is_socket_unix(int fd, int type, int listening, const char *path
 
 _public_ int sd_is_mq(int fd, const char *path) {
         struct mq_attr attr;
+        int r;
 
         /* Check that the fd is valid */
-        assert_return(fcntl(fd, F_GETFD) >= 0, -errno);
+        r = fd_validate(fd);
+        if (r < 0)
+                return r;
 
         if (mq_getattr(fd, &attr) < 0) {
                 if (errno == EBADF)
diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c
index f2b65d492a6..e49a5dde457 100644
--- a/src/test/test-fd-util.c
+++ b/src/test/test-fd-util.c
@@ -40,9 +40,9 @@ TEST(close_many) {
 
         close_many(fds, 2);
 
-        assert_se(fcntl(fds[0], F_GETFD) == -1);
-        assert_se(fcntl(fds[1], F_GETFD) == -1);
-        assert_se(fcntl(fds[2], F_GETFD) >= 0);
+        assert_se(fd_validate(fds[0]) == -EBADF);
+        assert_se(fd_validate(fds[1]) == -EBADF);
+        assert_se(fd_validate(fds[2]) >= 0);
 
         safe_close(fds[2]);
 }
@@ -57,6 +57,19 @@ TEST(close_nointr) {
         assert_se(close_nointr(fd) < 0);
 }
 
+TEST(fd_validate) {
+        assert_se(fd_validate(-EINVAL) == -EBADF);
+        assert_se(fd_validate(-EBADF) == -EBADF);
+
+        _cleanup_close_ int b = -EBADF;
+        assert_se((b = open("/dev/null", O_RDONLY|O_CLOEXEC)) >= 0);
+
+        assert_se(fd_validate(b) == 0);
+        safe_close(b);
+        assert_se(fd_validate(b) == -EBADF);
+        TAKE_FD(b);
+}
+
 TEST(same_fd) {
         _cleanup_close_pair_ int p[2];
         _cleanup_close_ int a, b, c;
@@ -222,9 +235,9 @@ static size_t validate_fds(
                         continue;
 
                 if (opened)
-                        assert_se(fcntl(fds[i], F_GETFD) >= 0);
+                        assert_se(fd_validate(fds[i]) >= 0);
                 else
-                        assert_se(fcntl(fds[i], F_GETFD) < 0 && errno == EBADF);
+                        assert_se(fd_validate(fds[i]) == -EBADF);
 
                 c++;
         }
diff --git a/src/test/test-fdset.c b/src/test/test-fdset.c
index cfbd8e270a8..7aeaf5f129c 100644
--- a/src/test/test-fdset.c
+++ b/src/test/test-fdset.c
@@ -23,8 +23,7 @@ TEST(fdset_new_fill) {
         assert_se(fdset_new_fill(/* filter_cloexec= */ -1, &fdset) >= 0);
         assert_se(fdset_contains(fdset, fd));
         fdset = fdset_free(fdset);
-        assert_se(fcntl(fd, F_GETFD) < 0);
-        assert_se(errno == EBADF);
+        assert_se(fd_validate(fd) == -EBADF);
 
         fd = open("/dev/null", O_CLOEXEC|O_RDONLY);
         assert_se(fd >= 0);
@@ -32,13 +31,12 @@ TEST(fdset_new_fill) {
         assert_se(fdset_new_fill(/* filter_cloexec= */ 0, &fdset) >= 0);
         assert_se(!fdset_contains(fdset, fd));
         fdset = fdset_free(fdset);
-        assert_se(fcntl(fd, F_GETFD) >= 0);
+        assert_se(fd_validate(fd) >= 0);
 
         assert_se(fdset_new_fill(/* filter_cloexec= */ 1, &fdset) >= 0);
         assert_se(fdset_contains(fdset, fd));
         fdset = fdset_free(fdset);
-        assert_se(fcntl(fd, F_GETFD) < 0);
-        assert_se(errno == EBADF);
+        assert_se(fd_validate(fd) == -EBADF);
 
         fd = open("/dev/null", O_RDONLY);
         assert_se(fd >= 0);
@@ -46,7 +44,7 @@ TEST(fdset_new_fill) {
         assert_se(fdset_new_fill(/* filter_cloexec= */ 1, &fdset) >= 0);
         assert_se(!fdset_contains(fdset, fd));
         fdset = fdset_free(fdset);
-        assert_se(fcntl(fd, F_GETFD) >= 0);
+        assert_se(fd_validate(fd) >= 0);
 
         assert_se(fdset_new_fill(/* filter_cloexec= */ 0, &fdset) >= 0);
         assert_se(fdset_contains(fdset, fd));
@@ -54,8 +52,7 @@ TEST(fdset_new_fill) {
         assert_se(flags >= 0);
         assert_se(FLAGS_SET(flags, FD_CLOEXEC));
         fdset = fdset_free(fdset);
-        assert_se(fcntl(fd, F_GETFD) < 0);
-        assert_se(errno == EBADF);
+        assert_se(fd_validate(fd) == -EBADF);
 
         log_open();
 }
@@ -102,10 +99,8 @@ TEST(fdset_cloexec) {
 }
 
 TEST(fdset_close_others) {
-        int fd = -EBADF;
-        int copyfd = -EBADF;
+        int fd = -EBADF, copyfd = -EBADF;
         _cleanup_fdset_free_ FDSet *fdset = NULL;
-        int flags = -1;
         _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-fdset_close_others.XXXXXX";
 
         fd = mkostemp_safe(name);
@@ -121,15 +116,13 @@ TEST(fdset_close_others) {
         log_close();
         assert_se(fdset_close_others(fdset) >= 0);
 
-        flags = fcntl(fd, F_GETFD);
-        assert_se(flags < 0);
+        assert_se(fd_validate(fd) == -EBADF);
 
         /* Open log again after checking that fd is invalid, since reopening the log might make fd a valid
          * file descriptor again. */
         (void) log_open();
 
-        flags = fcntl(copyfd, F_GETFD);
-        assert_se(flags >= 0);
+        assert_se(fd_validate(copyfd) >= 0);
 }
 
 TEST(fdset_remove) {
@@ -146,7 +139,7 @@ TEST(fdset_remove) {
         assert_se(fdset_remove(fdset, fd) >= 0);
         assert_se(!fdset_contains(fdset, fd));
 
-        assert_se(fcntl(fd, F_GETFD) >= 0);
+        assert_se(fd_validate(fd) >= 0);
 }
 
 TEST(fdset_iterate) {