path-util: make path_compare() and path_hash_func() ignore "."

This also makes path_compare() may return arbitrary integer as it now
simply pass the result of strcmp() or memcmp().

This changes the behavior of path_extract_filename/directory() when
e.g. "/." or "/./" are input. But the change should be desired.

diff --git a/src/basic/hash-funcs.c b/src/basic/hash-funcs.c
index 608131a1c1467aecf103fdb97f6c3c1a311c88c6..084ed0c0a2afb9e8efb86d06a5bb36dcd5c39a38 100644 (file)
--- a/src/basic/hash-funcs.c
+++ b/src/basic/hash-funcs.c
@@ -17,7 +17,7 @@ DEFINE_HASH_OPS_FULL(string_hash_ops_free_free,
                      void, free);
 
 void path_hash_func(const char *q, struct siphash *state) {
-        size_t n;
+        bool add_slash = false;
 
         assert(q);
         assert(state);
@@ -27,30 +27,31 @@ void path_hash_func(const char *q, struct siphash *state) {
          * similar checks and also doesn't care for trailing slashes. Note that relative and absolute paths (i.e. those
          * which begin in a slash or not) will hash differently though. */
 
-        n = strspn(q, "/");
-        if (n > 0) { /* Eat up initial slashes, and add one "/" to the hash for all of them */
-                siphash24_compress(q, 1, state);
-                q += n;
-        }
+        /* if path is absolute, add one "/" to the hash. */
+        if (path_is_absolute(q))
+                siphash24_compress("/", 1, state);
 
         for (;;) {
-                /* Determine length of next component */
-                n = strcspn(q, "/");
-                if (n == 0) /* Reached the end? */
-                        break;
-
-                /* Add this component to the hash and skip over it */
-                siphash24_compress(q, n, state);
-                q += n;
-
-                /* How many slashes follow this component? */
-                n = strspn(q, "/");
-                if (q[n] == 0) /* Is this a trailing slash? If so, we are at the end, and don't care about the slashes anymore */
-                        break;
-
-                /* We are not add the end yet. Hash exactly one slash for all of the ones we just encountered. */
-                siphash24_compress(q, 1, state);
-                q += n;
+                const char *e;
+                int r;
+
+                r = path_find_first_component(&q, true, &e);
+                if (r == 0)
+                        return;
+
+                if (add_slash)
+                        siphash24_compress_byte('/', state);
+
+                if (r < 0) {
+                        /* if a component is invalid, then add remaining part as a string. */
+                        string_hash_func(q, state);
+                        return;
+                }
+
+                /* Add this component to the hash. */
+                siphash24_compress(e, r, state);
+
+                add_slash = true;
         }
 }
 

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 6cfad9efe6307a19e0fc0c373bc222c640e26ec4..ef2ba44062154d17a7f0f916026340e3315d3485 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -474,7 +474,7 @@ char *path_startswith_full(const char *path, const char *prefix, bool accept_dot
 }
 
 int path_compare(const char *a, const char *b) {
-        int d;
+        int r;
 
         assert(a);
         assert(b);
@@ -482,40 +482,45 @@ int path_compare(const char *a, const char *b) {
         /* A relative path and an absolute path must not compare as equal.
          * Which one is sorted before the other does not really matter.
          * Here a relative path is ordered before an absolute path. */
-        d = (a[0] == '/') - (b[0] == '/');
-        if (d != 0)
-                return d;
+        r = CMP(path_is_absolute(a), path_is_absolute(b));
+        if (r != 0)
+                return r;
 
         for (;;) {
-                size_t j, k;
+                const char *aa, *bb;
+                int j, k;
 
-                a += strspn(a, "/");
-                b += strspn(b, "/");
+                j = path_find_first_component(&a, true, &aa);
+                k = path_find_first_component(&b, true, &bb);
 
-                if (*a == 0 && *b == 0)
-                        return 0;
+                if (j < 0 || k < 0) {
+                        /* When one of paths is invalid, order invalid path after valid one. */
+                        r = CMP(j < 0, k < 0);
+                        if (r != 0)
+                                return r;
+
+                        /* fallback to use strcmp() if both paths are invalid. */
+                        return strcmp(a, b);
+                }
 
                 /* Order prefixes first: "/foo" before "/foo/bar" */
-                if (*a == 0)
+                if (j == 0) {
+                        if (k == 0)
+                                return 0;
                         return -1;
-                if (*b == 0)
+                }
+                if (k == 0)
                         return 1;
 
-                j = strcspn(a, "/");
-                k = strcspn(b, "/");
-
                 /* Alphabetical sort: "/foo/aaa" before "/foo/b" */
-                d = memcmp(a, b, MIN(j, k));
-                if (d != 0)
-                        return (d > 0) - (d < 0); /* sign of d */
+                r = memcmp(aa, bb, MIN(j, k));
+                if (r != 0)
+                        return r;
 
                 /* Sort "/foo/a" before "/foo/aaa" */
-                d = (j > k) - (j < k);  /* sign of (j - k) */
-                if (d != 0)
-                        return d;
-
-                a += j;
-                b += k;
+                r = CMP(j, k);
+                if (r != 0)
+                        return r;
         }
 }
 

diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c
index 9ed6bee9da178c7db1c37f1e65db424852404b05..b9f20fbd94081f7c419083087cd616f76787d60d 100644 (file)
--- a/src/test/test-hashmap-plain.c
+++ b/src/test/test-hashmap-plain.c
@@ -1014,8 +1014,11 @@ static void test_path_hashmap(void) {
         assert_se(hashmap_put(h, "//foo", INT_TO_PTR(3)) == -EEXIST);
         assert_se(hashmap_put(h, "//foox/", INT_TO_PTR(4)) >= 0);
         assert_se(hashmap_put(h, "/foox////", INT_TO_PTR(5)) == -EEXIST);
+        assert_se(hashmap_put(h, "//././/foox//.//.", INT_TO_PTR(5)) == -EEXIST);
         assert_se(hashmap_put(h, "foo//////bar/quux//", INT_TO_PTR(6)) >= 0);
         assert_se(hashmap_put(h, "foo/bar//quux/", INT_TO_PTR(8)) == -EEXIST);
+        assert_se(hashmap_put(h, "foo./ba.r//.quux/", INT_TO_PTR(9)) >= 0);
+        assert_se(hashmap_put(h, "foo./ba.r//.//.quux///./", INT_TO_PTR(10)) == -EEXIST);
 
         assert_se(hashmap_get(h, "foo") == INT_TO_PTR(1));
         assert_se(hashmap_get(h, "foo/") == INT_TO_PTR(1));
@@ -1024,12 +1027,14 @@ static void test_path_hashmap(void) {
         assert_se(hashmap_get(h, "//foo") == INT_TO_PTR(2));
         assert_se(hashmap_get(h, "/////foo////") == INT_TO_PTR(2));
         assert_se(hashmap_get(h, "/////foox////") == INT_TO_PTR(4));
+        assert_se(hashmap_get(h, "/.///./foox//.//") == INT_TO_PTR(4));
         assert_se(hashmap_get(h, "/foox/") == INT_TO_PTR(4));
         assert_se(hashmap_get(h, "/foox") == INT_TO_PTR(4));
         assert_se(!hashmap_get(h, "foox"));
         assert_se(hashmap_get(h, "foo/bar/quux") == INT_TO_PTR(6));
         assert_se(hashmap_get(h, "foo////bar////quux/////") == INT_TO_PTR(6));
         assert_se(!hashmap_get(h, "/foo////bar////quux/////"));
+        assert_se(hashmap_get(h, "foo././//ba.r////.quux///.//.") == INT_TO_PTR(9));
 }
 
 static void test_string_strv_hashmap(void) {

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 6db4f07d90b28531e7543e84b08b0c954f060724..72c18c1b387f44833f32d7d76137ac287061bfb8 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -21,13 +21,6 @@ static void test_print_paths(void) {
         log_info("DEFAULT_USER_PATH=%s", DEFAULT_USER_PATH);
 }
 
-#define test_path_compare(a, b, result) {                 \
-                assert_se(path_compare(a, b) == result);  \
-                assert_se(path_compare(b, a) == -result); \
-                assert_se(path_equal(a, b) == !result);   \
-                assert_se(path_equal(b, a) == !result);   \
-        }
-
 static void test_path_simplify(const char *in, const char *out, const char *out_dot) {
         char *p;
 
@@ -43,30 +36,6 @@ static void test_path_simplify(const char *in, const char *out, const char *out_
 static void test_path(void) {
         log_info("/* %s */", __func__);
 
-        test_path_compare("/goo", "/goo", 0);
-        test_path_compare("/goo", "/goo", 0);
-        test_path_compare("//goo", "/goo", 0);
-        test_path_compare("//goo/////", "/goo", 0);
-        test_path_compare("goo/////", "goo", 0);
-
-        test_path_compare("/goo/boo", "/goo//boo", 0);
-        test_path_compare("//goo/boo", "/goo/boo//", 0);
-
-        test_path_compare("/", "///", 0);
-
-        test_path_compare("/x", "x/", 1);
-        test_path_compare("x/", "/", -1);
-
-        test_path_compare("/x/./y", "x/y", 1);
-        test_path_compare("x/.y", "x/y", -1);
-
-        test_path_compare("foo", "/foo", -1);
-        test_path_compare("/foo", "/foo/bar", -1);
-        test_path_compare("/foo/aaa", "/foo/b", -1);
-        test_path_compare("/foo/aaa", "/foo/b/a", -1);
-        test_path_compare("/foo/a", "/foo/aaa", -1);
-        test_path_compare("/foo/a/b", "/foo/aaa", -1);
-
         assert_se(path_is_absolute("/"));
         assert_se(!path_is_absolute("./"));
 
@@ -120,6 +89,45 @@ static void test_path(void) {
         assert_se(!path_equal_filename("/b", "/c"));
 }
 
+static void test_path_compare_one(const char *a, const char *b, int expected) {
+        int r;
+
+        assert_se(path_compare(a, a) == 0);
+        assert_se(path_compare(b, b) == 0);
+
+        r = path_compare(a, b);
+        assert_se((r > 0) == (expected > 0) && (r < 0) == (expected < 0));
+        r = path_compare(b, a);
+        assert_se((r < 0) == (expected > 0) && (r > 0) == (expected < 0));
+
+        assert_se(path_equal(a, a) == 1);
+        assert_se(path_equal(b, b) == 1);
+        assert_se(path_equal(a, b) == (expected == 0));
+        assert_se(path_equal(b, a) == (expected == 0));
+}
+
+static void test_path_compare(void) {
+        test_path_compare_one("/goo", "/goo", 0);
+        test_path_compare_one("/goo", "/goo", 0);
+        test_path_compare_one("//goo", "/goo", 0);
+        test_path_compare_one("//goo/////", "/goo", 0);
+        test_path_compare_one("goo/////", "goo", 0);
+        test_path_compare_one("/goo/boo", "/goo//boo", 0);
+        test_path_compare_one("//goo/boo", "/goo/boo//", 0);
+        test_path_compare_one("//goo/././//./boo//././//", "/goo/boo//.", 0);
+        test_path_compare_one("/.", "//.///", 0);
+        test_path_compare_one("/x", "x/", 1);
+        test_path_compare_one("x/", "/", -1);
+        test_path_compare_one("/x/./y", "x/y", 1);
+        test_path_compare_one("x/.y", "x/y", -1);
+        test_path_compare_one("foo", "/foo", -1);
+        test_path_compare_one("/foo", "/foo/bar", -1);
+        test_path_compare_one("/foo/aaa", "/foo/b", -1);
+        test_path_compare_one("/foo/aaa", "/foo/b/a", -1);
+        test_path_compare_one("/foo/a", "/foo/aaa", -1);
+        test_path_compare_one("/foo/a/b", "/foo/aaa", -1);
+}
+
 static void test_path_equal_root(void) {
         /* Nail down the details of how path_equal("/", ...) works. */
 
@@ -128,7 +136,7 @@ static void test_path_equal_root(void) {
         assert_se(path_equal("/", "/"));
         assert_se(path_equal("/", "//"));
 
-        assert_se(!path_equal("/", "/./"));
+        assert_se(path_equal("/", "/./"));
         assert_se(!path_equal("/", "/../"));
 
         assert_se(!path_equal("/", "/.../"));
@@ -717,7 +725,7 @@ static void test_path_extract_filename(void) {
         test_path_extract_filename_one("/..", NULL, -EINVAL);
         test_path_extract_filename_one("../", NULL, -EINVAL);
         test_path_extract_filename_one(".", NULL, -EINVAL);
-        test_path_extract_filename_one("/.", NULL, -EINVAL);
+        test_path_extract_filename_one("/.", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one("./", NULL, -EINVAL);
 }
 
@@ -775,7 +783,7 @@ static void test_path_extract_directory(void) {
         test_path_extract_directory_one("/..", "/", 0);
         test_path_extract_directory_one("../", NULL, -EDESTADDRREQ);
         test_path_extract_directory_one(".", NULL, -EDESTADDRREQ);
-        test_path_extract_directory_one("/.", "/", 0);
+        test_path_extract_directory_one("/.", NULL, -EADDRNOTAVAIL);
         test_path_extract_directory_one("./", NULL, -EDESTADDRREQ);
 }
 
@@ -973,6 +981,7 @@ int main(int argc, char **argv) {
 
         test_print_paths();
         test_path();
+        test_path_compare();
         test_path_equal_root();
         test_find_executable_full();
         test_find_executable(argv[0]);