From: Yu Watanabe Date: Tue, 1 Jul 2025 01:33:54 +0000 (+0900) Subject: conf-files: make conf_files_list() and friends internally use struct ConfFile X-Git-Tag: v258-rc1~101^2~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d38f87c56c6e2596d720a363b5a01c19166f5e2a;p=thirdparty%2Fsystemd.git conf-files: make conf_files_list() and friends internally use struct ConfFile No functional change, just refactoring. --- diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 0c5b464318a..dc3c17d2911 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -201,9 +201,15 @@ int conf_file_new(const char *path, const char *root, ChaseFlags chase_flags, Co return 0; } +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + conf_file_hash_ops, + char, string_hash_func, string_compare_func, + ConfFile, conf_file_free); + static int files_add( DIR *dir, - const char *dirpath, + const char *original_dirpath, + const char *resolved_dirpath, int rfd, const char *root, /* for logging, can be NULL */ Hashmap **files, @@ -214,7 +220,8 @@ static int files_add( int r; assert(dir); - assert(dirpath); + assert(original_dirpath); + assert(resolved_dirpath); assert(rfd >= 0 || rfd == AT_FDCWD); assert(files); assert(masked); @@ -222,46 +229,52 @@ static int files_add( root = strempty(root); FOREACH_DIRENT(de, dir, return log_debug_errno(errno, "Failed to read directory '%s/%s': %m", - root, skip_leading_slash(dirpath))) { + root, skip_leading_slash(original_dirpath))) { + + _cleanup_free_ char *original_path = path_join(original_dirpath, de->d_name); + if (!original_path) + return log_oom_debug(); /* Does this match the suffix? */ - if (suffix && !endswith(de->d_name, suffix)) + if (suffix && !endswith(de->d_name, suffix)) { + log_debug("Skipping file '%s/%s' with unexpected suffix.", root, skip_leading_slash(original_path)); continue; + } /* Has this file already been found in an earlier directory? */ if (hashmap_contains(*files, de->d_name)) { - log_debug("Skipping overridden file '%s/%s/%s'.", - root, skip_leading_slash(dirpath), de->d_name); + log_debug("Skipping overridden file '%s/%s'.", root, skip_leading_slash(original_path)); continue; } /* Has this been masked in an earlier directory? */ if ((flags & CONF_FILES_FILTER_MASKED) != 0 && set_contains(*masked, de->d_name)) { - log_debug("File '%s/%s/%s' is masked by previous entry.", - root, skip_leading_slash(dirpath), de->d_name); + log_debug("File '%s/%s' is masked by previous entry.", root, skip_leading_slash(original_path)); continue; } - _cleanup_free_ char *p = path_join(dirpath, de->d_name); + _cleanup_free_ char *p = path_join(resolved_dirpath, de->d_name); if (!p) return log_oom_debug(); _cleanup_free_ char *resolved_path = NULL; + _cleanup_close_ int fd = -EBADF; bool need_stat = (flags & (CONF_FILES_FILTER_MASKED | CONF_FILES_REGULAR | CONF_FILES_DIRECTORY | CONF_FILES_EXECUTABLE)) != 0; - struct stat st; - - if (!need_stat || FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) { + ChaseFlags chase_flags = CHASE_AT_RESOLVE_IN_ROOT; + if (!need_stat || FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) /* Even if no verification is requested, let's unconditionally call chaseat(), * to drop unsafe symlinks. */ + chase_flags |= CHASE_NONEXISTENT; - r = chaseat(rfd, p, CHASE_AT_RESOLVE_IN_ROOT | CHASE_NONEXISTENT, &resolved_path, /* ret_fd = */ NULL); - if (r < 0) { - log_debug_errno(r, "Failed to chase '%s/%s', ignoring: %m", - root, skip_leading_slash(p)); - continue; - } - if (r == 0 && FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) { + r = chaseat(rfd, p, chase_flags, &resolved_path, &fd); + if (r < 0) { + log_debug_errno(r, "Failed to chase '%s/%s', ignoring: %m", + root, skip_leading_slash(original_path)); + continue; + } + if (r == 0) { + if (FLAGS_SET(flags, CONF_FILES_FILTER_MASKED_BY_SYMLINK)) { /* If the path points to /dev/null in a image or so, then the device node may not exist. */ if (path_equal(skip_leading_slash(resolved_path), "dev/null")) { @@ -271,29 +284,25 @@ static int files_add( return log_oom_debug(); log_debug("File '%s/%s' is a mask (symlink to /dev/null).", - root, skip_leading_slash(p)); + root, skip_leading_slash(original_path)); continue; } - - /* If the flag is set, we need to have stat, hence, skip the entry. */ - log_debug_errno(SYNTHETIC_ERRNO(ENOENT), "Failed to chase '%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)); + if (need_stat) { + /* If we need to have stat, skip the entry. */ + log_debug_errno(SYNTHETIC_ERRNO(ENOENT), "Failed to chase '%s/%s', ignoring: %m", + root, skip_leading_slash(original_path)); continue; } + } - } else { - r = chase_and_statat(rfd, p, CHASE_AT_RESOLVE_IN_ROOT, &resolved_path, &st); - if (r < 0) { - log_debug_errno(r, "Failed to chase and stat '%s/%s', ignoring: %m", - root, skip_leading_slash(p)); - continue; - } + /* Even if we do not need stat, let's take stat now. The caller may use the info later. */ + struct stat st = {}; + if (fd >= 0 && fstat(fd, &st) < 0) { + log_debug_errno(errno, "Failed to stat '%s/%s', ignoring: %m", + root, skip_leading_slash(original_path)); + continue; } /* Is this a masking entry? */ @@ -303,7 +312,7 @@ static int files_add( if (r < 0) return log_oom_debug(); - log_debug("File '%s/%s' is a mask (symlink to /dev/null).", root, skip_leading_slash(p)); + log_debug("File '%s/%s' is a mask (symlink to /dev/null).", root, skip_leading_slash(original_path)); continue; } @@ -313,25 +322,25 @@ static int files_add( if (r < 0) return log_oom_debug(); - log_debug("File '%s/%s' is a mask (an empty file).", root, skip_leading_slash(p)); + log_debug("File '%s/%s' is a mask (an empty file).", root, skip_leading_slash(original_path)); continue; } if (FLAGS_SET(flags, CONF_FILES_REGULAR|CONF_FILES_DIRECTORY)) { if (!S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) { - log_debug("Ignoring '%s/%s', as it is neither a regular file or directory.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is neither a regular file or directory.", root, skip_leading_slash(original_path)); continue; } } else { /* Is this node a regular file? */ if (FLAGS_SET(flags, CONF_FILES_REGULAR) && !S_ISREG(st.st_mode)) { - log_debug("Ignoring '%s/%s', as it is not a regular file.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is not a regular file.", root, skip_leading_slash(original_path)); continue; } /* Is this node a directory? */ if (FLAGS_SET(flags, CONF_FILES_DIRECTORY) && !S_ISDIR(st.st_mode)) { - log_debug("Ignoring '%s/%s', as it is not a directory.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is not a directory.", root, skip_leading_slash(original_path)); continue; } } @@ -341,104 +350,103 @@ static int files_add( * here, as we care about whether the file is marked executable at all, and not whether it is * executable for us, because if so, such errors are stuff we should log about. */ if (FLAGS_SET(flags, CONF_FILES_EXECUTABLE) && (st.st_mode & 0111) == 0) { - log_debug("Ignoring '%s/%s', as it is not marked executable.", root, skip_leading_slash(p)); + log_debug("Ignoring '%s/%s', as it is not marked executable.", root, skip_leading_slash(original_path)); continue; } - _cleanup_free_ char *n = strdup(de->d_name); - if (!n) + _cleanup_(conf_file_freep) ConfFile *c = new(ConfFile, 1); + if (!c) return log_oom_debug(); - r = hashmap_ensure_put(files, &string_hash_ops_free_free, n, p); + *c = (ConfFile) { + .name = strdup(de->d_name), + .result = TAKE_PTR(p), + .original_path = TAKE_PTR(original_path), + .resolved_path = TAKE_PTR(resolved_path), + .fd = TAKE_FD(fd), + .st = st, + }; + + if (!c->name) + return log_oom_debug(); + + r = hashmap_ensure_put(files, &conf_file_hash_ops, c->name, c); if (r < 0) { assert(r == -ENOMEM); return log_oom_debug(); } assert(r > 0); - TAKE_PTR(n); - TAKE_PTR(p); + TAKE_PTR(c); } return 0; } static int copy_and_sort_files_from_hashmap(Hashmap *fh, const char *root, ConfFilesFlags flags, char ***ret) { - _cleanup_free_ char **sv = NULL; - _cleanup_strv_free_ char **files = NULL; - size_t n = 0; + _cleanup_strv_free_ char **results = NULL; + _cleanup_free_ ConfFile **files = NULL; + size_t n_files = 0, n_results = 0; int r; assert(ret); - r = hashmap_dump_sorted(fh, (void***) &sv, /* ret_n = */ NULL); + /* The entries in the array given by hashmap_dump_sorted() are still owned by the hashmap. + * Hence, do not use conf_file_free_many() for 'entries' */ + r = hashmap_dump_sorted(fh, (void***) &files, &n_files); if (r < 0) return log_oom_debug(); - /* The entries in the array given by hashmap_dump_sorted() are still owned by the hashmap. */ - STRV_FOREACH(s, sv) { - _cleanup_free_ char *p = NULL; + FOREACH_ARRAY(i, files, n_files) { + ConfFile *c = *i; - if (FLAGS_SET(flags, CONF_FILES_BASENAME)) { - r = path_extract_filename(*s, &p); - if (r < 0) - return log_debug_errno(r, "Failed to extract filename from '%s': %m", *s); - } else if (root) { - r = chaseat_prefix_root(*s, root, &p); + if (FLAGS_SET(flags, CONF_FILES_BASENAME)) + r = strv_extend_with_size(&results, &n_results, c->name); + else if (root) { + char *p; + + r = chaseat_prefix_root(c->result, root, &p); if (r < 0) - return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", *s, root); - } + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", c->result, root); - if (p) - r = strv_consume_with_size(&files, &n, TAKE_PTR(p)); - else - r = strv_extend_with_size(&files, &n, *s); + r = strv_consume_with_size(&results, &n_results, TAKE_PTR(p)); + } else + r = strv_extend_with_size(&results, &n_results, c->result); if (r < 0) return log_oom_debug(); } - *ret = TAKE_PTR(files); + *ret = TAKE_PTR(results); return 0; } -static int insert_replacement(Hashmap **fh, char *filename_replacement, char *resolved_replacement, char **ret) { - _cleanup_free_ char *fname = ASSERT_PTR(filename_replacement), *path = ASSERT_PTR(resolved_replacement); +static int insert_replacement(Hashmap **fh, ConfFile *replacement, const ConfFile **ret) { + _cleanup_(conf_file_freep) ConfFile *c = ASSERT_PTR(replacement); int r; assert(fh); + assert(ret); - /* This consumes the input filename and path. */ + /* This consumes the input ConfFile. */ - const char *existing = hashmap_get(*fh, fname); + ConfFile *existing = hashmap_get(*fh, c->name); if (existing) { - log_debug("An entry with higher priority already exists ('%s' -> '%s'), ignoring the replacement: %s", - fname, existing, path); - if (ret) - *ret = NULL; + log_debug("An entry with higher priority '%s' -> '%s' already exists, ignoring the replacement: %s", + existing->name, existing->result, c->original_path); + *ret = NULL; return 0; } - _cleanup_free_ char *copy = NULL; - if (ret) { - copy = strdup(path); - if (!copy) - return log_oom_debug(); - } - - r = hashmap_ensure_put(fh, &string_hash_ops_free_free, fname, path); + r = hashmap_ensure_put(fh, &conf_file_hash_ops, c->name, c); if (r < 0) { assert(r == -ENOMEM); return log_oom_debug(); } assert(r > 0); - log_debug("Inserted replacement: '%s' -> '%s'", fname, path); - - TAKE_PTR(fname); - TAKE_PTR(path); + log_debug("Inserted replacement: '%s' -> '%s'", c->name, c->result); - if (ret) - *ret = TAKE_PTR(copy); + *ret = TAKE_PTR(c); return 0; } @@ -450,33 +458,21 @@ static int conf_files_list_impl( const char * const *dirs, const char *replacement, Hashmap **ret, - char **ret_inserted) { + const ConfFile **ret_inserted) { _cleanup_hashmap_free_ Hashmap *fh = NULL; _cleanup_set_free_ Set *masked = NULL; + _cleanup_(conf_file_freep) ConfFile *c = NULL; + const ConfFile *inserted = NULL; int r; assert(rfd >= 0 || rfd == AT_FDCWD); assert(ret); - _cleanup_free_ char *filename_replacement = NULL, *resolved_dirpath_replacement = NULL, *resolved_replacement = NULL, *inserted = NULL; if (replacement) { - r = path_extract_filename(replacement, &filename_replacement); + r = conf_file_new_at(replacement, rfd, CHASE_NONEXISTENT, &c); if (r < 0) return r; - - _cleanup_free_ char *d = NULL; - r = path_extract_directory(replacement, &d); - if (r < 0) - return r; - - r = chaseat(rfd, d, CHASE_AT_RESOLVE_IN_ROOT | CHASE_NONEXISTENT, &resolved_dirpath_replacement, /* ret_fd = */ NULL); - if (r < 0) - return r; - - resolved_replacement = path_join(resolved_dirpath_replacement, filename_replacement); - if (!resolved_replacement) - return log_oom_debug(); } STRV_FOREACH(p, dirs) { @@ -486,30 +482,30 @@ static int conf_files_list_impl( 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%s', ignoring: %m", strempty(root), *p); + log_debug_errno(r, "Failed to chase and open directory '%s/%s', ignoring: %m", strempty(root), skip_leading_slash(*p)); continue; } - if (resolved_replacement && path_equal(resolved_dirpath_replacement, path)) { - r = insert_replacement(&fh, TAKE_PTR(filename_replacement), TAKE_PTR(resolved_replacement), ret_inserted ? &inserted : NULL); + if (c && streq_ptr(path_startswith(c->result, path), c->name)) { + r = insert_replacement(&fh, TAKE_PTR(c), &inserted); if (r < 0) return r; } - r = files_add(dir, path, rfd, root, &fh, &masked, suffix, flags); + r = files_add(dir, *p, path, rfd, root, &fh, &masked, suffix, flags); if (r == -ENOMEM) return r; } - if (resolved_replacement) { - r = insert_replacement(&fh, TAKE_PTR(filename_replacement), TAKE_PTR(resolved_replacement), ret_inserted ? &inserted : NULL); + if (c) { + r = insert_replacement(&fh, TAKE_PTR(c), &inserted); if (r < 0) return r; } *ret = TAKE_PTR(fh); if (ret_inserted) - *ret_inserted = TAKE_PTR(inserted); + *ret_inserted = inserted; return 0; } @@ -609,6 +605,7 @@ int conf_files_list_with_replacement( _cleanup_close_ int rfd = -EBADF; _cleanup_free_ char *root_abs = NULL; _cleanup_strv_free_ char **dirs_abs = NULL; + const ConfFile *c = NULL; int r; assert(ret_files); @@ -618,18 +615,14 @@ int conf_files_list_with_replacement( return r; r = conf_files_list_impl(".conf", rfd, root_abs, flags, (const char * const *) dirs_abs, - replacement, &fh, ret_inserted ? &inserted : NULL); + replacement, &fh, ret_inserted ? &c : NULL); if (r < 0) return r; - if (inserted) { - char *p; - - r = chaseat_prefix_root(inserted, root_abs, &p); + if (c) { + r = chaseat_prefix_root(c->result, root_abs, &inserted); 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); + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", c->result, empty_to_root(root_abs)); } r = copy_and_sort_files_from_hashmap(fh, empty_to_root(root_abs), flags, ret_files);