path-util: make path_extract_filename/directory() handle "." gracefully

This makes the functions handle "xx/" and "xx/." as equivalent.
Moreover, now path_extract_directory() returns normalized path, that is
no redundant "/" or "/./" are contained.

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index ab89623c5ab6d6633bea060ca1f33a105d59f647..0be5af8da156a549ec3d133dce0b10cc3bb9e5c1 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -1043,82 +1043,81 @@ const char *last_path_component(const char *path) {
         return path + k;
 }
 
-int path_extract_filename(const char *p, char **ret) {
+int path_extract_filename(const char *path, char **ret) {
         _cleanup_free_ char *a = NULL;
-        const char *c;
-        size_t n;
+        const char *c, *next = NULL;
+        int r;
 
         /* Extracts the filename part (i.e. right-most component) from a path, i.e. string that passes
          * filename_is_valid(). A wrapper around last_path_component(), but eats up trailing
          * slashes. Returns:
          *
-         * -EINVAL        → if the passed in path is not a valid path
-         * -EADDRNOTAVAIL → if only a directory was specified, but no filename, i.e. the root dir itself is specified
+         * -EINVAL        → if the path is not valid
+         * -EADDRNOTAVAIL → if only a directory was specified, but no filename, i.e. the root dir
+         *                  itself or "." is specified
          * -ENOMEM        → no memory
          *
-         * Returns >= 0 on success. If the input path has a trailing slash, returns O_DIRECTORY, to indicate
-         * the referenced file must be a directory.
+         * Returns >= 0 on success. If the input path has a trailing slash, returns O_DIRECTORY, to
+         * indicate the referenced file must be a directory.
          *
          * This function guarantees to return a fully valid filename, i.e. one that passes
          * filename_is_valid() – this means "." and ".." are not accepted. */
 
-        if (!path_is_valid(p))
+        if (!path_is_valid(path))
                 return -EINVAL;
 
-        /* Special case the root dir, because in that case we simply have no filename, but
-         * last_path_component() won't complain */
-        if (path_equal(p, "/"))
+        r = path_find_last_component(path, false, &next, &c);
+        if (r < 0)
+                return r;
+        if (r == 0) /* root directory */
                 return -EADDRNOTAVAIL;
 
-        c = last_path_component(p);
-        n = strcspn(c, "/");
-
-        a = strndup(c, n);
+        a = strndup(c, r);
         if (!a)
                 return -ENOMEM;
 
-        if (!filename_is_valid(a))
-                return -EINVAL;
-
         *ret = TAKE_PTR(a);
-        return c[n] == '/' ? O_DIRECTORY : 0;
+        return strlen(c) > (size_t)r ? O_DIRECTORY : 0;
 }
 
-int path_extract_directory(const char *p, char **ret) {
+int path_extract_directory(const char *path, char **ret) {
         _cleanup_free_ char *a = NULL;
-        const char *c;
+        const char *c, *next = NULL;
+        int r;
 
         /* The inverse of path_extract_filename(), i.e. returns the directory path prefix. Returns:
          *
-         * -EINVAL        → if the passed in path is not a valid path
+         * -EINVAL        → if the path is not valid
          * -EDESTADDRREQ  → if no directory was specified in the passed in path, i.e. only a filename was passed
-         * -EADDRNOTAVAIL → if the passed in parameter had no filename but did have a directory, i.e. the root dir itself was specified
+         * -EADDRNOTAVAIL → if the passed in parameter had no filename but did have a directory, i.e.
+         *                   the root dir itself or "." was specified
          * -ENOMEM        → no memory (surprise!)
          *
          * This function guarantees to return a fully valid path, i.e. one that passes path_is_valid().
          */
 
-        if (!path_is_valid(p))
-                return -EINVAL;
-
-        /* Special case the root dir, because otherwise for an input of "///" last_path_component() returns
-         * the pointer to the last slash only, which might be seen as a valid path below. */
-        if (path_equal(p, "/"))
-                return -EADDRNOTAVAIL;
-
-        c = last_path_component(p);
-
-        /* Delete trailing slashes, but keep one */
-        while (c > p+1 && c[-1] == '/')
-                c--;
-
-        if (p == c) /* No path whatsoever? Then return a recognizable error */
-                return -EDESTADDRREQ;
+        r = path_find_last_component(path, false, &next, &c);
+        if (r < 0)
+                return r;
+        if (r == 0) /* empty or root */
+                return isempty(path) ? -EINVAL : -EADDRNOTAVAIL;
+        if (next == path) {
+                if (*path != '/') /* filename only */
+                        return -EDESTADDRREQ;
+
+                a = strdup("/");
+                if (!a)
+                        return -ENOMEM;
+                *ret = TAKE_PTR(a);
+                return 0;
+        }
 
-        a = strndup(p, c - p);
+        a = strndup(path, next - path);
         if (!a)
                 return -ENOMEM;
 
+        path_simplify(a, true);
+
         if (!path_is_valid(a))
                 return -EINVAL;
 

diff --git a/src/basic/path-util.h b/src/basic/path-util.h
index 4c4e7a1e77052a392c75e9a9792267eeeb88e18f..e447608d40b0436eeb5124741342bc36c28a4ac5 100644 (file)
--- a/src/basic/path-util.h
+++ b/src/basic/path-util.h
@@ -155,8 +155,8 @@ char* dirname_malloc(const char *path);
 int path_find_first_component(const char **p, bool accept_dot_dot, const char **ret);
 int path_find_last_component(const char *path, bool accept_dot_dot, const char **next, const char **ret);
 const char *last_path_component(const char *path);
-int path_extract_filename(const char *p, char **ret);
-int path_extract_directory(const char *p, char **ret);
+int path_extract_filename(const char *path, char **ret);
+int path_extract_directory(const char *path, char **ret);
 
 bool filename_is_valid(const char *p) _pure_;
 bool path_is_valid_full(const char *p, bool accept_dot_dot) _pure_;

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 4dcfcaabe17a912481693df1e5d52842b9a57f7a..c07c22a4072414998ea0a8d29201d669f35ca467 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -786,26 +786,28 @@ static void test_path_extract_filename(void) {
         test_path_extract_filename_one("/", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one("//", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one("///", NULL, -EADDRNOTAVAIL);
-        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, -EADDRNOTAVAIL);
+        test_path_extract_filename_one("./", NULL, -EADDRNOTAVAIL);
+        test_path_extract_filename_one("./.", NULL, -EADDRNOTAVAIL);
+        test_path_extract_filename_one("././", NULL, -EADDRNOTAVAIL);
+        test_path_extract_filename_one("././/", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one("/foo/a", "a", 0);
         test_path_extract_filename_one("/foo/a/", "a", O_DIRECTORY);
         test_path_extract_filename_one("", NULL, -EINVAL);
         test_path_extract_filename_one("a", "a", 0);
         test_path_extract_filename_one("a/", "a", O_DIRECTORY);
+        test_path_extract_filename_one("a/././//.", "a", O_DIRECTORY);
         test_path_extract_filename_one("/a", "a", 0);
         test_path_extract_filename_one("/a/", "a", O_DIRECTORY);
+        test_path_extract_filename_one("/a//./.", "a", O_DIRECTORY);
         test_path_extract_filename_one("/////////////a/////////////", "a", O_DIRECTORY);
-        test_path_extract_filename_one("xx/.", NULL, -EINVAL);
+        test_path_extract_filename_one("//./a/.///b./././.c//./d//.", "d", O_DIRECTORY);
+        test_path_extract_filename_one("xx/.", "xx", O_DIRECTORY);
         test_path_extract_filename_one("xx/..", 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, -EINVAL);
-        test_path_extract_filename_one("/.", NULL, -EADDRNOTAVAIL);
-        test_path_extract_filename_one("./", NULL, -EINVAL);
 }
 
 static void test_path_extract_directory_one(const char *input, const char *output, int ret) {
@@ -844,26 +846,28 @@ static void test_path_extract_directory(void) {
         test_path_extract_directory_one("/", NULL, -EADDRNOTAVAIL);
         test_path_extract_directory_one("//", NULL, -EADDRNOTAVAIL);
         test_path_extract_directory_one("///", NULL, -EADDRNOTAVAIL);
-        test_path_extract_directory_one(".", NULL, -EDESTADDRREQ);
-        test_path_extract_directory_one("./.", ".", 0);
-        test_path_extract_directory_one("././", ".", 0);
-        test_path_extract_directory_one("././/", ".", 0);
+        test_path_extract_directory_one("/.", NULL, -EADDRNOTAVAIL);
+        test_path_extract_directory_one(".", NULL, -EADDRNOTAVAIL);
+        test_path_extract_directory_one("./", NULL, -EADDRNOTAVAIL);
+        test_path_extract_directory_one("./.", NULL, -EADDRNOTAVAIL);
+        test_path_extract_directory_one("././", NULL, -EADDRNOTAVAIL);
+        test_path_extract_directory_one("././/", NULL, -EADDRNOTAVAIL);
         test_path_extract_directory_one("/foo/a", "/foo", 0);
         test_path_extract_directory_one("/foo/a/", "/foo", 0);
         test_path_extract_directory_one("", NULL, -EINVAL);
         test_path_extract_directory_one("a", NULL, -EDESTADDRREQ);
         test_path_extract_directory_one("a/", NULL, -EDESTADDRREQ);
+        test_path_extract_directory_one("a/././//.", NULL, -EDESTADDRREQ);
         test_path_extract_directory_one("/a", "/", 0);
         test_path_extract_directory_one("/a/", "/", 0);
+        test_path_extract_directory_one("/a//./.", "/", 0);
         test_path_extract_directory_one("/////////////a/////////////", "/", 0);
-        test_path_extract_directory_one("xx/.", "xx", 0);
-        test_path_extract_directory_one("xx/..", "xx", 0);
-        test_path_extract_directory_one("..", NULL, -EDESTADDRREQ);
-        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("/.", NULL, -EADDRNOTAVAIL);
-        test_path_extract_directory_one("./", NULL, -EDESTADDRREQ);
+        test_path_extract_directory_one("//./a/.///b./././.c//./d//.", "/a/b./.c", 0);
+        test_path_extract_directory_one("xx/.", NULL, -EDESTADDRREQ);
+        test_path_extract_directory_one("xx/..", NULL, -EINVAL);
+        test_path_extract_directory_one("..", NULL, -EINVAL);
+        test_path_extract_directory_one("/..", NULL, -EINVAL);
+        test_path_extract_directory_one("../", NULL, -EINVAL);
 }
 
 static void test_filename_is_valid(void) {

diff --git a/src/test/test-tmpfile-util.c b/src/test/test-tmpfile-util.c
index 83bac15d003043c65919ec3ffd2a7c35e06d3505..af18df1bd571c50ab3fc5f5826b817d49e2d98cd 100644 (file)
--- a/src/test/test-tmpfile-util.c
+++ b/src/test/test-tmpfile-util.c
@@ -26,8 +26,10 @@ static void test_tempfn_random_one(const char *p, const char *extra, const char
 }
 
 static void test_tempfn_random(void) {
+        log_info("/* %s */", __func__);
+
         test_tempfn_random_one("", NULL, NULL, -EINVAL);
-        test_tempfn_random_one(".", NULL, NULL, -EINVAL);
+        test_tempfn_random_one(".", NULL, NULL, -EADDRNOTAVAIL);
         test_tempfn_random_one("..", NULL, NULL, -EINVAL);
         test_tempfn_random_one("/", NULL, NULL, -EADDRNOTAVAIL);
 
@@ -68,8 +70,10 @@ static void test_tempfn_xxxxxx_one(const char *p, const char *extra, const char
 }
 
 static void test_tempfn_xxxxxx(void) {
+        log_info("/* %s */", __func__);
+
         test_tempfn_xxxxxx_one("", NULL, NULL, -EINVAL);
-        test_tempfn_xxxxxx_one(".", NULL, NULL, -EINVAL);
+        test_tempfn_xxxxxx_one(".", NULL, NULL, -EADDRNOTAVAIL);
         test_tempfn_xxxxxx_one("..", NULL, NULL, -EINVAL);
         test_tempfn_xxxxxx_one("/", NULL, NULL, -EADDRNOTAVAIL);