path-util: make path_simplify() use path_find_first_component()

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 82d6a28257a6f742bbd05e73a8f5efc8f4707089..2742a276728f54fa71c4b507f265b34fb7edfeaf 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -328,62 +328,52 @@ char **path_strv_resolve_uniq(char **l, const char *root) {
 }
 
 char *path_simplify(char *path) {
-        char *f, *t;
-        bool slash = false, ignore_slash = false, absolute;
+        bool add_slash = false;
+        char *f = path;
+        int r;
 
         assert(path);
 
-        /* Removes redundant inner and trailing slashes. Also removes unnecessary dots
-         * if kill_dots is true. Modifies the passed string in-place.
+        /* Removes redundant inner and trailing slashes. Also removes unnecessary dots.
+         * Modifies the passed string in-place.
          *
-         * ///foo//./bar/.   becomes /foo/./bar/.      (if kill_dots is false)
-         * ///foo//./bar/.   becomes /foo/bar          (if kill_dots is true)
-         * .//./foo//./bar/. becomes ././foo/./bar/.   (if kill_dots is false)
-         * .//./foo//./bar/. becomes foo/bar           (if kill_dots is true)
+         * ///foo//./bar/.   becomes /foo/bar
+         * .//./foo//./bar/. becomes foo/bar
          */
 
         if (isempty(path))
                 return path;
 
-        absolute = path_is_absolute(path);
-
-        f = path;
-        if (*f == '.' && IN_SET(f[1], 0, '/')) {
-                ignore_slash = true;
+        if (path_is_absolute(path))
                 f++;
-        }
 
-        for (t = path; *f; f++) {
+        for (const char *p = f;;) {
+                const char *e;
 
-                if (*f == '/') {
-                        slash = true;
-                        continue;
-                }
+                r = path_find_first_component(&p, true, &e);
+                if (r == 0)
+                        break;
 
-                if (slash) {
-                        if (*f == '.' && IN_SET(f[1], 0, '/'))
-                                continue;
+                if (add_slash)
+                        *f++ = '/';
 
-                        slash = false;
-                        if (ignore_slash)
-                                ignore_slash = false;
-                        else
-                                *(t++) = '/';
+                if (r < 0) {
+                        /* if path is invalid, then refuse to simplify remaining part. */
+                        memmove(f, p, strlen(p) + 1);
+                        return path;
                 }
 
-                *(t++) = *f;
-        }
+                memmove(f, e, r);
+                f += r;
 
-        /* Special rule, if we stripped everything, we either need a "/" (for the root directory)
-         * or "." for the current directory */
-        if (t == path) {
-                if (absolute)
-                        *(t++) = '/';
-                else
-                        *(t++) = '.';
+                add_slash = true;
         }
 
-        *t = 0;
+        /* Special rule, if we stripped everything, we need a "." for the current directory. */
+        if (f == path)
+                *f++ = '.';
+
+        *f = '\0';
         return path;
 }
 

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 8e8802b72f52d02ffc5ff5defde1c1a62636939c..622e38c1bb33bff01e04b4970ae7f3ca7c43bc3d 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -21,15 +21,6 @@ static void test_print_paths(void) {
         log_info("DEFAULT_USER_PATH=%s", DEFAULT_USER_PATH);
 }
 
-static void test_path_simplify(const char *in, const char *out) {
-        char *p;
-
-        log_info("/* %s */", __func__);
-
-        p = strdupa(in);
-        assert_se(streq(path_simplify(p), out));
-}
-
 static void test_path(void) {
         log_info("/* %s */", __func__);
 
@@ -45,25 +36,6 @@ static void test_path(void) {
         assert_se(streq(basename("/aa///file..."), "file..."));
         assert_se(streq(basename("file.../"), ""));
 
-        test_path_simplify("aaa/bbb////ccc", "aaa/bbb/ccc");
-        test_path_simplify("//aaa/.////ccc", "/aaa/ccc");
-        test_path_simplify("///", "/");
-        test_path_simplify("///.//", "/");
-        test_path_simplify("///.//.///", "/");
-        test_path_simplify("////.././///../.", "/../..");
-        test_path_simplify(".", ".");
-        test_path_simplify("./", ".");
-        test_path_simplify(".///.//./.", ".");
-        test_path_simplify(".///.//././/", ".");
-        test_path_simplify("//./aaa///.//./.bbb/..///c.//d.dd///..eeee/.",
-                           "/aaa/.bbb/../c./d.dd/..eeee");
-        test_path_simplify("//./aaa///.//./.bbb/..///c.//d.dd///..eeee/..",
-                           "/aaa/.bbb/../c./d.dd/..eeee/..");
-        test_path_simplify(".//./aaa///.//./.bbb/..///c.//d.dd///..eeee/..",
-                           "aaa/.bbb/../c./d.dd/..eeee/..");
-        test_path_simplify("..//./aaa///.//./.bbb/..///c.//d.dd///..eeee/..",
-                           "../aaa/.bbb/../c./d.dd/..eeee/..");
-
         assert_se(PATH_IN_SET("/bin", "/", "/bin", "/foo"));
         assert_se(PATH_IN_SET("/bin", "/bin"));
         assert_se(PATH_IN_SET("/bin", "/foo/bar", "/bin"));
@@ -82,6 +54,60 @@ static void test_path(void) {
         assert_se(!path_equal_filename("/b", "/c"));
 }
 
+static void test_path_simplify_one(const char *in, const char *out) {
+        char *p;
+
+        p = strdupa(in);
+        path_simplify(p);
+        log_debug("/* test_path_simplify(%s) → %s (expected: %s) */", in, p, out);
+        assert_se(streq(p, out));
+}
+
+static void test_path_simplify(void) {
+        _cleanup_free_ char *hoge = NULL, *hoge_out = NULL;
+        char foo[NAME_MAX * 2];
+
+        log_info("/* %s */", __func__);
+
+        test_path_simplify_one("", "");
+        test_path_simplify_one("aaa/bbb////ccc", "aaa/bbb/ccc");
+        test_path_simplify_one("//aaa/.////ccc", "/aaa/ccc");
+        test_path_simplify_one("///", "/");
+        test_path_simplify_one("///.//", "/");
+        test_path_simplify_one("///.//.///", "/");
+        test_path_simplify_one("////.././///../.", "/../..");
+        test_path_simplify_one(".", ".");
+        test_path_simplify_one("./", ".");
+        test_path_simplify_one(".///.//./.", ".");
+        test_path_simplify_one(".///.//././/", ".");
+        test_path_simplify_one("//./aaa///.//./.bbb/..///c.//d.dd///..eeee/.",
+                               "/aaa/.bbb/../c./d.dd/..eeee");
+        test_path_simplify_one("//./aaa///.//./.bbb/..///c.//d.dd///..eeee/..",
+                               "/aaa/.bbb/../c./d.dd/..eeee/..");
+        test_path_simplify_one(".//./aaa///.//./.bbb/..///c.//d.dd///..eeee/..",
+                               "aaa/.bbb/../c./d.dd/..eeee/..");
+        test_path_simplify_one("..//./aaa///.//./.bbb/..///c.//d.dd///..eeee/..",
+                               "../aaa/.bbb/../c./d.dd/..eeee/..");
+
+        memset(foo, 'a', sizeof(foo) -1);
+        char_array_0(foo);
+
+        test_path_simplify_one(foo, foo);
+
+        hoge = strjoin("/", foo);
+        assert_se(hoge);
+        test_path_simplify_one(hoge, hoge);
+        hoge = mfree(hoge);
+
+        hoge = strjoin("a////.//././//./b///././/./c/////././//./", foo, "//.//////d/e/.//f/");
+        assert_se(hoge);
+
+        hoge_out = strjoin("a/b/c/", foo, "//.//////d/e/.//f/");
+        assert_se(hoge_out);
+
+        test_path_simplify_one(hoge, hoge_out);
+}
+
 static void test_path_compare_one(const char *a, const char *b, int expected) {
         int r;
 
@@ -1057,6 +1083,7 @@ int main(int argc, char **argv) {
 
         test_print_paths();
         test_path();
+        test_path_simplify();
         test_path_compare();
         test_path_equal_root();
         test_find_executable_full();