Merge pull request #21097 from poettering/dir-is-empty-fix

dir_is_empty_at() tweaks

src/basic/dirent-util.h

@@ -33,3 +33,32 @@ struct dirent *readdir_no_dot(DIR *dirp);
                         }                                               \
                         break;                                          \
                 } else
+
+/* Maximum space one dirent structure might require at most */
+#define DIRENT_SIZE_MAX CONST_MAX(sizeof(struct dirent), offsetof(struct dirent, d_name) + NAME_MAX + 1)
+
+/* Only if 64bit off_t is enabled struct dirent + struct dirent64 are actually the same. We require this, and
+ * we want them to be interchangeable to make getdents64() work, hence verify that. */
+assert_cc(_FILE_OFFSET_BITS == 64);
+assert_cc(sizeof(struct dirent) == sizeof(struct dirent64));
+assert_cc(offsetof(struct dirent, d_ino) == offsetof(struct dirent64, d_ino));
+assert_cc(sizeof_field(struct dirent, d_ino) == sizeof_field(struct dirent64, d_ino));
+assert_cc(offsetof(struct dirent, d_off) == offsetof(struct dirent64, d_off));
+assert_cc(sizeof_field(struct dirent, d_off) == sizeof_field(struct dirent64, d_off));
+assert_cc(offsetof(struct dirent, d_reclen) == offsetof(struct dirent64, d_reclen));
+assert_cc(sizeof_field(struct dirent, d_reclen) == sizeof_field(struct dirent64, d_reclen));
+assert_cc(offsetof(struct dirent, d_type) == offsetof(struct dirent64, d_type));
+assert_cc(sizeof_field(struct dirent, d_type) == sizeof_field(struct dirent64, d_type));
+assert_cc(offsetof(struct dirent, d_name) == offsetof(struct dirent64, d_name));
+assert_cc(sizeof_field(struct dirent, d_name) == sizeof_field(struct dirent64, d_name));
+
+#define FOREACH_DIRENT_IN_BUFFER(de, buf, sz)                           \
+        for (void *_end = (uint8_t*) ({ (de) = (buf); }) + (sz);        \
+             (uint8_t*) (de) < (uint8_t*) _end;                         \
+             (de) = (struct dirent*) ((uint8_t*) (de) + (de)->d_reclen))
+
+#define DEFINE_DIRENT_BUFFER(name, sz)                                  \
+        union {                                                         \
+                struct dirent de;                                       \
+                uint8_t data[(sz) * DIRENT_SIZE_MAX];                   \
+        } name

src/basic/recurse-dir.c

@@ -25,38 +25,21 @@ static bool ignore_dirent(const struct dirent *de, RecurseDirFlags flags) {
                 dot_or_dot_dot(de->d_name);
 }
 
-/* Maximum space one direent structure might require at most */
-#define DIRENT_SIZE_MAX MAX(sizeof(struct dirent), offsetof(struct dirent, d_name) + NAME_MAX + 1)
-
 int readdir_all(int dir_fd,
                 RecurseDirFlags flags,
                 DirectoryEntries **ret) {
 
         _cleanup_free_ DirectoryEntries *de = NULL;
+        struct dirent *entry;
         DirectoryEntries *nde;
         size_t add, sz, j;
 
         assert(dir_fd >= 0);
 
         /* Returns an array with pointers to "struct dirent" directory entries, optionally sorted. Free the
-         * array with readdir_all_freep(). */
-
-        /* Only if 64bit off_t is enabled struct dirent + struct dirent64 are actually the same. We require
-         * this, and we want them to be interchangeable, hence verify that. */
-        assert_cc(_FILE_OFFSET_BITS == 64);
-        assert_cc(sizeof(struct dirent) == sizeof(struct dirent64));
-        assert_cc(offsetof(struct dirent, d_ino) == offsetof(struct dirent64, d_ino));
-        assert_cc(sizeof(((struct dirent*) NULL)->d_ino) == sizeof(((struct dirent64*) NULL)->d_ino));
-        assert_cc(offsetof(struct dirent, d_off) == offsetof(struct dirent64, d_off));
-        assert_cc(sizeof(((struct dirent*) NULL)->d_off) == sizeof(((struct dirent64*) NULL)->d_off));
-        assert_cc(offsetof(struct dirent, d_reclen) == offsetof(struct dirent64, d_reclen));
-        assert_cc(sizeof(((struct dirent*) NULL)->d_reclen) == sizeof(((struct dirent64*) NULL)->d_reclen));
-        assert_cc(offsetof(struct dirent, d_type) == offsetof(struct dirent64, d_type));
-        assert_cc(sizeof(((struct dirent*) NULL)->d_type) == sizeof(((struct dirent64*) NULL)->d_type));
-        assert_cc(offsetof(struct dirent, d_name) == offsetof(struct dirent64, d_name));
-        assert_cc(sizeof(((struct dirent*) NULL)->d_name) == sizeof(((struct dirent64*) NULL)->d_name));
-
-        /* Start with space for up to 8 directory entries. We expect at least 2 ("." + ".."), hence hopefully
+         * array with readdir_all_freep().
+         *
+         * Start with space for up to 8 directory entries. We expect at least 2 ("." + ".."), hence hopefully
          * 8 will cover most cases comprehensively. (Note that most likely a lot more entries will actually
          * fit in the buffer, given we calculate maximum file name length here.) */
         de = malloc(offsetof(DirectoryEntries, buffer) + DIRENT_SIZE_MAX * 8);
@@ -71,12 +54,14 @@ int readdir_all(int dir_fd,
                 bs = MIN(MALLOC_SIZEOF_SAFE(de) - offsetof(DirectoryEntries, buffer), (size_t) SSIZE_MAX);
                 assert(bs > de->buffer_size);
 
-                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
+                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
                 if (n < 0)
                         return -errno;
                 if (n == 0)
                         break;
 
+                msan_unpoison((uint8_t*) de->buffer + de->buffer_size, n);
+
                 de->buffer_size += n;
 
                 if (de->buffer_size < bs - DIRENT_SIZE_MAX) /* Still room for one more entry, then try to
@@ -95,10 +80,7 @@ int readdir_all(int dir_fd,
         }
 
         de->n_entries = 0;
-        for (struct dirent *entry = (struct dirent*) de->buffer;
-             (uint8_t*) entry < de->buffer + de->buffer_size;
-             entry = (struct dirent*) ((uint8_t*) entry + entry->d_reclen)) {
-
+        FOREACH_DIRENT_IN_BUFFER(entry, de->buffer, de->buffer_size) {
                 if (ignore_dirent(entry, flags))
                         continue;
 
@@ -118,10 +100,7 @@ int readdir_all(int dir_fd,
         de->entries = (struct dirent**) ((uint8_t*) de + ALIGN(offsetof(DirectoryEntries, buffer) + de->buffer_size));
 
         j = 0;
-        for (struct dirent *entry = (struct dirent*) de->buffer;
-             (uint8_t*) entry < de->buffer + de->buffer_size;
-             entry = (struct dirent*) ((uint8_t*) entry + entry->d_reclen)) {
-
+        FOREACH_DIRENT_IN_BUFFER(entry, de->buffer, de->buffer_size) {
                 if (ignore_dirent(entry, flags))
                         continue;
 

src/basic/recurse-dir.h

@@ -71,7 +71,7 @@ typedef struct DirectoryEntries {
         size_t n_entries;
         struct dirent** entries;
         size_t buffer_size;
-        uint8_t buffer[] _alignas_(struct dirent);
+        struct dirent buffer[];
 } DirectoryEntries;
 
 int readdir_all(int dir_fd, RecurseDirFlags flags, DirectoryEntries **ret);

src/basic/stat-util.c

@@ -73,27 +73,41 @@ int is_device_node(const char *path) {
 
 int dir_is_empty_at(int dir_fd, const char *path) {
         _cleanup_close_ int fd = -1;
-        _cleanup_closedir_ DIR *d = NULL;
+        /* Allocate space for at least 3 full dirents, since every dir has at least two entries ("."  +
+         * ".."), and only once we have seen if there's a third we know whether the dir is empty or not. */
+        DEFINE_DIRENT_BUFFER(buffer, 3);
         struct dirent *de;
+        ssize_t n;
 
         if (path) {
+                assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
+
                 fd = openat(dir_fd, path, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
                 if (fd < 0)
                         return -errno;
+        } else if (dir_fd == AT_FDCWD) {
+                fd = open(".", O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+                if (fd < 0)
+                        return -errno;
         } else {
-                /* Note that DUPing is not enough, as the internal pointer
-                 * would still be shared and moved by FOREACH_DIRENT. */
-                fd = fd_reopen(dir_fd, O_CLOEXEC);
+                /* Note that DUPing is not enough, as the internal pointer would still be shared and moved
+                 * getedents64(). */
+                assert(dir_fd >= 0);
+
+                fd = fd_reopen(dir_fd, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
                 if (fd < 0)
                         return fd;
         }
 
-        d = take_fdopendir(&fd);
-        if (!d)
+        n = getdents64(fd, &buffer, sizeof(buffer));
+        if (n < 0)
                 return -errno;
 
-        FOREACH_DIRENT(de, d, return -errno)
-                return 0;
+        msan_unpoison(&buffer, n);
+
+        FOREACH_DIRENT_IN_BUFFER(de, &buffer.de, n)
+                if (!dot_or_dot_dot(de->d_name))
+                        return 0;
 
         return 1;
 }

src/test/test-stat-util.c

@@ -8,10 +8,12 @@
 #include "alloc-util.h"
 #include "errno-list.h"
 #include "fd-util.h"
+#include "fs-util.h"
 #include "macro.h"
 #include "mountpoint-util.h"
 #include "namespace-util.h"
 #include "path-util.h"
+#include "rm-rf.h"
 #include "stat-util.h"
 #include "tests.h"
 #include "tmpfile-util.h"
@@ -223,6 +225,35 @@ static void test_device_path_make_canonical(void) {
         }
 }
 
+static void test_dir_is_empty(void) {
+        _cleanup_(rm_rf_physical_and_freep) char *empty_dir = NULL;
+        _cleanup_free_ char *j = NULL, *jj = NULL;
+
+        log_info("/* %s */", __func__);
+
+        assert_se(dir_is_empty_at(AT_FDCWD, "/proc") == 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, "/icertainlydontexistdoi") == -ENOENT);
+
+        assert_se(mkdtemp_malloc("/tmp/emptyXXXXXX", &empty_dir) >= 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) > 0);
+
+        j = path_join(empty_dir, "zzz");
+        assert_se(j);
+        assert_se(touch(j) >= 0);
+
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) == 0);
+
+        jj = path_join(empty_dir, "ppp");
+        assert_se(jj);
+        assert_se(touch(jj) >= 0);
+
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) == 0);
+        assert_se(unlink(j) >= 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) == 0);
+        assert_se(unlink(jj) >= 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) > 0);
+}
+
 int main(int argc, char *argv[]) {
         log_show_color(true);
         test_setup_logging(LOG_INFO);
@@ -235,6 +266,7 @@ int main(int argc, char *argv[]) {
         test_fd_is_ns();
         test_device_major_minor_valid();
         test_device_path_make_canonical();
+        test_dir_is_empty();
 
         return 0;
 }