From 2a5f950e5643a74bef70b1c3c46ec33ad0e3fd41 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 23 Jun 2025 08:55:54 +0900 Subject: [PATCH] glob-util: rework safe_glob() Currently, callers of safe_glob() set an empty glob_t or glob_t with opendir func, and all other components are always zero. So, let's introduce safe_glob_full() which optionally takes opendir function, rather than glob_t, and returns result strv, rather than storing results in glob_t. Also, introduce safe_glob() which is a trivial wrapper of safe_glob_full() without opendir func. No functional change, just refactoring. --- src/basic/glob-util.c | 92 +++++++++++++++++++------------------- src/basic/glob-util.h | 10 +++-- src/core/exec-credential.c | 6 +-- src/core/execute.c | 8 ++-- src/test/test-glob-util.c | 27 +++++------ src/tmpfiles/tmpfiles.c | 16 +++---- 6 files changed, 78 insertions(+), 81 deletions(-) diff --git a/src/basic/glob-util.c b/src/basic/glob-util.c index 9eec8b3f092..5843ef088f3 100644 --- a/src/basic/glob-util.c +++ b/src/basic/glob-util.c @@ -6,81 +6,83 @@ #include "dirent-util.h" #include "errno-util.h" #include "glob-util.h" -#include "log.h" #include "string-util.h" #include "strv.h" -static void closedir_wrapper(void* v) { - (void) closedir(v); -} - -int safe_glob(const char *path, int flags, glob_t *pglob) { - int k; +DEFINE_TRIVIAL_DESTRUCTOR(closedir_wrapper, void, closedir); - /* We want to set GLOB_ALTDIRFUNC ourselves, don't allow it to be set. */ - assert(!(flags & GLOB_ALTDIRFUNC)); +int safe_glob_full(const char *path, int flags, opendir_t opendir_func, char ***ret) { + _cleanup_(globfree) glob_t g = { + .gl_closedir = closedir_wrapper, + .gl_readdir = (struct dirent* (*)(void *)) readdir_no_dot, + .gl_opendir = (void* (*)(const char *)) (opendir_func ?: opendir), + .gl_lstat = lstat, + .gl_stat = stat, + }; + int r; - if (!pglob->gl_closedir) - pglob->gl_closedir = closedir_wrapper; - if (!pglob->gl_readdir) - pglob->gl_readdir = (struct dirent *(*)(void *)) readdir_no_dot; - if (!pglob->gl_opendir) - pglob->gl_opendir = (void *(*)(const char *)) opendir; - if (!pglob->gl_lstat) - pglob->gl_lstat = lstat; - if (!pglob->gl_stat) - pglob->gl_stat = stat; + assert(path); errno = 0; - k = glob(path, flags | GLOB_ALTDIRFUNC, NULL, pglob); - if (k == GLOB_NOMATCH) + r = glob(path, flags | GLOB_ALTDIRFUNC, NULL, &g); + if (r == GLOB_NOMATCH) return -ENOENT; - if (k == GLOB_NOSPACE) + if (r == GLOB_NOSPACE) return -ENOMEM; - if (k != 0) + if (r != 0) return errno_or_else(EIO); - if (strv_isempty(pglob->gl_pathv)) + + if (strv_isempty(g.gl_pathv)) return -ENOENT; + if (ret) { + *ret = g.gl_pathv; + TAKE_STRUCT(g); /* To avoid the result being freed. */ + } + return 0; } -int glob_first(const char *path, char **ret_first) { - _cleanup_globfree_ glob_t g = {}; - int k; +int glob_first(const char *path, char **ret) { + _cleanup_strv_free_ char **v = NULL; + int r; assert(path); - k = safe_glob(path, GLOB_NOSORT|GLOB_BRACE, &g); - if (k == -ENOENT) { - if (ret_first) - *ret_first = NULL; + r = safe_glob(path, GLOB_NOSORT|GLOB_BRACE, &v); + if (r == -ENOENT) { + if (ret) + *ret = NULL; return false; } - if (k < 0) - return k; + if (r < 0) + return r; + + assert(!strv_isempty(v)); - if (ret_first) { - assert(g.gl_pathv && g.gl_pathv[0]); + if (ret) { + /* Free all results except for the first one. */ + STRV_FOREACH(p, strv_skip(v, 1)) + *p = mfree(*p); - char *first = strdup(g.gl_pathv[0]); - if (!first) - return log_oom_debug(); - *ret_first = first; + /* Then, take the first result. */ + *ret = TAKE_PTR(*v); } return true; } int glob_extend(char ***strv, const char *path, int flags) { - _cleanup_globfree_ glob_t g = {}; - int k; + char **v; + int r; + + assert(path); - k = safe_glob(path, GLOB_NOSORT|GLOB_BRACE|flags, &g); - if (k < 0) - return k; + r = safe_glob(path, GLOB_NOSORT|GLOB_BRACE|flags, &v); + if (r < 0) + return r; - return strv_extend_strv(strv, g.gl_pathv, false); + return strv_extend_strv_consume(strv, v, /* filter_duplicates = */ false); } int glob_non_glob_prefix(const char *path, char **ret) { diff --git a/src/basic/glob-util.h b/src/basic/glob-util.h index 4fa23f50489..0e9088895f9 100644 --- a/src/basic/glob-util.h +++ b/src/basic/glob-util.h @@ -5,11 +5,15 @@ #include "forward.h" -/* Note: this function modifies pglob to set various functions. */ -int safe_glob(const char *path, int flags, glob_t *pglob); +typedef DIR* (*opendir_t)(const char *); + +int safe_glob_full(const char *path, int flags, opendir_t opendir_func, char ***ret); +static inline int safe_glob(const char *path, int flags, char ***ret) { + return safe_glob_full(path, flags, NULL, ret); +} /* Note: which match is returned depends on the implementation/system and not guaranteed to be stable */ -int glob_first(const char *path, char **ret_first); +int glob_first(const char *path, char **ret); #define glob_exists(path) glob_first(path, NULL) int glob_extend(char ***strv, const char *path, int flags); diff --git a/src/core/exec-credential.c b/src/core/exec-credential.c index 9bc7540e5ea..3077ddd92ed 100644 --- a/src/core/exec-credential.c +++ b/src/core/exec-credential.c @@ -539,20 +539,20 @@ static int load_credential_glob( assert(search_path); STRV_FOREACH(d, search_path) { - _cleanup_globfree_ glob_t pglob = {}; + _cleanup_strv_free_ char **paths = NULL; _cleanup_free_ char *j = NULL; j = path_join(*d, ic->glob); if (!j) return -ENOMEM; - r = safe_glob(j, 0, &pglob); + r = safe_glob(j, /* flags = */ 0, &paths); if (r == -ENOENT) continue; if (r < 0) return r; - FOREACH_ARRAY(p, pglob.gl_pathv, pglob.gl_pathc) { + STRV_FOREACH(p, paths) { _cleanup_free_ char *fn = NULL; _cleanup_(erase_and_freep) char *data = NULL; size_t size; diff --git a/src/core/execute.c b/src/core/execute.c index 1fdb3190427..e3c3f49887f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -911,7 +911,7 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c, assert(ret); STRV_FOREACH(i, c->environment_files) { - _cleanup_globfree_ glob_t pglob = {}; + _cleanup_strv_free_ char **paths = NULL; bool ignore = false; char *fn = *i; @@ -927,7 +927,7 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c, } /* Filename supports globbing, take all matching files */ - r = safe_glob(fn, 0, &pglob); + r = safe_glob(fn, /* flags = */ 0, &paths); if (r < 0) { if (ignore) continue; @@ -935,9 +935,9 @@ static int exec_context_load_environment(const Unit *unit, const ExecContext *c, } /* When we don't match anything, -ENOENT should be returned */ - assert(pglob.gl_pathc > 0); + assert(!strv_isempty(paths)); - FOREACH_ARRAY(path, pglob.gl_pathv, pglob.gl_pathc) { + STRV_FOREACH(path, paths) { _cleanup_strv_free_ char **p = NULL; r = load_env_file(NULL, *path, &p); diff --git a/src/test/test-glob-util.c b/src/test/test-glob-util.c index 754dc774471..a9880f15c83 100644 --- a/src/test/test-glob-util.c +++ b/src/test/test-glob-util.c @@ -9,6 +9,7 @@ #include "fs-util.h" #include "glob-util.h" #include "rm-rf.h" +#include "strv.h" #include "tests.h" #include "tmpfile-util.h" @@ -54,31 +55,25 @@ TEST(glob_exists) { TEST(safe_glob) { char template[] = "/tmp/test-glob-util.XXXXXXX"; const char *fn, *fn2, *fname; + _cleanup_strv_free_ char **v = NULL; - _cleanup_globfree_ glob_t g = {}; - int r; - - assert_se(mkdtemp(template)); + ASSERT_NOT_NULL(mkdtemp(template)); fn = strjoina(template, "/*"); - r = safe_glob(fn, 0, &g); - assert_se(r == -ENOENT); + ASSERT_ERROR(safe_glob(fn, /* flags = */ 0, &v), ENOENT); fn2 = strjoina(template, "/.*"); - r = safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &g); - assert_se(r == -ENOENT); + ASSERT_ERROR(safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &v), ENOENT); fname = strjoina(template, "/.foobar"); - assert_se(touch(fname) == 0); + ASSERT_OK(touch(fname)); - r = safe_glob(fn, 0, &g); - assert_se(r == -ENOENT); + ASSERT_ERROR(safe_glob(fn, /* flags = */ 0, &v), ENOENT); - r = safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &g); - assert_se(r == 0); - assert_se(g.gl_pathc == 1); - ASSERT_STREQ(g.gl_pathv[0], fname); - ASSERT_NULL(g.gl_pathv[1]); + ASSERT_OK(safe_glob(fn2, GLOB_NOSORT|GLOB_BRACE, &v)); + ASSERT_EQ(strv_length(v), 1u); + ASSERT_STREQ(v[0], fname); + ASSERT_NULL(v[1]); (void) rm_rf(template, REMOVE_ROOT|REMOVE_PHYSICAL); } diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 16e2742fc1f..be9462b97b7 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -2570,23 +2570,21 @@ finish: } static int glob_item(Context *c, Item *i, action_t action) { - _cleanup_globfree_ glob_t g = { - .gl_opendir = (void *(*)(const char *)) opendir_nomod, - }; + _cleanup_strv_free_ char **paths = NULL; int r; assert(c); assert(i); assert(action); - r = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g); + r = safe_glob_full(i->path, GLOB_NOSORT|GLOB_BRACE, opendir_nomod, &paths); if (r == -ENOENT) return 0; if (r < 0) return log_error_errno(r, "Failed to glob '%s': %m", i->path); r = 0; - STRV_FOREACH(fn, g.gl_pathv) + STRV_FOREACH(fn, paths) /* We pass CREATION_EXISTING here, since if we are globbing for it, it always has to exist */ RET_GATHER(r, action(c, i, *fn, CREATION_EXISTING)); @@ -2598,23 +2596,21 @@ static int glob_item_recursively( Item *i, fdaction_t action) { - _cleanup_globfree_ glob_t g = { - .gl_opendir = (void *(*)(const char *)) opendir_nomod, - }; + _cleanup_strv_free_ char **paths = NULL; int r; assert(c); assert(i); assert(action); - r = safe_glob(i->path, GLOB_NOSORT|GLOB_BRACE, &g); + r = safe_glob_full(i->path, GLOB_NOSORT|GLOB_BRACE, opendir_nomod, &paths); if (r == -ENOENT) return 0; if (r < 0) return log_error_errno(r, "Failed to glob '%s': %m", i->path); r = 0; - STRV_FOREACH(fn, g.gl_pathv) { + STRV_FOREACH(fn, paths) { _cleanup_close_ int fd = -EBADF; /* Make sure we won't trigger/follow file object (such as device nodes, automounts, ...) -- 2.47.3