[systemd-commits] 5 commits - .gitignore Makefile.am src/shared src/test

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Thu Jun 19 21:11:40 PDT 2014


 .gitignore                 |    1 
 Makefile.am                |    9 ++++
 src/shared/conf-files.c    |   18 +++------
 src/shared/install.c       |   13 +-----
 src/shared/path-lookup.c   |    6 +--
 src/shared/path-util.c     |    6 +--
 src/shared/path-util.h     |    4 +-
 src/shared/util.c          |    7 ++-
 src/test/test-conf-files.c |   84 +++++++++++++++++++++++++++++++++++++++++++++
 src/test/test-path-util.c  |   31 ++++++++++++++++
 10 files changed, 147 insertions(+), 32 deletions(-)

New commits:
commit 375eadd911a9f83f89f1e7de5e05f44cc81e3642
Author: Michael Marineau <michael.marineau at coreos.com>
Date:   Thu Jun 19 19:07:06 2014 -0700

    shared: fix search_and_fopen with alternate roots
    
    Update for the current behavior of path_strv_resolve which now returns
    paths relative to the given root, not the full absolute paths.

diff --git a/src/shared/util.c b/src/shared/util.c
index c1e1f9f..aaf109e 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -5686,7 +5686,10 @@ static int search_and_fopen_internal(const char *path, const char *mode, const c
                 _cleanup_free_ char *p = NULL;
                 FILE *f;
 
-                p = strjoin(*i, "/", path, NULL);
+                if (root)
+                        p = strjoin(root, *i, "/", path, NULL);
+                else
+                        p = strjoin(*i, "/", path, NULL);
                 if (!p)
                         return -ENOMEM;
 

commit 09e00c524fd4d21a3508c27d01d265b8a6c9ae30
Author: Michael Marineau <michael.marineau at coreos.com>
Date:   Thu Jun 19 19:07:05 2014 -0700

    test: ensure conf_files_list returns absolute paths

diff --git a/.gitignore b/.gitignore
index c7c0793..979ab45 100644
--- a/.gitignore
+++ b/.gitignore
@@ -138,6 +138,7 @@
 /test-cgroup
 /test-cgroup-mask
 /test-cgroup-util
+/test-conf-files
 /test-daemon
 /test-date
 /test-device-nodes
diff --git a/Makefile.am b/Makefile.am
index fd3205d..bd26780 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1243,7 +1243,8 @@ tests += \
 	test-xml \
 	test-architecture \
 	test-socket-util \
-	test-fdset
+	test-fdset \
+	test-conf-files
 
 EXTRA_DIST += \
 	test/sched_idle_bad.service \
@@ -1600,6 +1601,12 @@ test_sched_prio_LDADD = \
 	libsystemd-core.la \
 	$(RT_LIBS)
 
+test_conf_files_SOURCES = \
+	src/test/test-conf-files.c
+
+test_conf_files_LDADD = \
+	libsystemd-shared.la
+
 # ------------------------------------------------------------------------------
 ## .PHONY so it always rebuilds it
 .PHONY: coverage lcov-run lcov-report coverage-sync
diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c
new file mode 100644
index 0000000..e801c59
--- /dev/null
+++ b/src/test/test-conf-files.c
@@ -0,0 +1,84 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Michael Marineau
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <stdio.h>
+#include <stdarg.h>
+
+#include "conf-files.h"
+#include "macro.h"
+#include "strv.h"
+#include "util.h"
+
+
+static void setup_test_dir(char *tmp_dir, const char *files, ...) {
+        va_list ap;
+
+        assert_se(mkdtemp(tmp_dir) != NULL);
+
+        va_start(ap, files);
+        while (files != NULL) {
+                _cleanup_free_ char *path = strappend(tmp_dir, files);
+                assert_se(touch_file(path, true, (usec_t) -1, (uid_t) -1, (gid_t) -1, 0) == 0);
+                files = va_arg(ap, const char *);
+        }
+        va_end(ap);
+}
+
+static void test_conf_files_list(bool use_root) {
+        char tmp_dir[] = "/tmp/test-conf-files-XXXXXX";
+        _cleanup_strv_free_ char **found_files = NULL;
+        const char *root_dir, *search_1, *search_2, *expect_a, *expect_b;
+
+        setup_test_dir(tmp_dir,
+                       "/dir1/a.conf",
+                       "/dir2/a.conf",
+                       "/dir2/b.conf",
+                       NULL);
+
+        if (use_root) {
+                root_dir = tmp_dir;
+                search_1 = "/dir1";
+                search_2 = "/dir2";
+        } else {
+                root_dir = NULL;
+                search_1 = strappenda(tmp_dir, "/dir1");
+                search_2 = strappenda(tmp_dir, "/dir2");
+        }
+
+        expect_a = strappenda(tmp_dir, "/dir1/a.conf");
+        expect_b = strappenda(tmp_dir, "/dir2/b.conf");
+
+        assert_se(conf_files_list(&found_files, ".conf", root_dir, 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(rm_rf_dangerous(tmp_dir, false, true, false) == 0);
+}
+
+int main(int argc, char **argv) {
+        test_conf_files_list(false);
+        test_conf_files_list(true);
+        return 0;
+}

commit cba2ef02722114da2b730d57f1e3bb43013d8921
Author: Michael Marineau <michael.marineau at coreos.com>
Date:   Thu Jun 19 19:07:04 2014 -0700

    conf-files: include root in returned file paths
    
    This restores the original root handling logic that was present prior to
    112cfb18 when path expansion moved to path_strv_canonicalize_absolute.
    That behavior partially went away in 12ed81d9.
    
    Alternatively all users of conf_files_list* could be updated to
    concatenate the paths themselves as unit_file_query_preset did but since
    no user needs the un-concatenated form that is pointless duplication.

diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c
index 44e137e..64ce8a0 100644
--- a/src/shared/conf-files.c
+++ b/src/shared/conf-files.c
@@ -37,20 +37,16 @@
 #include "hashmap.h"
 #include "conf-files.h"
 
-static int files_add(Hashmap *h, const char *dirpath, const char *suffix, const char *root) {
+static int files_add(Hashmap *h, const char *root, const char *path, const char *suffix) {
         _cleanup_closedir_ DIR *dir = NULL;
+        char *dirpath;
 
-        assert(dirpath);
+        assert(path);
         assert(suffix);
 
-        if (isempty(root))
-                dir = opendir(dirpath);
-        else {
-                const char *p;
+        dirpath = strappenda(root ? root : "", path);
 
-                p = strappenda3(root, "/", dirpath);
-                dir = opendir(p);
-        }
+        dir = opendir(dirpath);
         if (!dir) {
                 if (errno == ENOENT)
                         return 0;
@@ -118,7 +114,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const
                 return -ENOMEM;
 
         STRV_FOREACH(p, dirs) {
-                r = files_add(fh, *p, suffix, root);
+                r = files_add(fh, root, *p, suffix);
                 if (r == -ENOMEM) {
                         hashmap_free_free(fh);
                         return r;
diff --git a/src/shared/install.c b/src/shared/install.c
index 4f71793..190c554 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1776,7 +1776,7 @@ UnitFileState unit_file_get_state(
 
 int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name) {
         _cleanup_strv_free_ char **files = NULL;
-        char **i;
+        char **p;
         int r;
 
         assert(scope >= 0);
@@ -1804,17 +1804,10 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
         if (r < 0)
                 return r;
 
-        STRV_FOREACH(i, files) {
-                _cleanup_free_ char *buf = NULL;
+        STRV_FOREACH(p, files) {
                 _cleanup_fclose_ FILE *f;
-                const char *p;
-
-                if (root_dir)
-                        p = buf = strjoin(root_dir, "/", *i, NULL);
-                else
-                        p = *i;
 
-                f = fopen(p, "re");
+                f = fopen(*p, "re");
                 if (!f) {
                         if (errno == ENOENT)
                                 continue;

commit 3e8a78c8dceedb001587cb6c1eaa31cb8aa56729
Author: Michael Marineau <michael.marineau at coreos.com>
Date:   Thu Jun 19 19:07:03 2014 -0700

    test: unit test for using alternate roots with path_strv_resolve

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 9f8ae4d..4ee33a9 100644
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -20,10 +20,12 @@
 ***/
 
 #include <stdio.h>
+#include <unistd.h>
 
 #include "path-util.h"
 #include "util.h"
 #include "macro.h"
+#include "strv.h"
 
 
 static void test_path(void) {
@@ -191,11 +193,40 @@ static void test_make_relative(void) {
         test("//extra/////slashes///won't////fool///anybody//", "////extra///slashes////are/just///fine///", "../../../are/just/fine");
 }
 
+static void test_strv_resolve(void) {
+        char tmp_dir[] = "/tmp/test-path-util-XXXXXX";
+        _cleanup_strv_free_ char **search_dirs = NULL;
+        _cleanup_strv_free_ char **absolute_dirs = NULL;
+        char **d;
+
+        assert_se(mkdtemp(tmp_dir) != NULL);
+
+        search_dirs = strv_new("/dir1", "/dir2", "/dir3", NULL);
+        assert_se(search_dirs);
+        STRV_FOREACH(d, search_dirs) {
+                char *p = strappend(tmp_dir, *d);
+                assert_se(p);
+                assert_se(strv_push(&absolute_dirs, p) == 0);
+        }
+
+        assert_se(mkdir(absolute_dirs[0], 0700) == 0);
+        assert_se(mkdir(absolute_dirs[1], 0700) == 0);
+        assert_se(symlink("dir2", absolute_dirs[2]) == 0);
+
+        path_strv_resolve(search_dirs, tmp_dir);
+        assert_se(streq(search_dirs[0], "/dir1"));
+        assert_se(streq(search_dirs[1], "/dir2"));
+        assert_se(streq(search_dirs[2], "/dir2"));
+
+        assert_se(rm_rf_dangerous(tmp_dir, false, true, false) == 0);
+}
+
 int main(int argc, char **argv) {
         test_path();
         test_find_binary(argv[0]);
         test_prefixes();
         test_fsck_exists();
         test_make_relative();
+        test_strv_resolve();
         return 0;
 }

commit 7d8da2c9641c584ff977493eeb8148300dce8759
Author: Michael Marineau <michael.marineau at coreos.com>
Date:   Thu Jun 19 19:07:02 2014 -0700

    shared: rename path_strv_canonicalize_absolute functions
    
    Since 12ed81d9 path_strv_canonicalize_absolute leaves the search list
    relative to the given root directory instead of resolving paths to their
    true location as the name implies. To better reflect this behavior
    rename to the less strongly worded path_strv_resolve.

diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c
index 59bc8ce..44e137e 100644
--- a/src/shared/conf-files.c
+++ b/src/shared/conf-files.c
@@ -110,7 +110,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const
         assert(suffix);
 
         /* This alters the dirs string array */
-        if (!path_strv_canonicalize_absolute_uniq(dirs, root))
+        if (!path_strv_resolve_uniq(dirs, root))
                 return -ENOMEM;
 
         fh = hashmap_new(string_hash_func, string_compare_func);
diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index e072fd6..e0aaf44 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -284,7 +284,7 @@ int lookup_paths_init(
                 }
         }
 
-        if (!path_strv_canonicalize_absolute_uniq(p->unit_path, root_dir))
+        if (!path_strv_resolve_uniq(p->unit_path, root_dir))
                 return -ENOMEM;
 
         if (!strv_isempty(p->unit_path)) {
@@ -338,10 +338,10 @@ int lookup_paths_init(
                                 return -ENOMEM;
                 }
 
-                if (!path_strv_canonicalize_absolute_uniq(p->sysvinit_path, root_dir))
+                if (!path_strv_resolve_uniq(p->sysvinit_path, root_dir))
                         return -ENOMEM;
 
-                if (!path_strv_canonicalize_absolute_uniq(p->sysvrcnd_path, root_dir))
+                if (!path_strv_resolve_uniq(p->sysvrcnd_path, root_dir))
                         return -ENOMEM;
 
                 if (!strv_isempty(p->sysvinit_path)) {
diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index efe464d..d193494 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -238,7 +238,7 @@ char **path_strv_make_absolute_cwd(char **l) {
         return l;
 }
 
-char **path_strv_canonicalize_absolute(char **l, const char *prefix) {
+char **path_strv_resolve(char **l, const char *prefix) {
         char **s;
         unsigned k = 0;
         bool enomem = false;
@@ -323,12 +323,12 @@ char **path_strv_canonicalize_absolute(char **l, const char *prefix) {
         return l;
 }
 
-char **path_strv_canonicalize_absolute_uniq(char **l, const char *prefix) {
+char **path_strv_resolve_uniq(char **l, const char *prefix) {
 
         if (strv_isempty(l))
                 return l;
 
-        if (!path_strv_canonicalize_absolute(l, prefix))
+        if (!path_strv_resolve(l, prefix))
                 return NULL;
 
         return strv_uniq(l);
diff --git a/src/shared/path-util.h b/src/shared/path-util.h
index 6882d78..976d2b2 100644
--- a/src/shared/path-util.h
+++ b/src/shared/path-util.h
@@ -47,8 +47,8 @@ char* path_startswith(const char *path, const char *prefix) _pure_;
 bool path_equal(const char *a, const char *b) _pure_;
 
 char** path_strv_make_absolute_cwd(char **l);
-char** path_strv_canonicalize_absolute(char **l, const char *prefix);
-char** path_strv_canonicalize_absolute_uniq(char **l, const char *prefix);
+char** path_strv_resolve(char **l, const char *prefix);
+char** path_strv_resolve_uniq(char **l, const char *prefix);
 
 int path_is_mount_point(const char *path, bool allow_symlink);
 int path_is_read_only_fs(const char *path);
diff --git a/src/shared/util.c b/src/shared/util.c
index 34e9176..c1e1f9f 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -5679,7 +5679,7 @@ static int search_and_fopen_internal(const char *path, const char *mode, const c
         assert(mode);
         assert(_f);
 
-        if (!path_strv_canonicalize_absolute_uniq(search, root))
+        if (!path_strv_resolve_uniq(search, root))
                 return -ENOMEM;
 
         STRV_FOREACH(i, search) {



More information about the systemd-commits mailing list