From: Yu Watanabe Date: Sat, 1 May 2021 21:39:55 +0000 (+0900) Subject: path-util: make path_compare() and path_hash_func() ignore "." X-Git-Tag: v249-rc1~131^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=353df4438ee29068df8f36187ae7e73f889e2120;p=thirdparty%2Fsystemd.git 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 608131a1c14..084ed0c0a2a 100644 --- 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 6cfad9efe63..ef2ba440621 100644 --- 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 9ed6bee9da1..b9f20fbd940 100644 --- 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 6db4f07d90b..72c18c1b387 100644 --- 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]);