From b395aac97d4b8be2f3e6232ba0412ff8203b47bb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 16 Apr 2018 21:24:13 +0200 Subject: [PATCH] conf-files: beef up conf-files.[ch] a bit This adds fozr new flags: - If CONF_FILES_DIRECTORY is specified conf_file_list() and friends will look for directories only. - Similar CONF_FILES_REGULAR means we'll look only for regular files. - If CONF_FILES_BASENAME is specified the resulting list will contain only the basenames of all discovered files or directories, not the full paths. - If CONF_FILES_FILTER_MASKED is specified the resulting list will have masked entries removed (i.e. those symlinked to /dev/null and suchlike) These four flags are useful for discovering portable service profile information. While we are at it, also improve a couple of other things: - More debug logging - use path_hash_ops instead of string_hash_ops when putting together the path lists --- src/basic/conf-files.c | 128 +++++++++++++++++++++++++------------ src/basic/conf-files.h | 6 +- src/basic/exec-util.c | 2 +- src/test/test-conf-files.c | 28 +++++--- 4 files changed, 111 insertions(+), 53 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index a168be8b8..298f4b7f9 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -21,18 +21,28 @@ #include "macro.h" #include "missing.h" #include "path-util.h" +//#include "set.h" #include "stat-util.h" #include "string-util.h" #include "strv.h" //#include "terminal-util.h" #include "util.h" -static int files_add(Hashmap *h, const char *suffix, const char *root, unsigned flags, const char *path) { +static int files_add( + Hashmap *h, + Set *masked, + const char *suffix, + const char *root, + unsigned flags, + const char *path) { + _cleanup_closedir_ DIR *dir = NULL; const char *dirpath; struct dirent *de; int r; + assert(h); + assert((flags & CONF_FILES_FILTER_MASKED) == 0 || masked); assert(path); dirpath = prefix_roota(root, path); @@ -41,62 +51,89 @@ static int files_add(Hashmap *h, const char *suffix, const char *root, unsigned if (!dir) { if (errno == ENOENT) return 0; - return -errno; + + return log_debug_errno(errno, "Failed to open directory '%s': %m", dirpath); } FOREACH_DIRENT(de, dir, return -errno) { - char *p; + struct stat st; + char *p, *key; + + /* Does this match the suffix? */ + if (suffix && !endswith(de->d_name, suffix)) + continue; - if (!dirent_is_file_with_suffix(de, suffix)) { - log_debug("Ignoring %s/%s, because it's not a regular file with suffix %s.", dirpath, de->d_name, strna(suffix)); + /* Has this file already been found in an earlier directory? */ + if (hashmap_contains(h, de->d_name)) { + log_debug("Skipping overridden file '%s/%s'.", dirpath, de->d_name); continue; } - if (flags & CONF_FILES_EXECUTABLE) { - struct stat st; + /* Has this been masked in an earlier directory? */ + if ((flags & CONF_FILES_FILTER_MASKED) && set_contains(masked, de->d_name)) { + log_debug("File '%s/%s' is masked by previous entry.", dirpath, de->d_name); + continue; + } + /* Read file metadata if we shall validate the check for file masks, for node types or whether the node is marked executable. */ + if (flags & (CONF_FILES_FILTER_MASKED|CONF_FILES_REGULAR|CONF_FILES_DIRECTORY|CONF_FILES_EXECUTABLE)) + if (fstatat(dirfd(dir), de->d_name, &st, 0) < 0) { + log_debug_errno(errno, "Failed to stat '%s/%s', ignoring: %m", dirpath, de->d_name); + continue; + } + + /* Is this a masking entry? */ + if ((flags & CONF_FILES_FILTER_MASKED)) + if (null_or_empty(&st)) { + /* Mark this one as masked */ + r = set_put_strdup(masked, de->d_name); + if (r < 0) + return r; + + log_debug("File '%s/%s' is a mask.", dirpath, de->d_name); + continue; + } + + /* Does this node have the right type? */ + if (flags & (CONF_FILES_REGULAR|CONF_FILES_DIRECTORY)) + if (!((flags & CONF_FILES_DIRECTORY) && S_ISDIR(st.st_mode)) && + !((flags & CONF_FILES_REGULAR) && S_ISREG(st.st_mode))) { + log_debug("Ignoring '%s/%s', as it is not a of the right type.", dirpath, de->d_name); + continue; + } + + /* Does this node have the executable bit set? */ + if (flags & CONF_FILES_EXECUTABLE) /* As requested: check if the file is marked exectuable. Note that we don't check access(X_OK) * here, as we care about whether the file is marked executable at all, and not whether it is - * executable for us, because if such errors are stuff we should log about. */ + * executable for us, because if so, such errors are stuff we should log about. */ - if (fstatat(dirfd(dir), de->d_name, &st, 0) < 0) { - log_debug_errno(errno, "Failed to stat %s/%s, ignoring: %m", dirpath, de->d_name); + if ((st.st_mode & 0111) == 0) { /* not executable */ + log_debug("Ignoring '%s/%s', as it is not marked executable.", dirpath, de->d_name); continue; } - if (!null_or_empty(&st)) { - /* A mask is a symlink to /dev/null or an empty file. It does not even - * have to be executable. Other entries must be regular executable files - * or symlinks to them. */ - if (S_ISREG(st.st_mode)) { - if ((st.st_mode & 0111) == 0) { /* not executable */ - log_debug("Ignoring %s/%s, as it is not marked executable.", - dirpath, de->d_name); - continue; - } - } else { - log_debug("Ignoring %s/%s, as it is neither a regular file nor a mask.", - dirpath, de->d_name); - continue; - } - } - } + if (flags & CONF_FILES_BASENAME) { + p = strdup(de->d_name); + if (!p) + return -ENOMEM; - p = strjoin(dirpath, "/", de->d_name); - if (!p) - return -ENOMEM; + key = p; + } else { + p = strjoin(dirpath, "/", de->d_name); + if (!p) + return -ENOMEM; - r = hashmap_put(h, basename(p), p); - if (r == -EEXIST) { - log_debug("Skipping overridden file: %s.", p); - free(p); - } else if (r < 0) { - free(p); - return r; - } else if (r == 0) { - log_debug("Duplicate file %s", p); + key = basename(p); + } + + r = hashmap_put(h, key, p); + if (r < 0) { free(p); + return log_debug_errno(r, "Failed to add item to hashmap: %m"); } + + assert(r > 0); } return 0; @@ -112,6 +149,7 @@ static int base_cmp(const void *a, const void *b) { static int conf_files_list_strv_internal(char ***strv, const char *suffix, const char *root, unsigned flags, char **dirs) { _cleanup_hashmap_free_ Hashmap *fh = NULL; + _cleanup_set_free_free_ Set *masked = NULL; char **files, **p; int r; @@ -121,12 +159,18 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const if (!path_strv_resolve_uniq(dirs, root)) return -ENOMEM; - fh = hashmap_new(&string_hash_ops); + fh = hashmap_new(&path_hash_ops); if (!fh) return -ENOMEM; + if (flags & CONF_FILES_FILTER_MASKED) { + masked = set_new(&path_hash_ops); + if (!masked) + return -ENOMEM; + } + STRV_FOREACH(p, dirs) { - r = files_add(fh, suffix, root, flags, *p); + r = files_add(fh, masked, suffix, root, flags, *p); if (r == -ENOMEM) return r; if (r < 0) @@ -154,8 +198,8 @@ int conf_files_insert(char ***strv, const char *root, char **dirs, const char *p * - do nothing if our new entry matches the existing entry, * - replace the existing entry if our new entry has higher priority. */ + size_t i; char *t; - unsigned i; int r; for (i = 0; i < strv_length(*strv); i++) { diff --git a/src/basic/conf-files.h b/src/basic/conf-files.h index 2dd1b272c..78d03e6e0 100644 --- a/src/basic/conf-files.h +++ b/src/basic/conf-files.h @@ -9,7 +9,11 @@ ***/ enum { - CONF_FILES_EXECUTABLE = 1, + CONF_FILES_EXECUTABLE = 1U << 0, + CONF_FILES_REGULAR = 1U << 1, + CONF_FILES_DIRECTORY = 1U << 2, + CONF_FILES_BASENAME = 1U << 3, + CONF_FILES_FILTER_MASKED = 1U << 4, }; int conf_files_list(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dir, ...); diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index e7b0e0fc0..ad486db1a 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -89,7 +89,7 @@ static int do_execute( * If callbacks is nonnull, execution is serial. Otherwise, we default to parallel. */ - r = conf_files_list_strv(&paths, NULL, NULL, CONF_FILES_EXECUTABLE, (const char* const*) directories); + r = conf_files_list_strv(&paths, NULL, NULL, CONF_FILES_EXECUTABLE|CONF_FILES_REGULAR|CONF_FILES_FILTER_MASKED, (const char* const*) directories); if (r < 0) return r; diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c index 4bc92faca..6604f4c9a 100644 --- a/src/test/test-conf-files.c +++ b/src/test/test-conf-files.c @@ -10,8 +10,10 @@ #include "alloc-util.h" #include "conf-files.h" +//#include "fileio.h" #include "fs-util.h" #include "macro.h" +//#include "mkdir.h" #include "parse-util.h" #include "rm-rf.h" #include "string-util.h" @@ -22,12 +24,16 @@ static void setup_test_dir(char *tmp_dir, const char *files, ...) { va_list ap; - assert_se(mkdtemp(tmp_dir) != NULL); + assert_se(mkdtemp(tmp_dir)); va_start(ap, files); - while (files != NULL) { - _cleanup_free_ char *path = strappend(tmp_dir, files); - assert_se(touch_file(path, true, USEC_INFINITY, UID_INVALID, GID_INVALID, MODE_INVALID) == 0); + while (files) { + _cleanup_free_ char *path; + + assert_se(path = strappend(tmp_dir, files)); + (void) mkdir_parents(path, 0755); + assert_se(write_string_file(path, "foobar", WRITE_STRING_FILE_CREATE) >= 0); + files = va_arg(ap, const char *); } va_end(ap); @@ -36,7 +42,7 @@ static void setup_test_dir(char *tmp_dir, const char *files, ...) { static void test_conf_files_list(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_1, *search_2, *expect_a, *expect_b, *expect_c; + const char *root_dir, *search_1, *search_2, *expect_a, *expect_b, *expect_c, *mask; log_debug("/* %s */", __func__); @@ -45,8 +51,12 @@ static void test_conf_files_list(bool use_root) { "/dir2/a.conf", "/dir2/b.conf", "/dir2/c.foo", + "/dir2/d.conf", NULL); + mask = strjoina(tmp_dir, "/dir1/d.conf"); + assert_se(symlink("/dev/null", mask) >= 0); + if (use_root) { root_dir = tmp_dir; search_1 = "/dir1"; @@ -63,23 +73,23 @@ static void test_conf_files_list(bool use_root) { log_debug("/* Check when filtered by suffix */"); - assert_se(conf_files_list(&found_files, ".conf", root_dir, 0, search_1, search_2, NULL) == 0); + assert_se(conf_files_list(&found_files, ".conf", root_dir, CONF_FILES_FILTER_MASKED, search_1, search_2, NULL) == 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] == NULL); + assert_se(!found_files[2]); log_debug("/* Check when unfiltered */"); - assert_se(conf_files_list(&found_files2, NULL, root_dir, 0, search_1, search_2, NULL) == 0); + assert_se(conf_files_list(&found_files2, NULL, root_dir, CONF_FILES_FILTER_MASKED, search_1, search_2, NULL) == 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] == NULL); + assert_se(!found_files2[3]); assert_se(rm_rf(tmp_dir, REMOVE_ROOT|REMOVE_PHYSICAL) == 0); } -- 2.30.2