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.

diff --git a/src/basic/glob-util.c b/src/basic/glob-util.c
index 9eec8b3f092b687ec5b5b729616c80bcf99289c7..5843ef088f3f3a11e2ab276af8f4b75284e5d09a 100644 (file)
--- 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 4fa23f504897adba008098923d80f33d2bf4d83f..0e9088895f9227c6d34e79c6c83ab7c1f5eda66a 100644 (file)
--- 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 9bc7540e5ea77fafb82ece53cee0e0c6b8d760f3..3077ddd92edcef19ccd305f00f69f0ff9bbaab6f 100644 (file)
--- 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 1fdb3190427944f16043dc97e5c42e74657dd18f..e3c3f49887f826a106ae80474b23411d13159852 100644 (file)
--- 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 754dc774471544d39a0eb8717ea0dc4cf41bb87c..a9880f15c839810ebcd1d2e426c9c57636447e6d 100644 (file)
--- 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 16e2742fc1fbd237129a69047627ff90d8d2e1bb..be9462b97b78ff637d67fda144b15e43d081058f 100644 (file)
--- 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, ...)