From b351ebdc9731139a9ee1599e45ccb38dcbaee357 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 22:52:22 +0900 Subject: [PATCH 1/8] Revert "tree-wide: use CONF_FILES_CHASE_BASENAME where root directory is specified" This reverts commit 6b606f3586db64a1010a64921d302028134e1902. The commit was mistakenly merged without review, and has several issues. Let's revert at least now. --- src/libsystemd/sd-journal/catalog.c | 4 +--- src/shared/hwdb-util.c | 4 +--- src/shared/install.c | 4 +--- src/udev/udevadm-util.c | 8 ++------ test/units/TEST-17-UDEV.verify.sh | 4 ++-- 5 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 30b7e4a9de..3ca3b2dab4 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -450,9 +450,7 @@ int catalog_update(const char *database, const char *root, const char* const *di dirs = catalog_file_dirs; _cleanup_strv_free_ char **files = NULL; - r = conf_files_list_strv(&files, ".catalog", root, - CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME | CONF_FILES_FILTER_MASKED, - dirs); + r = conf_files_list_strv(&files, ".catalog", root, 0, dirs); if (r < 0) return log_error_errno(r, "Failed to get catalog files: %m"); diff --git a/src/shared/hwdb-util.c b/src/shared/hwdb-util.c index 5e76024d86..10d6e8ee48 100644 --- a/src/shared/hwdb-util.c +++ b/src/shared/hwdb-util.c @@ -601,9 +601,7 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co trie->nodes_count++; - err = conf_files_list_strv(&files, ".hwdb", root, - CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME | CONF_FILES_FILTER_MASKED, - conf_file_dirs); + err = conf_files_list_strv(&files, ".hwdb", root, 0, conf_file_dirs); if (err < 0) return log_error_errno(err, "Failed to enumerate hwdb files: %m"); diff --git a/src/shared/install.c b/src/shared/install.c index 8bf4a64728..8c6909ddf2 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3298,9 +3298,7 @@ static int presets_find_config(RuntimeScope scope, const char *root_dir, char ** else assert_not_reached(); - return conf_files_list_strv(files, ".preset", root_dir, - CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME | CONF_FILES_FILTER_MASKED, - dirs); + return conf_files_list_strv(files, ".preset", root_dir, 0, dirs); } static int read_presets(RuntimeScope scope, const char *root_dir, UnitFilePresets *presets) { diff --git a/src/udev/udevadm-util.c b/src/udev/udevadm-util.c index bc0877405b..7952d1dc14 100644 --- a/src/udev/udevadm-util.c +++ b/src/udev/udevadm-util.c @@ -282,9 +282,7 @@ static int search_rules_file(const char *s, const char *root, char ***files) { if (r == -EISDIR) { _cleanup_strv_free_ char **files_in_dir = NULL; - r = conf_files_list_strv(&files_in_dir, ".rules", root, - CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME | CONF_FILES_FILTER_MASKED, - STRV_MAKE_CONST(s)); + r = conf_files_list_strv(&files_in_dir, ".rules", root, 0, (const char* const*) STRV_MAKE_CONST(s)); if (r < 0) return log_error_errno(r, "Failed to enumerate rules files in '%s': %m", resolved); @@ -311,9 +309,7 @@ int search_rules_files(char * const *a, const char *root, char ***ret) { assert(ret); if (strv_isempty(a)) { - r = conf_files_list_strv(&files, ".rules", root, - CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME | CONF_FILES_FILTER_MASKED, - (const char* const*) CONF_PATHS_STRV("udev/rules.d")); + r = conf_files_list_strv(&files, ".rules", root, 0, (const char* const*) CONF_PATHS_STRV("udev/rules.d")); if (r < 0) return log_error_errno(r, "Failed to enumerate rules files: %m"); diff --git a/test/units/TEST-17-UDEV.verify.sh b/test/units/TEST-17-UDEV.verify.sh index 6dd8f3ed3e..f9ec6612b4 100755 --- a/test/units/TEST-17-UDEV.verify.sh +++ b/test/units/TEST-17-UDEV.verify.sh @@ -130,9 +130,9 @@ assert_1 --root="${workdir}" cp "${workdir}/output_0_files" "${exo}" assert_0 "${rules_dir}" -# Directory with an invalid loop. +# Directory with a loop. ln -s . "${rules_dir}/loop.rules" -assert_0 "${rules_dir}" +assert_1 "${rules_dir}" rm "${rules_dir}/loop.rules" # Empty rules. From 2d0ec7f9000b922e5e180bfa1fe0777e8033e8df Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 22:54:04 +0900 Subject: [PATCH 2/8] Revert "pretty-print: also chase symlinks in the last component" This reverts commit 2f2977cab8ee94e5479d86c93c7a4346789a920a. The commit was mistakenly merged without review. Also as pointed at https://github.com/systemd/systemd/pull/37972#discussion_r2173234198 now the output is confusing. Let's revert at least now. --- src/shared/pretty-print.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index 55bc585b83..8be99a8d3f 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -6,7 +6,6 @@ #include #include "alloc-util.h" -#include "chase.h" #include "color-util.h" #include "conf-files.h" #include "constants.h" @@ -419,16 +418,12 @@ int conf_files_cat(const char *root, const char *name, CatFlags flags) { /* First locate the main config file, if any */ if (!is_collection) { STRV_FOREACH(prefix, prefixes) { - _cleanup_free_ char *p = path_join(*prefix, name); - if (!p) + path = path_join(root, *prefix, name); + if (!path) return log_oom(); - - r = chase_and_access(p, root, CHASE_PREFIX_ROOT, F_OK, &path); - if (r < 0) { - if (r != -ENOENT) - log_debug_errno(r, "Failed to chase %s%s, ignoring: %m", strempty(root), p); - continue; - } + if (access(path, F_OK) == 0) + break; + path = mfree(path); } if (!path) @@ -439,7 +434,7 @@ int conf_files_cat(const char *root, const char *name, CatFlags flags) { } /* Then locate the drop-ins, if any */ - r = conf_files_list_strv(&files, extension, root, CONF_FILES_CHASE_BASENAME, (const char* const*) dirs); + r = conf_files_list_strv(&files, extension, root, 0, (const char* const*) dirs); if (r < 0) return log_error_errno(r, "Failed to query file list: %m"); From c43223ff9f0f1c8b4bf2b40d01f56465476013bd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 22:58:05 +0900 Subject: [PATCH 3/8] conf-files: drop use of CONF_FILES_CHASE_BASENAME in conf_files_list_with_replacement() The flag has several potential issues, and will be dropped in the next commit. Follow-up for 41fb58595a22d50ca79278a64de4bff28f6dfd24. --- src/basic/conf-files.c | 11 +----- src/test/test-conf-files.c | 80 +++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 3233679c74..48fabc0442 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -295,15 +295,6 @@ static int conf_files_list_impl( resolved_replacement = path_join(resolved_dirpath_replacement, filename_replacement); if (!resolved_replacement) return log_oom_debug(); - - if (FLAGS_SET(flags, CONF_FILES_CHASE_BASENAME)) { - _cleanup_free_ char *p = NULL; - r = chaseat(rfd, resolved_replacement, CHASE_AT_RESOLVE_IN_ROOT | CHASE_NONEXISTENT, &p, /* ret_fd = */ NULL); - if (r < 0) - return r; - - free_and_replace(resolved_replacement, p); - } } STRV_FOREACH(p, dirs) { @@ -428,7 +419,7 @@ int conf_files_list_with_replacement( _cleanup_hashmap_free_ Hashmap *fh = NULL; _cleanup_free_ char *inserted = NULL; - ConfFilesFlags flags = CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME | CONF_FILES_FILTER_MASKED_BY_SYMLINK; + ConfFilesFlags flags = CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED_BY_SYMLINK; int r; assert(ret_files); diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c index e7dd95f18c..90b2f2f9b9 100644 --- a/src/test/test-conf-files.c +++ b/src/test/test-conf-files.c @@ -332,12 +332,12 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t2, "/absolute-empty.real"), - strjoina(t2, "/absolute-non-empty.real"), + strjoina(search3, "absolute-empty.conf"), + strjoina(search3, "absolute-non-empty.conf"), search1_b, strjoina(search2, "mm.conf"), - strjoina(t2, "/relative-empty.real"), - strjoina(t2, "/relative-non-empty.real")))); + strjoina(search3, "relative-empty.conf"), + strjoina(search3, "relative-non-empty.conf")))); ASSERT_STREQ(inserted, search1_a); result = strv_free(result); inserted = mfree(inserted); @@ -346,12 +346,12 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, strjoina(search1, "aa.conf"), - strjoina(t2, "/absolute-empty.real"), - strjoina(t2, "/absolute-non-empty.real"), + strjoina(search3, "absolute-empty.conf"), + strjoina(search3, "absolute-non-empty.conf"), search1_b, strjoina(search2, "mm.conf"), - strjoina(t2, "/relative-empty.real"), - strjoina(t2, "/relative-non-empty.real")))); + strjoina(search3, "relative-empty.conf"), + strjoina(search3, "relative-non-empty.conf")))); ASSERT_STREQ(inserted, strjoina(search1, "aa.conf")); result = strv_free(result); inserted = mfree(inserted); @@ -360,12 +360,12 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t2, "/absolute-empty.real"), - strjoina(t2, "/absolute-non-empty.real"), + strjoina(search3, "absolute-empty.conf"), + strjoina(search3, "absolute-non-empty.conf"), search1_b, strjoina(search2, "mm.conf"), - strjoina(t2, "/relative-empty.real"), - strjoina(t2, "/relative-non-empty.real")))); + strjoina(search3, "relative-empty.conf"), + strjoina(search3, "relative-non-empty.conf")))); ASSERT_NULL(inserted); result = strv_free(result); inserted = mfree(inserted); @@ -374,12 +374,12 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t2, "/absolute-empty.real"), - strjoina(t2, "/absolute-non-empty.real"), + strjoina(search3, "absolute-empty.conf"), + strjoina(search3, "absolute-non-empty.conf"), search1_b, strjoina(search2, "mm.conf"), - strjoina(t2, "/relative-empty.real"), - strjoina(t2, "/relative-non-empty.real")))); + strjoina(search3, "relative-empty.conf"), + strjoina(search3, "relative-non-empty.conf")))); ASSERT_NULL(inserted); result = strv_free(result); inserted = mfree(inserted); @@ -388,12 +388,12 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t2, "/absolute-empty.real"), - strjoina(t2, "/absolute-non-empty.real"), + strjoina(search3, "absolute-empty.conf"), + strjoina(search3, "absolute-non-empty.conf"), search1_b, strjoina(search2, "mm.conf"), - strjoina(t2, "/relative-empty.real"), - strjoina(t2, "/relative-non-empty.real"), + strjoina(search3, "relative-empty.conf"), + strjoina(search3, "relative-non-empty.conf"), strjoina(t, "/dir4/x.conf")))); ASSERT_STREQ(inserted, strjoina(t, "/dir4/x.conf")); result = strv_free(result); @@ -403,11 +403,11 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t, "/absolute-empty-for-root.real"), - strjoina(t, "/absolute-non-empty-for-root.real"), + strjoina(search3, "absolute-empty-for-root.conf"), + strjoina(search3, "absolute-non-empty-for-root.conf"), search1_b, - strjoina(t, "/relative-empty-for-root.real"), - strjoina(t, "/relative-non-empty-for-root.real")))); + strjoina(search3, "relative-empty-for-root.conf"), + strjoina(search3, "relative-non-empty-for-root.conf")))); ASSERT_STREQ(inserted, search1_a); result = strv_free(result); inserted = mfree(inserted); @@ -416,11 +416,11 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, strjoina(search1, "aa.conf"), - strjoina(t, "/absolute-empty-for-root.real"), - strjoina(t, "/absolute-non-empty-for-root.real"), + strjoina(search3, "absolute-empty-for-root.conf"), + strjoina(search3, "absolute-non-empty-for-root.conf"), search1_b, - strjoina(t, "/relative-empty-for-root.real"), - strjoina(t, "/relative-non-empty-for-root.real")))); + strjoina(search3, "relative-empty-for-root.conf"), + strjoina(search3, "relative-non-empty-for-root.conf")))); ASSERT_STREQ(inserted, strjoina(search1, "aa.conf")); result = strv_free(result); inserted = mfree(inserted); @@ -429,11 +429,11 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t, "/absolute-empty-for-root.real"), - strjoina(t, "/absolute-non-empty-for-root.real"), + strjoina(search3, "absolute-empty-for-root.conf"), + strjoina(search3, "absolute-non-empty-for-root.conf"), search1_b, - strjoina(t, "/relative-empty-for-root.real"), - strjoina(t, "/relative-non-empty-for-root.real")))); + strjoina(search3, "relative-empty-for-root.conf"), + strjoina(search3, "relative-non-empty-for-root.conf")))); ASSERT_NULL(inserted); result = strv_free(result); inserted = mfree(inserted); @@ -442,11 +442,11 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t, "/absolute-empty-for-root.real"), - strjoina(t, "/absolute-non-empty-for-root.real"), + strjoina(search3, "absolute-empty-for-root.conf"), + strjoina(search3, "absolute-non-empty-for-root.conf"), search1_b, - strjoina(t, "/relative-empty-for-root.real"), - strjoina(t, "/relative-non-empty-for-root.real")))); + strjoina(search3, "relative-empty-for-root.conf"), + strjoina(search3, "relative-non-empty-for-root.conf")))); ASSERT_NULL(inserted); result = strv_free(result); inserted = mfree(inserted); @@ -455,11 +455,11 @@ TEST(conf_files_list) { strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search2_aa, - strjoina(t, "/absolute-empty-for-root.real"), - strjoina(t, "/absolute-non-empty-for-root.real"), + strjoina(search3, "absolute-empty-for-root.conf"), + strjoina(search3, "absolute-non-empty-for-root.conf"), search1_b, - strjoina(t, "/relative-empty-for-root.real"), - strjoina(t, "/relative-non-empty-for-root.real"), + strjoina(search3, "relative-empty-for-root.conf"), + strjoina(search3, "relative-non-empty-for-root.conf"), strjoina(t, "/dir4/x.conf")))); ASSERT_STREQ(inserted, strjoina(t, "/dir4/x.conf")); result = strv_free(result); From 07c2fadbed66b26bef93cfc2221383640a434113 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 23:03:44 +0900 Subject: [PATCH 4/8] Revert "conf-files: introduce CONF_FILES_CHASE_BASENAME flag" This reverts commit 49653adef3ff47c7bcd1098c763eb2a55297df15. The commit was merged without review, and has several issues. Let's revert the change now, and address the issue pointed out by the commit later in another way. --- src/basic/conf-files.c | 3 --- src/basic/conf-files.h | 5 ++--- src/test/test-conf-files.c | 32 -------------------------------- 3 files changed, 2 insertions(+), 38 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 48fabc0442..0f736202e5 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -164,9 +164,6 @@ static int files_add( if (!n) return log_oom_debug(); - if (FLAGS_SET(flags, CONF_FILES_CHASE_BASENAME)) - free_and_replace(p, resolved_path); - r = hashmap_ensure_put(files, &string_hash_ops_free_free, n, p); if (r < 0) { assert(r == -ENOMEM); diff --git a/src/basic/conf-files.h b/src/basic/conf-files.h index 67489316b7..710e74c959 100644 --- a/src/basic/conf-files.h +++ b/src/basic/conf-files.h @@ -8,9 +8,8 @@ typedef enum ConfFilesFlags { CONF_FILES_REGULAR = 1 << 1, CONF_FILES_DIRECTORY = 1 << 2, CONF_FILES_BASENAME = 1 << 3, - CONF_FILES_CHASE_BASENAME = 1 << 4, /* If set, also resolve symlink files in config directories. */ - CONF_FILES_FILTER_MASKED_BY_SYMLINK = 1 << 5, - CONF_FILES_FILTER_MASKED_BY_EMPTY = 1 << 6, + CONF_FILES_FILTER_MASKED_BY_SYMLINK = 1 << 4, + CONF_FILES_FILTER_MASKED_BY_EMPTY = 1 << 5, CONF_FILES_FILTER_MASKED = CONF_FILES_FILTER_MASKED_BY_SYMLINK | CONF_FILES_FILTER_MASKED_BY_EMPTY, } ConfFilesFlags; diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c index 90b2f2f9b9..44106ef7a8 100644 --- a/src/test/test-conf-files.c +++ b/src/test/test-conf-files.c @@ -185,14 +185,6 @@ TEST(conf_files_list) { strjoina(search3, "relative-non-empty.conf")))); result = strv_free(result); - ASSERT_OK(conf_files_list(&result, /* suffix = */ NULL, /* root = */ NULL, CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME, search3)); - strv_print(result); - ASSERT_TRUE(strv_equal(result, STRV_MAKE(strjoina(t2, "/absolute-empty.real"), - strjoina(t2, "/absolute-non-empty.real"), - strjoina(t2, "/relative-empty.real"), - strjoina(t2, "/relative-non-empty.real")))); - result = strv_free(result); - ASSERT_OK(conf_files_list(&result, /* suffix = */ NULL, t, CONF_FILES_FILTER_MASKED, "/dir3/")); strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(strjoina(search3, "absolute-non-empty-for-root.conf"), @@ -221,14 +213,6 @@ TEST(conf_files_list) { strjoina(search3, "relative-non-empty-for-root.conf")))); result = strv_free(result); - ASSERT_OK(conf_files_list(&result, /* suffix = */ NULL, t, CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME, "/dir3/")); - strv_print(result); - ASSERT_TRUE(strv_equal(result, STRV_MAKE(strjoina(t, "/absolute-empty-for-root.real"), - strjoina(t, "/absolute-non-empty-for-root.real"), - strjoina(t, "/relative-empty-for-root.real"), - strjoina(t, "/relative-non-empty-for-root.real")))); - result = strv_free(result); - ASSERT_OK(conf_files_list_at(&result, /* suffix = */ NULL, AT_FDCWD, CONF_FILES_FILTER_MASKED, search3)); strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(strjoina(search3, "absolute-non-empty.conf"), @@ -257,14 +241,6 @@ TEST(conf_files_list) { strjoina(search3, "relative-non-empty.conf")))); result = strv_free(result); - ASSERT_OK(conf_files_list_at(&result, /* suffix = */ NULL, AT_FDCWD, CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME, search3)); - strv_print(result); - ASSERT_TRUE(strv_equal(result, STRV_MAKE(strjoina(t2, "/absolute-empty.real"), - strjoina(t2, "/absolute-non-empty.real"), - strjoina(t2, "/relative-empty.real"), - strjoina(t2, "/relative-non-empty.real")))); - result = strv_free(result); - ASSERT_OK(conf_files_list_at(&result, /* suffix = */ NULL, tfd, CONF_FILES_FILTER_MASKED, "/dir3/")); strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE("dir3/absolute-non-empty-for-root.conf", @@ -293,14 +269,6 @@ TEST(conf_files_list) { "dir3/relative-non-empty-for-root.conf"))); result = strv_free(result); - ASSERT_OK(conf_files_list_at(&result, /* suffix = */ NULL, tfd, CONF_FILES_REGULAR | CONF_FILES_CHASE_BASENAME, "/dir3/")); - strv_print(result); - ASSERT_TRUE(strv_equal(result, STRV_MAKE("absolute-empty-for-root.real", - "absolute-non-empty-for-root.real", - "relative-empty-for-root.real", - "relative-non-empty-for-root.real"))); - result = strv_free(result); - /* filename only */ ASSERT_OK(conf_files_list_strv(&result, ".conf", NULL, CONF_FILES_FILTER_MASKED | CONF_FILES_BASENAME, STRV_MAKE_CONST(search1, search2))); strv_print(result); From cdd3aba3dfe3618f9e22522b32dda88075ef7804 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 11:00:56 +0900 Subject: [PATCH 5/8] conf-files: fstatat() sets errno on failure Follow-up for 50c81130b69d04288f50217bede709bac6ca2b1a. --- src/basic/conf-files.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 0f736202e5..49fa1dce79 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -100,13 +100,10 @@ static int files_add( continue; } - if (need_stat) { - r = fstatat(rfd, resolved_path, &st, AT_SYMLINK_NOFOLLOW); - if (r < 0) { - log_debug_errno(r, "Failed to stat '%s/%s', ignoring: %m", - root, skip_leading_slash(p)); - continue; - } + if (need_stat && fstatat(rfd, resolved_path, &st, AT_SYMLINK_NOFOLLOW) < 0) { + log_debug_errno(errno, "Failed to stat '%s/%s', ignoring: %m", + root, skip_leading_slash(p)); + continue; } } else { From 8ce463639a67b68ba5b4cec50cf36e138a9b0f0e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 20:55:11 +0900 Subject: [PATCH 6/8] path-util: move empty_or_root_to_null() from chase.c And rename it to empty_or_root_harder_to_null(), as it also checks if the input path effectively points to the root by calling path_is_root(). This also adds simple test cases for the function. --- src/basic/chase.c | 27 +++------------------------ src/basic/path-util.c | 21 +++++++++++++++++++++ src/basic/path-util.h | 1 + src/test/test-path-util.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 24 deletions(-) diff --git a/src/basic/chase.c b/src/basic/chase.c index 34423cc300..8fd7e67559 100644 --- a/src/basic/chase.c +++ b/src/basic/chase.c @@ -565,27 +565,6 @@ chased_one: return 0; } -static int empty_or_root_to_null(const char **path) { - int r; - - assert(path); - - /* This nullifies the input path when the path is empty or points to "/". */ - - if (empty_or_root(*path)) { - *path = NULL; - return 0; - } - - r = path_is_root(*path); - if (r < 0) - return r; - if (r > 0) - *path = NULL; - - return 0; -} - int chase(const char *path, const char *root, ChaseFlags flags, char **ret_path, int *ret_fd) { _cleanup_free_ char *root_abs = NULL, *absolute = NULL, *p = NULL; _cleanup_close_ int fd = -EBADF, pfd = -EBADF; @@ -596,7 +575,7 @@ int chase(const char *path, const char *root, ChaseFlags flags, char **ret_path, if (isempty(path)) return -EINVAL; - r = empty_or_root_to_null(&root); + r = empty_or_root_harder_to_null(&root); if (r < 0) return r; @@ -696,7 +675,7 @@ int chaseat_prefix_root(const char *path, const char *root, char **ret) { if (!path_is_absolute(path)) { _cleanup_free_ char *root_abs = NULL; - r = empty_or_root_to_null(&root); + r = empty_or_root_harder_to_null(&root); if (r < 0 && r != -ENOENT) return r; @@ -735,7 +714,7 @@ int chase_extract_filename(const char *path, const char *root, char **ret) { if (!path_is_absolute(path)) return -EINVAL; - r = empty_or_root_to_null(&root); + r = empty_or_root_harder_to_null(&root); if (r < 0 && r != -ENOENT) return r; diff --git a/src/basic/path-util.c b/src/basic/path-util.c index dd23d3f6ab..36cf9a1649 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1406,6 +1406,27 @@ const char* empty_to_root(const char *path) { return isempty(path) ? "/" : path; } +int empty_or_root_harder_to_null(const char **path) { + int r; + + assert(path); + + /* This nullifies the input path when the path is empty or points to "/". */ + + if (empty_or_root(*path)) { + *path = NULL; + return 0; + } + + r = path_is_root(*path); + if (r < 0) + return r; + if (r > 0) + *path = NULL; + + return 0; +} + bool path_strv_contains(char * const *l, const char *path) { assert(path); diff --git a/src/basic/path-util.h b/src/basic/path-util.h index a209223bcc..45518c07e5 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -179,6 +179,7 @@ static inline const char* skip_dev_prefix(const char *p) { bool empty_or_root(const char *path) _pure_; const char* empty_to_root(const char *path) _pure_; +int empty_or_root_harder_to_null(const char **path); bool path_strv_contains(char * const *l, const char *path); bool prefixed_path_strv_contains(char * const *l, const char *path); diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index ea025cb135..cbe0341fe4 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -1238,6 +1238,42 @@ TEST(empty_or_root) { assert_se(!empty_or_root("//yy//")); } +TEST(empty_or_root_harder_to_null) { + const char *p; + + p = NULL; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_NULL(p); + + p = "/"; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_NULL(p); + + p = "////////"; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_NULL(p); + + p = "/../../././//"; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_NULL(p); + + p = "/usr"; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_STREQ(p, "/usr"); + + p = "/usr/../../../"; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_NULL(p); + + p = "/usr/"; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_STREQ(p, "/usr/"); + + p = "///././/../../usr////"; + ASSERT_OK(empty_or_root_harder_to_null(&p)); + ASSERT_STREQ(p, "///././/../../usr////"); +} + TEST(path_startswith_set) { ASSERT_STREQ(PATH_STARTSWITH_SET("/foo/bar", "/foo/quux", "/foo/bar", "/zzz"), ""); ASSERT_STREQ(PATH_STARTSWITH_SET("/foo/bar", "/foo/quux", "/foo/", "/zzz"), "bar"); From 9000db5f6eec813f2d853d197de088c06a99ba42 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 10:25:05 +0900 Subject: [PATCH 7/8] conf-files: fix an empty root handling in conf_files_list_strv() Before 50c81130b69d04288f50217bede709bac6ca2b1a, the function used chase(), hence if root is an empty string, each config directory made prefixed with the current working directory if it is relative. See implementation of chase(). With 50c81130b69d04288f50217bede709bac6ca2b1a, conf_files_list_strv() internally uses chaseat(), hence each config directory is not prefixed anymore even if it is relative. To restore the previous behavior, this makes - if root is an empty string, prefix each config directories with the current working directory if relative. - if root is relative, make it absolute to make the prefixed results also absolute, and debugging logs show absolute paths. - use chaseat_prefix_root() to prefix the results, for safety. Follow-ups for 50c81130b69d04288f50217bede709bac6ca2b1a. --- src/basic/conf-files.c | 87 ++++++++++++++++++++++++++++++-------- src/test/test-conf-files.c | 47 ++++++++++++++++++++ 2 files changed, 116 insertions(+), 18 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 49fa1dce79..cabdbd0f4d 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -196,9 +196,9 @@ static int copy_and_sort_files_from_hashmap(Hashmap *fh, const char *root, ConfF if (r < 0) return log_debug_errno(r, "Failed to extract filename from '%s': %m", *s); } else if (root) { - p = path_join(root, skip_leading_slash(*s)); - if (!p) - return log_oom_debug(); + r = chaseat_prefix_root(*s, root, &p); + if (r < 0) + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", *s, root); } if (p) @@ -325,6 +325,49 @@ static int conf_files_list_impl( return 0; } +static int prepare_dirs(const char *root, char * const *dirs, int *ret_rfd, char **ret_root, char ***ret_dirs) { + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; + int r; + + assert(ret_rfd); + assert(ret_root); + assert(ret_dirs); + + r = empty_or_root_harder_to_null(&root); + if (r < 0) + return log_debug_errno(r, "Failed to determine if '%s' points to the root directory: %m", strempty(root)); + + dirs_abs = strv_copy(dirs); + if (!dirs_abs) + return log_oom(); + + if (root) { + /* WHen a non-trivial root is specified, we will prefix the result later. Hence, it is not + * necessary to modify each config directories here. but needs to normalize the root directory. */ + r = path_make_absolute_cwd(root, &root_abs); + if (r < 0) + return log_debug_errno(r, "Failed to make '%s' absolute: %m", root); + + path_simplify(root_abs); + } else { + /* When an empty root or "/" is specified, we will open "/" below, hence we need to make + * each config directory absolute if relative. */ + r = path_strv_make_absolute_cwd(dirs_abs); + if (r < 0) + return log_debug_errno(r, "Failed to make directories absolute: %m"); + } + + _cleanup_close_ int rfd = open(empty_to_root(root_abs), O_CLOEXEC|O_DIRECTORY|O_PATH); + if (rfd < 0) + return log_debug_errno(errno, "Failed to open '%s': %m", empty_to_root(root_abs)); + + *ret_rfd = TAKE_FD(rfd); + *ret_root = TAKE_PTR(root_abs); + *ret_dirs = TAKE_PTR(dirs_abs); + return 0; +} + int conf_files_list_strv( char ***ret, const char *suffix, @@ -333,19 +376,23 @@ int conf_files_list_strv( const char * const *dirs) { _cleanup_hashmap_free_ Hashmap *fh = NULL; + _cleanup_close_ int rfd = -EBADF; + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; int r; assert(ret); - _cleanup_close_ int rfd = open(empty_to_root(root), O_CLOEXEC|O_DIRECTORY|O_PATH); - if (rfd < 0) - return log_debug_errno(errno, "Failed to open '%s': %m", root); - - r = conf_files_list_impl(suffix, rfd, root, flags, dirs, /* replacement = */ NULL, &fh, /* ret_inserted = */ NULL); + r = prepare_dirs(root, (char**) dirs, &rfd, &root_abs, &dirs_abs); if (r < 0) return r; - return copy_and_sort_files_from_hashmap(fh, empty_to_root(root), flags, ret); + r = conf_files_list_impl(suffix, rfd, root_abs, flags, (const char * const *) dirs_abs, + /* replacement = */ NULL, &fh, /* ret_inserted = */ NULL); + if (r < 0) + return r; + + return copy_and_sort_files_from_hashmap(fh, empty_to_root(root_abs), flags, ret); } int conf_files_list_strv_at( @@ -414,29 +461,33 @@ int conf_files_list_with_replacement( _cleanup_hashmap_free_ Hashmap *fh = NULL; _cleanup_free_ char *inserted = NULL; ConfFilesFlags flags = CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED_BY_SYMLINK; + _cleanup_close_ int rfd = -EBADF; + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; int r; assert(ret_files); - root = empty_to_root(root); - _cleanup_close_ int rfd = open(root, O_CLOEXEC|O_DIRECTORY|O_PATH); - if (rfd < 0) - return log_debug_errno(errno, "Failed to open '%s': %m", root); + r = prepare_dirs(root, config_dirs, &rfd, &root_abs, &dirs_abs); + if (r < 0) + return r; - r = conf_files_list_impl(".conf", rfd, root, flags, (const char * const *) config_dirs, + r = conf_files_list_impl(".conf", rfd, root_abs, flags, (const char * const *) dirs_abs, replacement, &fh, ret_inserted ? &inserted : NULL); if (r < 0) return r; if (inserted) { - char *p = path_join(empty_to_root(root), skip_leading_slash(inserted)); - if (!p) - return log_oom_debug(); + char *p; + + r = chaseat_prefix_root(inserted, root_abs, &p); + if (r < 0) + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", inserted, empty_to_root(root_abs)); free_and_replace(inserted, p); } - r = copy_and_sort_files_from_hashmap(fh, empty_to_root(root), flags, ret_files); + r = copy_and_sort_files_from_hashmap(fh, empty_to_root(root_abs), flags, ret_files); if (r < 0) return r; diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c index 44106ef7a8..ec60e24cff 100644 --- a/src/test/test-conf-files.c +++ b/src/test/test-conf-files.c @@ -88,6 +88,18 @@ TEST(conf_files_list) { result = strv_free(result); + ASSERT_OK(conf_files_list(&result, NULL, "/", CONF_FILES_FILTER_MASKED, search1)); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, "///../../././//", CONF_FILES_FILTER_MASKED, search1)); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + + result = strv_free(result); + ASSERT_OK(conf_files_list(&result, NULL, t, CONF_FILES_FILTER_MASKED, "/dir1/")); strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); @@ -106,6 +118,41 @@ TEST(conf_files_list) { result = strv_free(result); + /* search dir1 with relative path */ + ASSERT_OK_ERRNO(chdir("/tmp/")); + + ASSERT_OK(conf_files_list(&result, NULL, NULL, CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, "/", CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, "///../../././//", CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, t, CONF_FILES_FILTER_MASKED, "dir1")); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list_at(&result, NULL, AT_FDCWD, CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(path_startswith(search1_a, "/tmp/"), + path_startswith(search1_b, "/tmp/"), + path_startswith(search1_c, "/tmp/")))); + result = strv_free(result); + + ASSERT_OK(conf_files_list_at(&result, NULL, tfd, CONF_FILES_FILTER_MASKED, "dir1")); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE("dir1/a.conf", "dir1/b.conf", "dir1/c.foo"))); + result = strv_free(result); + /* search dir1 with suffix */ ASSERT_OK(conf_files_list(&result, ".conf", NULL, CONF_FILES_FILTER_MASKED, search1)); strv_print(result); From bb60dd27a6ca2134f5afd996d4a072a73718e635 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 10:39:32 +0900 Subject: [PATCH 8/8] TEST-17-UDEV: check the exit code at last Otherwise, it is hard to debug when the test failed. --- test/units/TEST-17-UDEV.verify.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/units/TEST-17-UDEV.verify.sh b/test/units/TEST-17-UDEV.verify.sh index f9ec6612b4..243fc00f55 100755 --- a/test/units/TEST-17-UDEV.verify.sh +++ b/test/units/TEST-17-UDEV.verify.sh @@ -76,7 +76,6 @@ assert_1_impl() { udevadm verify "$@" >"${out}" 2>"${err}" rc=$? set -e - assert_eq "$rc" 1 if [ -f "${exp}" ]; then diff -u "${exp}" "${err}" @@ -87,6 +86,7 @@ assert_1_impl() { elif [ -f "${rules}" ]; then diff -u "${workdir}/default_output_1_fail" "${out}" fi + assert_eq "$rc" 1 } assert_1() {