path-util: use path_find_first_component() in path_make_relative()

This also makes the function checks the result is a valid path or not.

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index aa65e2950fb06068d48016f80939a5566d3a3b17..6cfad9efe6307a19e0fc0c373bc222c640e26ec4 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -107,93 +107,96 @@ int path_make_absolute_cwd(const char *p, char **ret) {
         return 0;
 }
 
-int path_make_relative(const char *from_dir, const char *to_path, char **_r) {
-        char *f, *t, *r, *p;
-        unsigned n_parents = 0;
-
-        assert(from_dir);
-        assert(to_path);
-        assert(_r);
+int path_make_relative(const char *from, const char *to, char **ret) {
+        _cleanup_free_ char *result = NULL;
+        unsigned n_parents;
+        const char *f, *t;
+        int r, k;
+        char *p;
+
+        assert(from);
+        assert(to);
+        assert(ret);
 
         /* Strips the common part, and adds ".." elements as necessary. */
 
-        if (!path_is_absolute(from_dir) || !path_is_absolute(to_path))
+        if (!path_is_absolute(from) || !path_is_absolute(to))
                 return -EINVAL;
 
-        f = strdupa(from_dir);
-        t = strdupa(to_path);
-
-        path_simplify(f, true);
-        path_simplify(t, true);
-
-        /* Skip the common part. */
         for (;;) {
-                size_t a, b;
+                r = path_find_first_component(&from, true, &f);
+                if (r < 0)
+                        return r;
+
+                k = path_find_first_component(&to, true, &t);
+                if (k < 0)
+                        return k;
+
+                if (r == 0) {
+                        /* end of 'from' */
+                        if (k == 0) {
+                                /* from and to are equivalent. */
+                                result = strdup(".");
+                                if (!result)
+                                        return -ENOMEM;
+                        } else {
+                                /* 'to' is inside of 'from'. */
+                                result = strdup(t);
+                                if (!result)
+                                        return -ENOMEM;
 
-                f += *f == '/';
-                t += *t == '/';
+                                path_simplify(result, true);
 
-                if (!*f) {
-                        if (!*t)
-                                /* from_dir equals to_path. */
-                                r = strdup(".");
-                        else
-                                /* from_dir is a parent directory of to_path. */
-                                r = strdup(t);
-                        if (!r)
-                                return -ENOMEM;
+                                if (!path_is_valid(result))
+                                        return -EINVAL;
+                        }
 
-                        *_r = r;
+                        *ret = TAKE_PTR(result);
                         return 0;
                 }
 
-                if (!*t)
-                        break;
-
-                a = strcspn(f, "/");
-                b = strcspn(t, "/");
-
-                if (a != b || memcmp(f, t, a) != 0)
+                if (r != k || !strneq(f, t, r))
                         break;
-
-                f += a;
-                t += b;
         }
 
         /* If we're here, then "from_dir" has one or more elements that need to
          * be replaced with "..". */
 
-        /* Count the number of necessary ".." elements. */
-        for (; *f;) {
-                size_t w;
+        for (n_parents = 1;; n_parents++) {
+                /* If this includes ".." we can't do a simple series of "..". */
+                r = path_find_first_component(&from, false, &f);
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        break;
+        }
 
-                w = strcspn(f, "/");
+        if (isempty(t) && n_parents * 3 > PATH_MAX)
+                /* PATH_MAX is counted *with* the trailing NUL byte */
+                return -EINVAL;
 
-                /* If this includes ".." we can't do a simple series of "..", refuse */
-                if (w == 2 && f[0] == '.' && f[1] == '.')
-                        return -EINVAL;
+        result = new(char, n_parents * 3 + !isempty(t) + strlen_ptr(t));
+        if (!result)
+                return -ENOMEM;
 
-                /* Count number of elements */
-                n_parents++;
+        for (p = result; n_parents > 0; n_parents--)
+                p = mempcpy(p, "../", 3);
 
-                f += w;
-                f += *f == '/';
+        if (isempty(t)) {
+                /* Remove trailing slash and terminate string. */
+                *(--p) = '\0';
+                *ret = TAKE_PTR(result);
+                return 0;
         }
 
-        r = new(char, n_parents * 3 + strlen(t) + 1);
-        if (!r)
-                return -ENOMEM;
+        strcpy(p, t);
 
-        for (p = r; n_parents > 0; n_parents--)
-                p = mempcpy(p, "../", 3);
+        path_simplify(result, true);
 
-        if (*t)
-                strcpy(p, t);
-        else
-                /* Remove trailing slash */
-                *(--p) = 0;
+        if (!path_is_valid(result))
+                return -EINVAL;
 
-        *_r = r;
+        *ret = TAKE_PTR(result);
         return 0;
 }
 

diff --git a/src/basic/path-util.h b/src/basic/path-util.h
index 50a7b021a49330a82d06bfa7f99bf48c53b51166..fad40bd8b6e7e368b151f14a904cbda08a5eb6b2 100644 (file)
--- a/src/basic/path-util.h
+++ b/src/basic/path-util.h
@@ -56,7 +56,7 @@ int path_split_and_make_absolute(const char *p, char ***ret);
 char* path_make_absolute(const char *p, const char *prefix);
 int safe_getcwd(char **ret);
 int path_make_absolute_cwd(const char *p, char **ret);
-int path_make_relative(const char *from_dir, const char *to_path, char **_r);
+int path_make_relative(const char *from, const char *to, char **ret);
 char *path_startswith_full(const char *path, const char *prefix, bool accept_dot_dot) _pure_;
 static inline char* path_startswith(const char *path, const char *prefix) {
         return path_startswith_full(path, prefix, true);

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 49a547422e626fdae1207d12ed84915eb951c95b..6db4f07d90b28531e7543e84b08b0c954f060724 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -420,29 +420,32 @@ static void test_fsck_exists(void) {
         assert_se(fsck_exists("/../bin/") == 0);
 }
 
-static void test_make_relative(void) {
-        char *result;
+static void test_path_make_relative_one(const char *from, const char *to, const char *expected) {
+        _cleanup_free_ char *z = NULL;
+        int r;
 
-        log_info("/* %s */", __func__);
+        log_info("/* %s(%s, %s) */", __func__, from, to);
 
-        assert_se(path_make_relative("some/relative/path", "/some/path", &result) < 0);
-        assert_se(path_make_relative("/some/path", "some/relative/path", &result) < 0);
-        assert_se(path_make_relative("/some/dotdot/../path", "/some/path", &result) < 0);
+        r = path_make_relative(from, to, &z);
+        assert_se((r >= 0) == !!expected);
+        assert_se(streq_ptr(z, expected));
+}
 
-#define test(from_dir, to_path, expected) {                \
-                _cleanup_free_ char *z = NULL;             \
-                path_make_relative(from_dir, to_path, &z); \
-                assert_se(streq(z, expected));             \
-        }
+static void test_make_relative(void) {
+        log_info("/* %s */", __func__);
 
-        test("/", "/", ".");
-        test("/", "/some/path", "some/path");
-        test("/some/path", "/some/path", ".");
-        test("/some/path", "/some/path/in/subdir", "in/subdir");
-        test("/some/path", "/", "../..");
-        test("/some/path", "/some/other/path", "../other/path");
-        test("/some/path/./dot", "/some/further/path", "../../further/path");
-        test("//extra.//.//./.slashes//./won't////fo.ol///anybody//", "/././/extra././/.slashes////ar.e/.just/././.fine///", "../../../ar.e/.just/.fine");
+        test_path_make_relative_one("some/relative/path", "/some/path", NULL);
+        test_path_make_relative_one("/some/path", "some/relative/path", NULL);
+        test_path_make_relative_one("/some/dotdot/../path", "/some/path", NULL);
+
+        test_path_make_relative_one("/", "/", ".");
+        test_path_make_relative_one("/", "/some/path", "some/path");
+        test_path_make_relative_one("/some/path", "/some/path", ".");
+        test_path_make_relative_one("/some/path", "/some/path/in/subdir", "in/subdir");
+        test_path_make_relative_one("/some/path", "/", "../..");
+        test_path_make_relative_one("/some/path", "/some/other/path", "../other/path");
+        test_path_make_relative_one("/some/path/./dot", "/some/further/path", "../../further/path");
+        test_path_make_relative_one("//extra.//.//./.slashes//./won't////fo.ol///anybody//", "/././/extra././/.slashes////ar.e/.just/././.fine///", "../../../ar.e/.just/.fine");
 }
 
 static void test_strv_resolve(void) {