path-util: return O_DIRECTORY from path_extract_filename() when path ends in slash

Let's fine-tune the path_extract_filename() interface: on succes return
O_DIRECTORY as indicator that the input path was slash-suffixed, and
regular 0 otherwise. This is useful since in many cases it is useful to
filter out paths that must refer to dirs early on.

I opted for O_DIRECTORY instead of the following other ideas:

1. return -EISDIR: I think the function should return an extracted
   filename even when referring to an obvious dir, so this is not an
   option.

2. S_ISDIR, this was a strong contender, but I think O_DIRECTORY is a
   tiny bit nicer since quite likely we will go on and open the thing,
   maybe with openat(), and hence it's quite nice to be able to OR in
   the return value into the flags argument of openat().

3. A new enum defined with two values "dont-know" and
   "definitely-directory". But I figured this was unnecessary, given we
   have other options too, that reuse existing definitions for very
   similar purposes.

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index fe8321edce0349241292d708b8b3d06863dd9676..50ba44492e502bd8ad624243959d9be1a1afa0e7 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -819,11 +819,21 @@ 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;
+        size_t n;
 
         /* 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
-         * -EADDRNOTAVAIL if specified parameter includes no filename (i.e. is "/" or so). Returns -EINVAL if
-         * not a valid path in the first place. */
+         * 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
+         * -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.
+         *
+         * 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))
                 return -EINVAL;
@@ -834,8 +844,9 @@ int path_extract_filename(const char *p, char **ret) {
                 return -EADDRNOTAVAIL;
 
         c = last_path_component(p);
+        n = strcspn(c, "/");
 
-        a = strndup(c, strcspn(c, "/"));
+        a = strndup(c, n);
         if (!a)
                 return -ENOMEM;
 
@@ -843,7 +854,7 @@ int path_extract_filename(const char *p, char **ret) {
                 return -EINVAL;
 
         *ret = TAKE_PTR(a);
-        return 0;
+        return c[n] == '/' ? O_DIRECTORY : 0;
 }
 
 int path_extract_directory(const char *p, char **ret) {

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index db6c1a9efa8c537388021ca3a57480ba9adccf89..b92a77b9f4013cb0881294136d1156d1c5d1a462 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -570,7 +570,10 @@ static void test_path_extract_filename_one(const char *input, const char *output
         int r;
 
         r = path_extract_filename(input, &k);
-        log_info("%s → %s/%s [expected: %s/%s]", strnull(input), strnull(k), strerror_safe(r), strnull(output), strerror_safe(ret));
+        log_info_errno(r, "%s → %s/%m [expected: %s/%s]",
+                       strnull(input),
+                       strnull(k), /* strerror(r) is printed via %m, to avoid that the two strerror()'s overwrite each other's buffers */
+                       strnull(output), ret < 0 ? strerror_safe(ret) : "-");
         assert_se(streq_ptr(k, output));
         assert_se(r == ret);
 }
@@ -580,7 +583,7 @@ 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("a/b/c/", "c", O_DIRECTORY);
         test_path_extract_filename_one("/", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one("//", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one("///", NULL, -EADDRNOTAVAIL);
@@ -589,13 +592,13 @@ 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("/foo/a", "a", 0);
-        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", 0);
+        test_path_extract_filename_one("a/", "a", O_DIRECTORY);
         test_path_extract_filename_one("/a", "a", 0);
-        test_path_extract_filename_one("/a/", "a", 0);
-        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("xx/.", NULL, -EINVAL);
         test_path_extract_filename_one("xx/..", NULL, -EINVAL);
         test_path_extract_filename_one("..", NULL, -EINVAL);