From ee277c6bc750b6398546fd030208e720ee155012 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 23 Feb 2021 16:49:29 +0100 Subject: [PATCH] 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. --- src/basic/path-util.c | 21 ++++++++++++++++----- src/test/test-path-util.c | 15 +++++++++------ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index fe8321edce..50ba44492e 100644 --- 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 db6c1a9efa..b92a77b9f4 100644 --- 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);