path-util: tighten path_extract_filename()

Let's tighten the logic behind path_extract_filename() a bit: first of
all, refuse all cases of invalid paths with -EINVAL. More importantly
though return a recognizable error when a valid path is specified that
does not contain any filename. Specifically, "/" will now result in
-EADDRNOTAVAIL.

This changes API, but none of the existing callers care about the return
value, hence the change should be fine.

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 5bcbc7a7947278058c274b6b757cc7c34740aba9..3dff09b15198f12460be3760339d4e1ab123f9f2 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -823,6 +823,8 @@ const char *last_path_component(const char *path) {
          *    Also, the empty string is mapped to itself.
          *
          * This is different than basename(), which returns "" when a trailing slash is present.
+         *
+         * This always succeeds (except if you pass NULL in which case it returns NULL, too).
          */
 
         unsigned l, k;
@@ -848,24 +850,24 @@ const char *last_path_component(const char *path) {
 
 int path_extract_filename(const char *p, char **ret) {
         _cleanup_free_ char *a = NULL;
-        const char *c, *e = NULL, *q;
+        const char *c;
 
         /* 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. */
+         * filename_is_valid(). A wrapper around last_path_component(), but eats up trailing slashes. Returns
+         * -EADDRNOTAVAIL if specified parameter includes no filename (i.e. is "/" or so). Returns -EINVAL if
+         * not a valid path in the first place. */
 
-        if (!p)
+        if (!path_is_valid(p))
                 return -EINVAL;
 
-        c = last_path_component(p);
-
-        for (q = c; *q != 0; q++)
-                if (*q != '/')
-                        e = q + 1;
+        /* 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, "/"))
+                return -EADDRNOTAVAIL;
 
-        if (!e) /* no valid character? */
-                return -EINVAL;
+        c = last_path_component(p);
 
-        a = strndup(c, e - c);
+        a = strndup(c, strcspn(c, "/"));
         if (!a)
                 return -ENOMEM;
 
@@ -873,7 +875,6 @@ int path_extract_filename(const char *p, char **ret) {
                 return -EINVAL;
 
         *ret = TAKE_PTR(a);
-
         return 0;
 }
 

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index fcaffa4539107d4cb623f2735a1d307780c08b8c..206f5fd436db154cdebad9063343048ad42974f3 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -578,9 +578,9 @@ static void test_path_extract_filename(void) {
         test_path_extract_filename_one(NULL, NULL, -EINVAL);
         test_path_extract_filename_one("a/b/c", "c", 0);
         test_path_extract_filename_one("a/b/c/", "c", 0);
-        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, -EINVAL);
         test_path_extract_filename_one("./.", NULL, -EINVAL);
         test_path_extract_filename_one("././", NULL, -EINVAL);