diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 215e7951dc..c31fe79ebd 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -23,26 +23,19 @@ #include "terminal-util.h" static int files_add( - Hashmap **h, + DIR *dir, + const char *dirpath, + Hashmap **files, Set **masked, const char *suffix, - const char *root, - unsigned flags, - const char *path) { + unsigned flags) { - _cleanup_free_ char *dirpath = NULL; - _cleanup_closedir_ DIR *dir = NULL; int r; - assert(h); + assert(dir); + assert(dirpath); + assert(files); assert(masked); - assert(path); - - r = chase_and_opendir(path, root, CHASE_PREFIX_ROOT, &dirpath, &dir); - if (r == -ENOENT) - return 0; - if (r < 0) - return log_debug_errno(r, "Failed to open directory '%s/%s': %m", empty_or_root(root) ? "" : root, dirpath); FOREACH_DIRENT(de, dir, return -errno) { _cleanup_free_ char *n = NULL, *p = NULL; @@ -53,7 +46,7 @@ static int files_add( continue; /* Has this file already been found in an earlier directory? */ - if (hashmap_contains(*h, de->d_name)) { + if (hashmap_contains(*files, de->d_name)) { log_debug("Skipping overridden file '%s/%s'.", dirpath, de->d_name); continue; } @@ -107,13 +100,13 @@ static int files_add( return -ENOMEM; if ((flags & CONF_FILES_BASENAME)) - r = hashmap_ensure_put(h, &string_hash_ops_free, n, n); + r = hashmap_ensure_put(files, &string_hash_ops_free, n, n); else { p = path_join(dirpath, de->d_name); if (!p) return -ENOMEM; - r = hashmap_ensure_put(h, &string_hash_ops_free_free, n, p); + r = hashmap_ensure_put(files, &string_hash_ops_free_free, n, p); } if (r < 0) return r; @@ -127,36 +120,17 @@ static int files_add( } static int base_cmp(char * const *a, char * const *b) { - return strcmp(basename(*a), basename(*b)); + assert(a); + assert(b); + return path_compare_filename(*a, *b); } -static int conf_files_list_strv_internal( - char ***ret, - const char *suffix, - const char *root, - unsigned flags, - char **dirs) { - - _cleanup_hashmap_free_ Hashmap *fh = NULL; - _cleanup_set_free_ Set *masked = NULL; - _cleanup_strv_free_ char **files = NULL; +static int copy_and_sort_files_from_hashmap(Hashmap *fh, char ***ret) { _cleanup_free_ char **sv = NULL; - int r; + char **files; assert(ret); - /* This alters the dirs string array */ - if (!path_strv_resolve_uniq(dirs, root)) - return -ENOMEM; - - STRV_FOREACH(p, dirs) { - r = files_add(&fh, &masked, suffix, root, flags, *p); - if (r == -ENOMEM) - return r; - if (r < 0) - log_debug_errno(r, "Failed to search for files in %s, ignoring: %m", *p); - } - sv = hashmap_get_strv(fh); if (!sv) return -ENOMEM; @@ -167,11 +141,80 @@ static int conf_files_list_strv_internal( return -ENOMEM; typesafe_qsort(files, strv_length(files), base_cmp); - *ret = TAKE_PTR(files); + *ret = files; return 0; } +int conf_files_list_strv( + char ***ret, + const char *suffix, + const char *root, + unsigned flags, + const char * const *dirs) { + + _cleanup_hashmap_free_ Hashmap *fh = NULL; + _cleanup_set_free_ Set *masked = NULL; + int r; + + assert(ret); + + STRV_FOREACH(p, dirs) { + _cleanup_closedir_ DIR *dir = NULL; + _cleanup_free_ char *path = NULL; + + r = chase_and_opendir(*p, root, CHASE_PREFIX_ROOT, &path, &dir); + if (r < 0) { + if (r != -ENOENT) + log_debug_errno(r, "Failed to chase and open directory '%s', ignoring: %m", *p); + continue; + } + + r = files_add(dir, path, &fh, &masked, suffix, flags); + if (r == -ENOMEM) + return r; + if (r < 0) + log_debug_errno(r, "Failed to search for files in '%s', ignoring: %m", path); + } + + return copy_and_sort_files_from_hashmap(fh, ret); +} + +int conf_files_list_strv_at( + char ***ret, + const char *suffix, + int rfd, + unsigned flags, + const char * const *dirs) { + + _cleanup_hashmap_free_ Hashmap *fh = NULL; + _cleanup_set_free_ Set *masked = NULL; + int r; + + assert(rfd >= 0 || rfd == AT_FDCWD); + assert(ret); + + STRV_FOREACH(p, dirs) { + _cleanup_closedir_ DIR *dir = NULL; + _cleanup_free_ char *path = NULL; + + r = chase_and_opendirat(rfd, *p, CHASE_AT_RESOLVE_IN_ROOT, &path, &dir); + if (r < 0) { + if (r != -ENOENT) + log_debug_errno(r, "Failed to chase and open directory '%s', ignoring: %m", *p); + continue; + } + + r = files_add(dir, path, &fh, &masked, suffix, flags); + if (r == -ENOMEM) + return r; + if (r < 0) + log_debug_errno(r, "Failed to search for files in '%s', ignoring: %m", path); + } + + return copy_and_sort_files_from_hashmap(fh, ret); +} + int conf_files_insert(char ***strv, const char *root, char **dirs, const char *path) { /* Insert a path into strv, at the place honouring the usual sorting rules: * - we first compare by the basename @@ -240,28 +283,12 @@ int conf_files_insert(char ***strv, const char *root, char **dirs, const char *p return r; } -int conf_files_list_strv(char ***ret, const char *suffix, const char *root, unsigned flags, const char* const* dirs) { - _cleanup_strv_free_ char **copy = NULL; - - assert(ret); - - copy = strv_copy((char**) dirs); - if (!copy) - return -ENOMEM; - - return conf_files_list_strv_internal(ret, suffix, root, flags, copy); +int conf_files_list(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dir) { + return conf_files_list_strv(ret, suffix, root, flags, STRV_MAKE_CONST(dir)); } -int conf_files_list(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dir) { - _cleanup_strv_free_ char **dirs = NULL; - - assert(ret); - - dirs = strv_new(dir); - if (!dirs) - return -ENOMEM; - - return conf_files_list_strv_internal(ret, suffix, root, flags, dirs); +int conf_files_list_at(char ***ret, const char *suffix, int rfd, unsigned flags, const char *dir) { + return conf_files_list_strv_at(ret, suffix, rfd, flags, STRV_MAKE_CONST(dir)); } int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dirs) { @@ -273,7 +300,19 @@ int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, un if (!d) return -ENOMEM; - return conf_files_list_strv_internal(ret, suffix, root, flags, d); + return conf_files_list_strv(ret, suffix, root, flags, (const char**) d); +} + +int conf_files_list_nulstr_at(char ***ret, const char *suffix, int rfd, unsigned flags, const char *dirs) { + _cleanup_strv_free_ char **d = NULL; + + assert(ret); + + d = strv_split_nulstr(dirs); + if (!d) + return -ENOMEM; + + return conf_files_list_strv_at(ret, suffix, rfd, flags, (const char**) d); } int conf_files_list_with_replacement( diff --git a/src/basic/conf-files.h b/src/basic/conf-files.h index 7774ed7054..547c2fc137 100644 --- a/src/basic/conf-files.h +++ b/src/basic/conf-files.h @@ -12,8 +12,11 @@ enum { }; int conf_files_list(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dir); +int conf_files_list_at(char ***ret, const char *suffix, int rfd, unsigned flags, const char *dir); int conf_files_list_strv(char ***ret, const char *suffix, const char *root, unsigned flags, const char* const* dirs); +int conf_files_list_strv_at(char ***ret, const char *suffix, int rfd, unsigned flags, const char * const *dirs); int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dirs); +int conf_files_list_nulstr_at(char ***ret, const char *suffix, int rfd, unsigned flags, const char *dirs); int conf_files_insert(char ***strv, const char *root, char **dirs, const char *path); int conf_files_list_with_replacement( const char *root, diff --git a/src/basic/path-util.c b/src/basic/path-util.c index ae0b25d155..a5ab6045cd 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -491,25 +491,33 @@ bool path_equal_or_files_same(const char *a, const char *b, int flags) { return path_equal(a, b) || files_same(a, b, flags) > 0; } -bool path_equal_filename(const char *a, const char *b) { - _cleanup_free_ char *a_basename = NULL, *b_basename = NULL; - int r; +int path_compare_filename(const char *a, const char *b) { + _cleanup_free_ char *fa = NULL, *fb = NULL; + int r, j, k; - assert(a); - assert(b); + /* Order NULL before non-NULL */ + r = CMP(!!a, !!b); + if (r != 0) + return r; - r = path_extract_filename(a, &a_basename); - if (r < 0) { - log_debug_errno(r, "Failed to parse basename of %s: %m", a); - return false; - } - r = path_extract_filename(b, &b_basename); - if (r < 0) { - log_debug_errno(r, "Failed to parse basename of %s: %m", b); - return false; - } + j = path_extract_filename(a, &fa); + k = path_extract_filename(b, &fb); - return path_equal(a_basename, b_basename); + /* When one of paths is "." or root, then order it earlier. */ + r = CMP(j != -EADDRNOTAVAIL, k != -EADDRNOTAVAIL); + if (r != 0) + return r; + + /* When one of paths is invalid (or we get OOM), 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. */ + if (j < 0) + return strcmp(a, b); + + return strcmp(fa, fb); } char* path_extend_internal(char **x, ...) { diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 56f01f41d8..7843599816 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -66,15 +66,18 @@ char *path_startswith_full(const char *path, const char *prefix, bool accept_dot static inline char* path_startswith(const char *path, const char *prefix) { return path_startswith_full(path, prefix, true); } -int path_compare(const char *a, const char *b) _pure_; +int path_compare(const char *a, const char *b) _pure_; static inline bool path_equal(const char *a, const char *b) { return path_compare(a, b) == 0; } +int path_compare_filename(const char *a, const char *b); +static inline bool path_equal_filename(const char *a, const char *b) { + return path_compare_filename(a, b) == 0; +} + bool path_equal_or_files_same(const char *a, const char *b, int flags); -/* Compares only the last portion of the input paths, ie: the filenames */ -bool path_equal_filename(const char *a, const char *b); char* path_extend_internal(char **x, ...); #define path_extend(x, ...) path_extend_internal(x, __VA_ARGS__, POINTER_MAX) diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c index beda749fb8..4253490443 100644 --- a/src/test/test-conf-files.c +++ b/src/test/test-conf-files.c @@ -8,89 +8,151 @@ #include "alloc-util.h" #include "conf-files.h" +#include "fd-util.h" #include "fileio.h" #include "fs-util.h" #include "macro.h" #include "mkdir.h" -#include "parse-util.h" #include "path-util.h" #include "rm-rf.h" #include "string-util.h" #include "strv.h" #include "tests.h" -#include "user-util.h" - -static void setup_test_dir(char *tmp_dir, const char *files, ...) { - va_list ap; - - assert_se(mkdtemp(tmp_dir)); - - va_start(ap, files); - while (files) { - _cleanup_free_ char *path; - - assert_se(path = path_join(tmp_dir, files)); - assert_se(write_string_file(path, "foobar", WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_MKDIR_0755) >= 0); - - files = va_arg(ap, const char *); - } - va_end(ap); -} - -static void test_conf_files_list_one(bool use_root) { - char tmp_dir[] = "/tmp/test-conf-files-XXXXXX"; - _cleanup_strv_free_ char **found_files = NULL, **found_files2 = NULL; - const char *root_dir, *search, *expect_a, *expect_b, *expect_c, *mask; - - log_info("/* %s(%s) */", __func__, yes_no(use_root)); - - setup_test_dir(tmp_dir, - "/dir/a.conf", - "/dir/b.conf", - "/dir/c.foo", - NULL); - - mask = strjoina(tmp_dir, "/dir/d.conf"); - assert_se(symlink("/dev/null", mask) >= 0); - - if (use_root) { - root_dir = tmp_dir; - search = "/dir"; - } else { - root_dir = NULL; - search = strjoina(tmp_dir, "/dir"); - } - - expect_a = strjoina(tmp_dir, "/dir/a.conf"); - expect_b = strjoina(tmp_dir, "/dir/b.conf"); - expect_c = strjoina(tmp_dir, "/dir/c.foo"); - - log_debug("/* Check when filtered by suffix */"); - - assert_se(conf_files_list(&found_files, ".conf", root_dir, CONF_FILES_FILTER_MASKED, search) == 0); - strv_print(found_files); - - assert_se(found_files); - assert_se(streq_ptr(found_files[0], expect_a)); - assert_se(streq_ptr(found_files[1], expect_b)); - assert_se(!found_files[2]); - - log_debug("/* Check when unfiltered */"); - assert_se(conf_files_list(&found_files2, NULL, root_dir, CONF_FILES_FILTER_MASKED, search) == 0); - strv_print(found_files2); - - assert_se(found_files2); - assert_se(streq_ptr(found_files2[0], expect_a)); - assert_se(streq_ptr(found_files2[1], expect_b)); - assert_se(streq_ptr(found_files2[2], expect_c)); - assert_se(!found_files2[3]); - - assert_se(rm_rf(tmp_dir, REMOVE_ROOT|REMOVE_PHYSICAL) == 0); -} +#include "tmpfile-util.h" TEST(conf_files_list) { - test_conf_files_list_one(false); - test_conf_files_list_one(true); + _cleanup_(rm_rf_physical_and_freep) char *t = NULL; + _cleanup_close_ int tfd = -EBADF; + _cleanup_strv_free_ char **result = NULL; + const char *search1, *search2, *search1_a, *search1_b, *search1_c, *search2_aa; + + tfd = mkdtemp_open("/tmp/test-conf-files-XXXXXX", O_PATH, &t); + assert(tfd >= 0); + + assert_se(mkdirat(tfd, "dir1", 0755) >= 0); + assert_se(mkdirat(tfd, "dir2", 0755) >= 0); + + search1 = strjoina(t, "/dir1/"); + search2 = strjoina(t, "/dir2/"); + + FOREACH_STRING(p, "a.conf", "b.conf", "c.foo") { + _cleanup_free_ char *path = NULL; + + assert_se(path = path_join(search1, p)); + assert_se(write_string_file(path, "foobar", WRITE_STRING_FILE_CREATE) >= 0); + } + + assert_se(symlinkat("/dev/null", tfd, "dir1/m.conf") >= 0); + + FOREACH_STRING(p, "a.conf", "aa.conf", "m.conf") { + _cleanup_free_ char *path = NULL; + + assert_se(path = path_join(search2, p)); + assert_se(write_string_file(path, "hogehoge", WRITE_STRING_FILE_CREATE) >= 0); + } + + search1_a = strjoina(search1, "a.conf"); + search1_b = strjoina(search1, "b.conf"); + search1_c = strjoina(search1, "c.foo"); + search2_aa = strjoina(search2, "aa.conf"); + + /* search dir1 without suffix */ + assert_se(conf_files_list(&result, NULL, NULL, CONF_FILES_FILTER_MASKED, search1) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + + result = strv_free(result); + + assert_se(conf_files_list(&result, NULL, t, CONF_FILES_FILTER_MASKED, "/dir1/") >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + + result = strv_free(result); + + assert_se(conf_files_list_at(&result, NULL, AT_FDCWD, CONF_FILES_FILTER_MASKED, search1) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + + result = strv_free(result); + + assert_se(conf_files_list_at(&result, NULL, tfd, CONF_FILES_FILTER_MASKED, "/dir1/") >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE("dir1/a.conf", "dir1/b.conf", "dir1/c.foo"))); + + result = strv_free(result); + + /* search dir1 with suffix */ + assert_se(conf_files_list(&result, ".conf", NULL, CONF_FILES_FILTER_MASKED, search1) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search1_b))); + + result = strv_free(result); + + assert_se(conf_files_list(&result, ".conf", t, CONF_FILES_FILTER_MASKED, "/dir1/") >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search1_b))); + + result = strv_free(result); + + assert_se(conf_files_list_at(&result, ".conf", AT_FDCWD, CONF_FILES_FILTER_MASKED, search1) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search1_b))); + + result = strv_free(result); + + assert_se(conf_files_list_at(&result, ".conf", tfd, CONF_FILES_FILTER_MASKED, "/dir1/") >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE("dir1/a.conf", "dir1/b.conf"))); + + result = strv_free(result); + + /* search two dirs */ + assert_se(conf_files_list_strv(&result, ".conf", NULL, CONF_FILES_FILTER_MASKED, STRV_MAKE_CONST(search1, search2)) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search2_aa, search1_b))); + + result = strv_free(result); + + assert_se(conf_files_list_strv(&result, ".conf", t, CONF_FILES_FILTER_MASKED, STRV_MAKE_CONST("/dir1/", "/dir2/")) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search2_aa, search1_b))); + + result = strv_free(result); + + assert_se(conf_files_list_strv_at(&result, ".conf", AT_FDCWD, CONF_FILES_FILTER_MASKED, STRV_MAKE_CONST(search1, search2)) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE(search1_a, search2_aa, search1_b))); + + result = strv_free(result); + + assert_se(conf_files_list_strv_at(&result, ".conf", tfd, CONF_FILES_FILTER_MASKED, STRV_MAKE_CONST("/dir1/", "/dir2/")) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE("dir1/a.conf", "dir2/aa.conf", "dir1/b.conf"))); + + result = strv_free(result); + + /* filename only */ + assert_se(conf_files_list_strv(&result, ".conf", NULL, CONF_FILES_FILTER_MASKED | CONF_FILES_BASENAME, STRV_MAKE_CONST(search1, search2)) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE("a.conf", "aa.conf", "b.conf"))); + + result = strv_free(result); + + assert_se(conf_files_list_strv(&result, ".conf", t, CONF_FILES_FILTER_MASKED | CONF_FILES_BASENAME, STRV_MAKE_CONST("/dir1/", "/dir2/")) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE("a.conf", "aa.conf", "b.conf"))); + + result = strv_free(result); + + assert_se(conf_files_list_strv_at(&result, ".conf", AT_FDCWD, CONF_FILES_FILTER_MASKED | CONF_FILES_BASENAME, STRV_MAKE_CONST(search1, search2)) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE("a.conf", "aa.conf", "b.conf"))); + + result = strv_free(result); + + assert_se(conf_files_list_strv_at(&result, ".conf", tfd, CONF_FILES_FILTER_MASKED | CONF_FILES_BASENAME, STRV_MAKE_CONST("/dir1/", "/dir2/")) >= 0); + strv_print(result); + assert_se(strv_equal(result, STRV_MAKE("a.conf", "aa.conf", "b.conf"))); } static void test_conf_files_insert_one(const char *root) { diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 136005d51f..87e33919e0 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -46,11 +46,6 @@ TEST(path) { assert_se(!path_equal_ptr("/a", "/b")); assert_se(!path_equal_ptr("/a", NULL)); assert_se(!path_equal_ptr(NULL, "/a")); - - assert_se(path_equal_filename("/a/c", "/b/c")); - assert_se(path_equal_filename("/a", "/a")); - assert_se(!path_equal_filename("/a/b", "/a/c")); - assert_se(!path_equal_filename("/b", "/c")); } static void test_path_simplify_one(const char *in, const char *out) { @@ -150,6 +145,55 @@ TEST(path_compare) { test_path_compare_one("/foo/a/b", "/foo/aaa", -1); } +static void test_path_compare_filename_one(const char *a, const char *b, int expected) { + int r; + + assert_se(path_compare_filename(a, a) == 0); + assert_se(path_compare_filename(b, b) == 0); + + r = path_compare_filename(a, b); + assert_se((r > 0) == (expected > 0) && (r < 0) == (expected < 0)); + r = path_compare_filename(b, a); + assert_se((r < 0) == (expected > 0) && (r > 0) == (expected < 0)); + + assert_se(path_equal_filename(a, a) == 1); + assert_se(path_equal_filename(b, b) == 1); + assert_se(path_equal_filename(a, b) == (expected == 0)); + assert_se(path_equal_filename(b, a) == (expected == 0)); +} + +TEST(path_compare_filename) { + test_path_compare_filename_one("/goo", "/goo", 0); + test_path_compare_filename_one("/goo", "/goo", 0); + test_path_compare_filename_one("//goo", "/goo", 0); + test_path_compare_filename_one("//goo/////", "/goo", 0); + test_path_compare_filename_one("goo/////", "goo", 0); + test_path_compare_filename_one("/goo/boo", "/goo//boo", 0); + test_path_compare_filename_one("//goo/boo", "/goo/boo//", 0); + test_path_compare_filename_one("//goo/././//./boo//././//", "/goo/boo//.", 0); + test_path_compare_filename_one("/.", "//.///", -1); + test_path_compare_filename_one("/x", "x/", 0); + test_path_compare_filename_one("x/", "/", 1); + test_path_compare_filename_one("/x/./y", "x/y", 0); + test_path_compare_filename_one("/x/./y", "/x/y", 0); + test_path_compare_filename_one("/x/./././y", "/x/y/././.", 0); + test_path_compare_filename_one("./x/./././y", "./x/y/././.", 0); + test_path_compare_filename_one(".", "./.", -1); + test_path_compare_filename_one(".", "././.", -1); + test_path_compare_filename_one("./..", ".", 1); + test_path_compare_filename_one("x/.y", "x/y", -1); + test_path_compare_filename_one("foo", "/foo", 0); + test_path_compare_filename_one("/foo", "/foo/bar", 1); + test_path_compare_filename_one("/foo/aaa", "/foo/b", -1); + test_path_compare_filename_one("/foo/aaa", "/foo/b/a", 1); + test_path_compare_filename_one("/foo/a", "/foo/aaa", -1); + test_path_compare_filename_one("/foo/a/b", "/foo/aaa", 1); + test_path_compare_filename_one("/a/c", "/b/c", 0); + test_path_compare_filename_one("/a", "/a", 0); + test_path_compare_filename_one("/a/b", "/a/c", -1); + test_path_compare_filename_one("/b", "/c", -1); +} + TEST(path_equal_root) { /* Nail down the details of how path_equal("/", ...) works. */