stat-util: fix dir_is_empty() with hidden/backup files

This is a follow-up for f470cb6d13558fc06131dc677d54a089a0b07359 which in
turn is a follow-up for a068aceafbffcba85398cce636c25d659265087a.

The latter started to honour hidden files when deciding whether a
directory is empty. The former reverted to the old behaviour to fix
issue #23220.

It introduced a bug though: when a directory contains a larger number of
hidden entries the getdents64() buffer will not suffice to read them,
since we just allocate three entries for it (which is definitely enough
if we just ignore the . + .. entries, but not ig we ignore more).

I think it's a bit confusing that dir_is_empty() can return true even if
rmdir() on the dir would return ENOTEMPTY. Hence, let's rework the
function to make it optional whether hidden files are ignored or not.
After all, I looking at the users of this function I am pretty sure in
more cases we want to honour hidden files.

diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c
index a7812714aa90c3bdb8a29d15e00e8aedc2917dc9..64c2f80f3c3ff6b1a6b69e2f46ac6406bbe4d362 100644 (file)
--- a/src/basic/stat-util.c
+++ b/src/basic/stat-util.c
@@ -71,13 +71,10 @@ int is_device_node(const char *path) {
         return !!(S_ISBLK(info.st_mode) || S_ISCHR(info.st_mode));
 }
 
-int dir_is_empty_at(int dir_fd, const char *path) {
+int dir_is_empty_at(int dir_fd, const char *path, bool ignore_hidden_or_backup) {
         _cleanup_close_ int fd = -1;
-        /* 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;
+        struct dirent *buf;
+        size_t m;
 
         if (path) {
                 assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
@@ -99,15 +96,30 @@ int dir_is_empty_at(int dir_fd, const char *path) {
                         return fd;
         }
 
-        n = getdents64(fd, &buffer, sizeof(buffer));
-        if (n < 0)
-                return -errno;
+        /* 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. If
+         * 'ignore_hidden_or_backup' is true we'll allocate a bit more, since we might skip over a bunch of
+         * entries that we end up ignoring. */
+        m = (ignore_hidden_or_backup ? 16 : 3) * DIRENT_SIZE_MAX;
+        buf = alloca(m);
+
+        for (;;) {
+                struct dirent *de;
+                ssize_t n;
+
+                n = getdents64(fd, buf, m);
+                if (n < 0)
+                        return -errno;
+                if (n == 0)
+                        break;
 
-        msan_unpoison(&buffer, n);
+                assert((size_t) n <= m);
+                msan_unpoison(buf, n);
 
-        FOREACH_DIRENT_IN_BUFFER(de, &buffer.de, n)
-                if (!hidden_or_backup_file(de->d_name))
-                        return 0;
+                FOREACH_DIRENT_IN_BUFFER(de, buf, n)
+                        if (!(ignore_hidden_or_backup ? hidden_or_backup_file(de->d_name) : dot_or_dot_dot(de->d_name)))
+                                return 0;
+        }
 
         return 1;
 }

diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h
index 4483ceb7de7665f7f751e4ca046b47082d9ea18b..ce3c8cccfecdb037c060853a278e38a242f5a405 100644 (file)
--- a/src/basic/stat-util.h
+++ b/src/basic/stat-util.h
@@ -17,14 +17,14 @@ int is_dir(const char *path, bool follow);
 int is_dir_fd(int fd);
 int is_device_node(const char *path);
 
-int dir_is_empty_at(int dir_fd, const char *path);
-static inline int dir_is_empty(const char *path) {
-        return dir_is_empty_at(AT_FDCWD, path);
+int dir_is_empty_at(int dir_fd, const char *path, bool ignore_hidden_or_backup);
+static inline int dir_is_empty(const char *path, bool ignore_hidden_or_backup) {
+        return dir_is_empty_at(AT_FDCWD, path, ignore_hidden_or_backup);
 }
 
-static inline int dir_is_populated(const char *path) {
+static inline int dir_is_populated(const char *path, bool ignore_hidden_or_backup) {
         int r;
-        r = dir_is_empty(path);
+        r = dir_is_empty(path, ignore_hidden_or_backup);
         if (r < 0)
                 return r;
         return !r;

diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c
index 69703a74c1ac321f493a6b91b8186c54ff65b0a0..058dac5381dabd0e93d57a4f6452392d53b28576 100644 (file)
--- a/src/boot/bootctl.c
+++ b/src/boot/bootctl.c
@@ -1639,7 +1639,7 @@ static int are_we_installed(const char *esp_path) {
                 return log_oom();
 
         log_debug("Checking whether %s contains any files…", p);
-        r = dir_is_empty(p);
+        r = dir_is_empty(p, /* ignore_hidden_or_backup= */ false);
         if (r < 0 && r != -ENOENT)
                 return log_error_errno(r, "Failed to check whether %s contains any files: %m", p);
 

diff --git a/src/core/main.c b/src/core/main.c
index 5fa404fc668fca8705da4f6a7d7bf456a87cc98e..409b84a0064c258135933a7f8f378b839059abf0 100644 (file)
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1290,7 +1290,7 @@ static void test_usr(void) {
 
         /* Check that /usr is either on the same file system as / or mounted already. */
 
-        if (dir_is_empty("/usr") <= 0)
+        if (dir_is_empty("/usr", /* ignore_hidden_or_backup= */ false) <= 0)
                 return;
 
         log_warning("/usr appears to be on its own filesystem and is not already mounted. This is not a supported setup. "

diff --git a/src/core/manager.c b/src/core/manager.c
index 18daff66c78064d81572d191681b5a73be28103f..d09d14911c353e39e40b8a274353fac196809a8c 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -976,7 +976,7 @@ int manager_new(LookupScope scope, ManagerTestRunFlags test_run_flags, Manager *
 
         m->taint_usr =
                 !in_initrd() &&
-                dir_is_empty("/usr") > 0;
+                dir_is_empty("/usr", /* ignore_hidden_or_backup= */ false) > 0;
 
         /* Note that we do not set up the notify fd here. We do that after deserialization,
          * since they might have gotten serialized across the reexec. */

diff --git a/src/core/path.c b/src/core/path.c
index 08143912221b14b4d5495c84b0000c467af2bf15..69bbddf1585cba8f310020517bace9aa4882abe1 100644 (file)
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -213,7 +213,7 @@ static bool path_spec_check_good(PathSpec *s, bool initial, bool from_trigger_no
         case PATH_DIRECTORY_NOT_EMPTY: {
                 int k;
 
-                k = dir_is_empty(s->path);
+                k = dir_is_empty(s->path, /* ignore_hidden_or_backup= */ true);
                 good = !(IN_SET(k, -ENOENT, -ENOTDIR) || k > 0);
                 break;
         }

diff --git a/src/core/unit.c b/src/core/unit.c
index ff1288dcac0198b414e457e1e98fad0252f0e0e7..42ee8892a4f45a040c61af30085f4bea82e9356b 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -4776,7 +4776,7 @@ void unit_warn_if_dir_nonempty(Unit *u, const char* where) {
         if (!unit_log_level_test(u, LOG_NOTICE))
                 return;
 
-        r = dir_is_empty(where);
+        r = dir_is_empty(where, /* ignore_hidden_or_backup= */ false);
         if (r > 0 || r == -ENOTDIR)
                 return;
         if (r < 0) {

diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c
index cd4b05c52d49eacc5e42c4e7b38f0e1bc31085c8..589a2cc5822b90441f6447f4bb1c477137f9b93e 100644 (file)
--- a/src/gpt-auto-generator/gpt-auto-generator.c
+++ b/src/gpt-auto-generator/gpt-auto-generator.c
@@ -305,7 +305,7 @@ static int path_is_busy(const char *where) {
                 return log_warning_errno(r, "Cannot check if \"%s\" is a mount point: %m", where);
 
         /* not a mountpoint but it contains files */
-        r = dir_is_empty(where);
+        r = dir_is_empty(where, /* ignore_hidden_or_backup= */ false);
         if (r < 0)
                 return log_warning_errno(r, "Cannot check if \"%s\" is empty: %m", where);
         if (r > 0)

diff --git a/src/home/homework-cifs.c b/src/home/homework-cifs.c
index 6d499f76b2442740007369e2ef8e8d789c6903fc..728a92260cfdb9253eca1fdd37a5b76679255bc8 100644 (file)
--- a/src/home/homework-cifs.c
+++ b/src/home/homework-cifs.c
@@ -207,7 +207,7 @@ int home_create_cifs(UserRecord *h, HomeSetup *setup, UserRecord **ret_home) {
         if (r < 0)
                 return r;
 
-        r = dir_is_empty_at(setup->root_fd, NULL);
+        r = dir_is_empty_at(setup->root_fd, NULL, /* ignore_hidden_or_backup= */ false);
         if (r < 0)
                 return log_error_errno(r, "Failed to detect if CIFS directory is empty: %m");
         if (r == 0)

diff --git a/src/home/user-record-util.c b/src/home/user-record-util.c
index 2b727df533b2215915fb17d088d761c89daa86e1..9d85f2abfa55a4e83d143a5ee76542e524890d47 100644 (file)
--- a/src/home/user-record-util.c
+++ b/src/home/user-record-util.c
@@ -445,7 +445,7 @@ int user_record_test_home_directory(UserRecord *h) {
         }
 
         /* Otherwise it's not OK */
-        r = dir_is_empty(hd);
+        r = dir_is_empty(hd, /* ignore_hidden_or_backup= */ false);
         if (r < 0)
                 return r;
         if (r == 0)

diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c
index fab4eb9609a7f7c924592c4221f789b3f255ec42..f3d7f403f91b2d545fae2aef06a4f8d7148c719b 100644 (file)
--- a/src/nspawn/nspawn-network.c
+++ b/src/nspawn/nspawn-network.c
@@ -449,7 +449,7 @@ int remove_bridge(const char *bridge_name) {
 
         path = strjoina("/sys/class/net/", bridge_name, "/brif");
 
-        r = dir_is_empty(path);
+        r = dir_is_empty(path, /* ignore_hidden_or_backup= */ false);
         if (r == -ENOENT) /* Already gone? */
                 return 0;
         if (r < 0)

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 59768bcc0b8d4c32da07f5d248cec4f91adb3a46..c5fd9783952882080edb8b049dc1f15e89f598f0 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2680,7 +2680,7 @@ static int setup_journal(const char *directory) {
         } else if (access(p, F_OK) < 0)
                 return 0;
 
-        if (dir_is_empty(q) == 0)
+        if (dir_is_empty(q, /* ignore_hidden_or_backup= */ false) == 0)
                 log_warning("%s is not empty, proceeding anyway.", q);
 
         r = userns_mkdir(directory, p, 0755, 0, 0);

diff --git a/src/shared/condition.c b/src/shared/condition.c
index 1d87ed53ce7b1e5cc6220d597810453859032ff9..67de6fc62f1bfaad5ac3a9fb68663c542b4cd6c5 100644 (file)
--- a/src/shared/condition.c
+++ b/src/shared/condition.c
@@ -922,7 +922,7 @@ static int condition_test_directory_not_empty(Condition *c, char **env) {
         assert(c->parameter);
         assert(c->type == CONDITION_DIRECTORY_NOT_EMPTY);
 
-        r = dir_is_empty(c->parameter);
+        r = dir_is_empty(c->parameter, /* ignore_hidden_or_backup= */ true);
         return r <= 0 && !IN_SET(r, -ENOENT, -ENOTDIR);
 }
 

diff --git a/src/shared/copy.c b/src/shared/copy.c
index 0043c4ff72aceb56e8e9747ca19be2c34fdc61d1..4d52b4c26b8eb3fa4095b449820876a73144a9de 100644 (file)
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -905,7 +905,7 @@ static int fd_copy_directory(
 
         exists = false;
         if (copy_flags & COPY_MERGE_EMPTY) {
-                r = dir_is_empty_at(dt, to);
+                r = dir_is_empty_at(dt, to, /* ignore_hidden_or_backup= */ false);
                 if (r < 0 && r != -ENOENT)
                         return r;
                 else if (r == 1)

diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c
index 25fed4cf508ea0ca8d6dd8fabf574cf788cf86cc..057fe14125ccefab8c0cb272493ee1f2dcf39545 100644 (file)
--- a/src/shared/dissect-image.c
+++ b/src/shared/dissect-image.c
@@ -1552,7 +1552,7 @@ int dissected_image_mount(
                                 if (r < 0) {
                                         if (r != -ENOENT)
                                                 return r;
-                                } else if (dir_is_empty(p) > 0) {
+                                } else if (dir_is_empty(p, /* ignore_hidden_or_backup= */ false) > 0) {
                                         /* It exists and is an empty directory. Let's mount the ESP there. */
                                         r = mount_partition(m->partitions + PARTITION_ESP, where, "/boot", uid_shift, uid_range, flags);
                                         if (r < 0)

diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c
index 2d1bc7cf465c4eb1647cda37942edd566e4efeb0..84120000aaec6baf3754361da333abb365aa6153 100644 (file)
--- a/src/shared/tpm2-util.c
+++ b/src/shared/tpm2-util.c
@@ -1466,7 +1466,7 @@ Tpm2Support tpm2_support(void) {
                  * got the host sysfs mounted. Since devices are generally not virtualized for containers,
                  * let's assume containers never have a TPM, at least for now. */
 
-                r = dir_is_empty("/sys/class/tpmrm");
+                r = dir_is_empty("/sys/class/tpmrm", /* ignore_hidden_or_backup= */ false);
                 if (r < 0) {
                         if (r != -ENOENT)
                                 log_debug_errno(r, "Unable to test whether /sys/class/tpmrm/ exists and is populated, assuming it is not: %m");

diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c
index 20fbb916e9b48c22772f6f2fcf715f98b872fe30..76c2fe69786fe1b5e233ad44100d175c7ac8d7ae 100644 (file)
--- a/src/sysext/sysext.c
+++ b/src/sysext/sysext.c
@@ -296,7 +296,7 @@ static int merge_hierarchy(
         else if (r < 0)
                 return log_error_errno(r, "Failed to resolve host hierarchy '%s': %m", hierarchy);
         else {
-                r = dir_is_empty(resolved_hierarchy);
+                r = dir_is_empty(resolved_hierarchy, /* ignore_hidden_or_backup= */ false);
                 if (r < 0)
                         return log_error_errno(r, "Failed to check if host hierarchy '%s' is empty: %m", resolved_hierarchy);
                 if (r > 0) {
@@ -337,7 +337,7 @@ static int merge_hierarchy(
                 if (r < 0)
                         return log_error_errno(r, "Failed to resolve hierarchy '%s' in extension '%s': %m", hierarchy, *p);
 
-                r = dir_is_empty(resolved);
+                r = dir_is_empty(resolved, /* ignore_hidden_or_backup= */ false);
                 if (r < 0)
                         return log_error_errno(r, "Failed to check if hierarchy '%s' in extension '%s' is empty: %m", resolved, *p);
                 if (r > 0) {

diff --git a/src/test/test-stat-util.c b/src/test/test-stat-util.c
index a8fdbb7c18300d69f1055a82baff99937696d19a..04d52d364fd857fcaadbb09c6571fa22fe6b16bf 100644 (file)
--- a/src/test/test-stat-util.c
+++ b/src/test/test-stat-util.c
@@ -153,17 +153,17 @@ TEST(dir_is_empty) {
         _cleanup_(rm_rf_physical_and_freep) char *empty_dir = NULL;
         _cleanup_free_ char *j = NULL, *jj = NULL, *jjj = NULL;
 
-        assert_se(dir_is_empty_at(AT_FDCWD, "/proc") == 0);
-        assert_se(dir_is_empty_at(AT_FDCWD, "/icertainlydontexistdoi") == -ENOENT);
+        assert_se(dir_is_empty_at(AT_FDCWD, "/proc", /* ignore_hidden_or_backup= */ true) == 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, "/icertainlydontexistdoi", /* ignore_hidden_or_backup= */ true) == -ENOENT);
 
         assert_se(mkdtemp_malloc("/tmp/emptyXXXXXX", &empty_dir) >= 0);
-        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) > 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ true) > 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);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ true) == 0);
 
         jj = path_join(empty_dir, "ppp");
         assert_se(jj);
@@ -173,13 +173,17 @@ TEST(dir_is_empty) {
         assert_se(jjj);
         assert_se(touch(jjj) >= 0);
 
-        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) == 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ true) == 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ false) == 0);
         assert_se(unlink(j) >= 0);
-        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) == 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ true) == 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ false) == 0);
         assert_se(unlink(jj) >= 0);
-        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) > 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ true) > 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ false) == 0);
         assert_se(unlink(jjj) >= 0);
-        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) > 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ true) > 0);
+        assert_se(dir_is_empty_at(AT_FDCWD, empty_dir, /* ignore_hidden_or_backup= */ false) > 0);
 }
 
 static int intro(void) {