From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 7 Jan 2015 01:14:14 +0000 (+0100)
Subject: util: make use of kcmp() to compare fds, if it is available
X-Git-Tag: v219~664
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=f7ad54a301e4ae8dceab54d3ab3934e56c1134ea

util: make use of kcmp() to compare fds, if it is available
---

diff --git a/configure.ac b/configure.ac
index 0496f5e3e..3dd40c357 100644
--- a/configure.ac
+++ b/configure.ac
@@ -310,7 +310,7 @@ LIBS="$save_LIBS"
 
 AC_CHECK_FUNCS([memfd_create])
 AC_CHECK_FUNCS([__secure_getenv secure_getenv])
-AC_CHECK_DECLS([gettid, pivot_root, name_to_handle_at, setns, getrandom, renameat2, LO_FLAGS_PARTSCAN],
+AC_CHECK_DECLS([gettid, pivot_root, name_to_handle_at, setns, getrandom, renameat2, kcmp, LO_FLAGS_PARTSCAN],
                [], [], [[
 #include <sys/types.h>
 #include <unistd.h>
diff --git a/src/shared/missing.h b/src/shared/missing.h
index 5cf179ef0..cdc38b2dd 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -711,3 +711,13 @@ static inline int renameat2(int oldfd, const char *oldname, int newfd, const cha
 #ifndef RENAME_NOREPLACE
 #define RENAME_NOREPLACE (1 << 0)
 #endif
+
+#if !HAVE_DECL_KCMP
+static inline int kcmp(pid_t pid1, pid_t pid2, int type, unsigned long idx1, unsigned long idx2) {
+        return syscall(__NR_kcmp, pid1, pid2, type, idx1, idx2);
+}
+#endif
+
+#ifndef KCMP_FILE
+#define KCMP_FILE 0
+#endif
diff --git a/src/shared/util.c b/src/shared/util.c
index 409ccc7ee..64059065d 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7678,13 +7678,35 @@ int fd_setcrtime(int fd, usec_t usec) {
 
 int same_fd(int a, int b) {
         struct stat sta, stb;
+        pid_t pid;
+        int r, fa, fb;
 
         assert(a >= 0);
         assert(b >= 0);
 
+        /* Compares two file descriptors. Note that semantics are
+         * quite different depending on whether we have kcmp() or we
+         * don't. If we have kcmp() this will only return true for
+         * dup()ed file descriptors, but not otherwise. If we don't
+         * have kcmp() this will also return true for two fds of the same
+         * file, created by separate open() calls. Since we use this
+         * call mostly for filtering out duplicates in the fd store
+         * this difference hopefully doesn't matter too much. */
+
         if (a == b)
                 return true;
 
+        /* Try to use kcmp() if we have it. */
+        pid = getpid();
+        r = kcmp(pid, pid, KCMP_FILE, a, b);
+        if (r == 0)
+                return true;
+        if (r > 0)
+                return false;
+        if (errno != ENOSYS)
+                return -errno;
+
+        /* We don't have kcmp(), use fstat() instead. */
         if (fstat(a, &sta) < 0)
                 return -errno;
 
@@ -7694,9 +7716,27 @@ int same_fd(int a, int b) {
         if ((sta.st_mode & S_IFMT) != (stb.st_mode & S_IFMT))
                 return false;
 
-        if (S_ISREG(sta.st_mode) || S_ISDIR(sta.st_mode) || S_ISFIFO(sta.st_mode) || S_ISSOCK(sta.st_mode) || S_ISLNK(sta.st_mode))
-                return (sta.st_dev == stb.st_dev) && (sta.st_ino == stb.st_ino);
+        /* We consider all device fds different, since two device fds
+         * might refer to quite different device contexts even though
+         * they share the same inode and backing dev_t. */
 
-        /* We consider all device fds different... */
-        return false;
+        if (S_ISCHR(sta.st_mode) || S_ISBLK(sta.st_mode))
+                return false;
+
+        if (sta.st_dev != stb.st_dev || sta.st_ino != stb.st_ino)
+                return false;
+
+        /* The fds refer to the same inode on disk, let's also check
+         * if they have the same fd flags. This is useful to
+         * distuingish the read and write side of a pipe created with
+         * pipe(). */
+        fa = fcntl(a, F_GETFL);
+        if (fa < 0)
+                return -errno;
+
+        fb = fcntl(b, F_GETFL);
+        if (fb < 0)
+                return -errno;
+
+        return fa == fb;
 }
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 3f1b5487f..3c79f8f4d 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -1381,6 +1381,40 @@ static void test_raw_clone(void) {
         }
 }
 
+static void test_same_fd(void) {
+        _cleanup_close_pair_ int p[2] = { -1, -1 };
+        _cleanup_close_ int a = -1, b = -1, c = -1;
+
+        assert_se(pipe2(p, O_CLOEXEC) >= 0);
+        assert_se((a = dup(p[0])) >= 0);
+        assert_se((b = open("/dev/null", O_RDONLY|O_CLOEXEC)) >= 0);
+        assert_se((c = dup(a)) >= 0);
+
+        assert_se(same_fd(p[0], p[0]) > 0);
+        assert_se(same_fd(p[1], p[1]) > 0);
+        assert_se(same_fd(a, a) > 0);
+        assert_se(same_fd(b, b) > 0);
+
+        assert_se(same_fd(a, p[0]) > 0);
+        assert_se(same_fd(p[0], a) > 0);
+        assert_se(same_fd(c, p[0]) > 0);
+        assert_se(same_fd(p[0], c) > 0);
+        assert_se(same_fd(a, c) > 0);
+        assert_se(same_fd(c, a) > 0);
+
+        assert_se(same_fd(p[0], p[1]) == 0);
+        assert_se(same_fd(p[1], p[0]) == 0);
+        assert_se(same_fd(p[0], b) == 0);
+        assert_se(same_fd(b, p[0]) == 0);
+        assert_se(same_fd(p[1], a) == 0);
+        assert_se(same_fd(a, p[1]) == 0);
+        assert_se(same_fd(p[1], b) == 0);
+        assert_se(same_fd(b, p[1]) == 0);
+
+        assert_se(same_fd(a, b) == 0);
+        assert_se(same_fd(b, a) == 0);
+}
+
 int main(int argc, char *argv[]) {
         log_parse_environment();
         log_open();
@@ -1455,6 +1489,7 @@ int main(int argc, char *argv[]) {
         test_unquote_many_words();
         test_parse_proc_cmdline();
         test_raw_clone();
+        test_same_fd();
 
         return 0;
 }