fs-util: rewrite rmdir_parents() with path_find_last_component()

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 165dffbefa8357f326d513285b091e16e833369c..6c3ef972fef6ac0f9bbe2e2ce24cd167e99fa375 100644 (file)
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -45,50 +45,39 @@ int unlink_noerrno(const char *path) {
 }
 
 int rmdir_parents(const char *path, const char *stop) {
-        size_t l;
-        int r = 0;
+        char *p;
+        int r;
 
         assert(path);
         assert(stop);
 
-        l = strlen(path);
-
-        /* Skip trailing slashes */
-        while (l > 0 && path[l-1] == '/')
-                l--;
+        if (!path_is_safe(path))
+                return -EINVAL;
 
-        while (l > 0) {
-                char *t;
+        if (!path_is_safe(stop))
+                return -EINVAL;
 
-                /* Skip last component */
-                while (l > 0 && path[l-1] != '/')
-                        l--;
+        p = strdupa(path);
 
-                /* Skip trailing slashes */
-                while (l > 0 && path[l-1] == '/')
-                        l--;
+        for (;;) {
+                char *slash = NULL;
 
-                if (l <= 0)
-                        break;
+                /* skip the last component. */
+                r = path_find_last_component(p, /* accept_dot_dot= */ false, (const char **) &slash, NULL);
+                if (r <= 0)
+                        return r;
+                if (slash == p)
+                        return 0;
 
-                t = strndup(path, l);
-                if (!t)
-                        return -ENOMEM;
+                assert(*slash == '/');
+                *slash = '\0';
 
-                if (path_startswith(stop, t)) {
-                        free(t);
+                if (path_startswith_full(stop, p, /* accept_dot_dot= */ false))
                         return 0;
-                }
 
-                r = rmdir(t);
-                free(t);
-
-                if (r < 0)
-                        if (errno != ENOENT)
-                                return -errno;
+                if (rmdir(p) < 0 && errno != ENOENT)
+                        return -errno;
         }
-
-        return 0;
 }
 
 int rename_noreplace(int olddirfd, const char *oldpath, int newdirfd, const char *newpath) {

diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index 08bebcf0e81ccce965c78595a56557c63561a1c9..ac7ee5d3bf75fcf67403429ce064ff2d4c213077 100644 (file)
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -865,6 +865,60 @@ static void test_conservative_rename(void) {
         assert_se(access(q, F_OK) < 0 && errno == ENOENT);
 }
 
+static void test_rmdir_parents_one(
+                const char *prefix,
+                const char *path,
+                const char *stop,
+                int expected,
+                const char *test_exist,
+                const char *test_nonexist_subdir) {
+
+        const char *p, *s;
+
+        log_debug("/* %s(%s, %s) */", __func__, path, stop);
+
+        p = strjoina(prefix, path);
+        s = strjoina(prefix, stop);
+
+        if (expected >= 0)
+                assert_se(mkdir_parents(p, 0700) >= 0);
+
+        assert_se(rmdir_parents(p, s) == expected);
+
+        if (expected >= 0) {
+                const char *e, *f;
+
+                e = strjoina(prefix, test_exist);
+                f = strjoina(e, test_nonexist_subdir);
+
+                assert_se(access(e, F_OK) >= 0);
+                assert_se(access(f, F_OK) < 0);
+        }
+}
+
+static void test_rmdir_parents(void) {
+        char *temp;
+
+        log_info("/* %s */", __func__);
+
+        temp = strjoina(arg_test_dir ?: "/tmp", "/test-rmdir.XXXXXX");
+        assert_se(mkdtemp(temp));
+
+        test_rmdir_parents_one(temp, "/aaa/../hoge/foo", "/hoge/foo", -EINVAL, NULL, NULL);
+        test_rmdir_parents_one(temp, "/aaa/bbb/ccc", "/hoge/../aaa", -EINVAL, NULL, NULL);
+
+        test_rmdir_parents_one(temp, "/aaa/bbb/ccc/ddd/eee", "/aaa/bbb/ccc/ddd", 0, "/aaa/bbb/ccc/ddd", "/eee");
+        test_rmdir_parents_one(temp, "/aaa/bbb/ccc/ddd/eee", "/aaa/bbb/ccc", 0, "/aaa/bbb/ccc", "/ddd");
+        test_rmdir_parents_one(temp, "/aaa/bbb/ccc/ddd/eee", "/aaa/bbb", 0, "/aaa/bbb", "/ccc");
+        test_rmdir_parents_one(temp, "/aaa/bbb/ccc/ddd/eee", "/aaa", 0, "/aaa", "/bbb");
+        test_rmdir_parents_one(temp, "/aaa/bbb/ccc/ddd/eee", "/", 0, "/", "/aaa");
+
+        test_rmdir_parents_one(temp, "/aaa/bbb/ccc/ddd/eee", "/aaa/hoge/foo", 0, "/aaa", "/bbb");
+        test_rmdir_parents_one(temp, "/aaa////bbb/.//ccc//ddd/eee///./.", "///././aaa/.", 0, "/aaa", "/bbb");
+
+        assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);
+}
+
 int main(int argc, char *argv[]) {
         test_setup_logging(LOG_INFO);
 
@@ -883,6 +937,7 @@ int main(int argc, char *argv[]) {
         test_rename_noreplace();
         test_chmod_and_chown();
         test_conservative_rename();
+        test_rmdir_parents();
 
         return 0;
 }