diff --git a/TODO b/TODO index e75f111bb6..c87f978070 100644 --- a/TODO +++ b/TODO @@ -885,6 +885,10 @@ Features: * fstab-generator: default to tmpfs-as-root if only usr= is specified on the kernel cmdline +* initrd-parse-etc.service: can we skip daemon-reload if /sysroot/etc/fstab is missing? + Note that we start initrd-fs.target and initrd-cleanup.target there, so a straightforward + ConditionPathExists= is not enough. + * docs: bring http://www.freedesktop.org/wiki/Software/systemd/MyServiceCantGetRealtime up to date * add a job mode that will fail if a transaction would mean stopping diff --git a/man/50-xdg-data-dirs.sh b/man/50-xdg-data-dirs.sh index 89e9fbb599..ce062e394b 100755 --- a/man/50-xdg-data-dirs.sh +++ b/man/50-xdg-data-dirs.sh @@ -5,8 +5,8 @@ XDG_DATA_DIRS="${XDG_DATA_DIRS:-/usr/local/share/:/usr/share}" # add a directory if it exists if [[ -d /opt/foo/share ]]; then - XDG_DATA_DIRS=/opt/foo/share:${XDG_DATA_DIRS} + XDG_DATA_DIRS="/opt/foo/share:${XDG_DATA_DIRS}" fi # write our output -echo XDG_DATA_DIRS=$XDG_DATA_DIRS +echo "XDG_DATA_DIRS=${XDG_DATA_DIRS}" diff --git a/src/core/manager.c b/src/core/manager.c index 21358a0469..f3275a4070 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -4511,16 +4511,14 @@ void manager_status_printf(Manager *m, StatusType type, const char *status, cons va_end(ap); } -Set *manager_get_units_requiring_mounts_for(Manager *m, const char *path) { - char p[strlen(path)+1]; - +Set* manager_get_units_requiring_mounts_for(Manager *m, const char *path) { assert(m); assert(path); - strcpy(p, path); - path_simplify(p); + if (path_equal(path, "/")) + path = ""; - return hashmap_get(m->units_requiring_mounts_for, streq(p, "/") ? "" : p); + return hashmap_get(m->units_requiring_mounts_for, path); } int manager_update_failed_units(Manager *m, Unit *u, bool failed) { diff --git a/src/core/unit.c b/src/core/unit.c index 0cfb2aa281..3792cee711 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4562,45 +4562,43 @@ int unit_kill_context( } int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) { - _cleanup_free_ char *p = NULL; - UnitDependencyInfo di; int r; assert(u); assert(path); - /* Registers a unit for requiring a certain path and all its prefixes. We keep a hashtable of these paths in - * the unit (from the path to the UnitDependencyInfo structure indicating how to the dependency came to - * be). However, we build a prefix table for all possible prefixes so that new appearing mount units can easily - * determine which units to make themselves a dependency of. */ + /* Registers a unit for requiring a certain path and all its prefixes. We keep a hashtable of these + * paths in the unit (from the path to the UnitDependencyInfo structure indicating how to the + * dependency came to be). However, we build a prefix table for all possible prefixes so that new + * appearing mount units can easily determine which units to make themselves a dependency of. */ if (!path_is_absolute(path)) return -EINVAL; - r = hashmap_ensure_allocated(&u->requires_mounts_for, &path_hash_ops); - if (r < 0) - return r; + if (hashmap_contains(u->requires_mounts_for, path)) /* Exit quickly if the path is already covered. */ + return 0; - p = strdup(path); + _cleanup_free_ char *p = strdup(path); if (!p) return -ENOMEM; + /* Use the canonical form of the path as the stored key. We call path_is_normalized() + * only after simplification, since path_is_normalized() rejects paths with '.'. + * path_is_normalized() also verifies that the path fits in PATH_MAX. */ path = path_simplify(p); if (!path_is_normalized(path)) return -EPERM; - if (hashmap_contains(u->requires_mounts_for, path)) - return 0; - - di = (UnitDependencyInfo) { + UnitDependencyInfo di = { .origin_mask = mask }; - r = hashmap_put(u->requires_mounts_for, path, di.data); + r = hashmap_ensure_put(&u->requires_mounts_for, &path_hash_ops, p, di.data); if (r < 0) return r; - p = NULL; + assert(r > 0); + TAKE_PTR(p); /* path remains a valid pointer to the string stored in the hashmap */ char prefix[strlen(path) + 1]; PATH_FOREACH_PREFIX_MORE(prefix, path) { diff --git a/src/test/meson.build b/src/test/meson.build index cf49990b83..4999697804 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -370,6 +370,8 @@ tests += [ [], [threads]], + [['src/test/test-hash-funcs.c']], + [['src/test/test-bitmap.c']], [['src/test/test-xml.c']], diff --git a/src/test/test-hash-funcs.c b/src/test/test-hash-funcs.c new file mode 100644 index 0000000000..db68e3a1e7 --- /dev/null +++ b/src/test/test-hash-funcs.c @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "tests.h" +#include "hash-funcs.h" +#include "set.h" + +static void test_path_hash_set(void) { + /* The goal is to make sure that non-simplified path are hashed as expected, + * and that we don't need to simplify them beforehand. */ + + log_info("/* %s */", __func__); + + /* No freeing of keys, we operate on static strings here… */ + _cleanup_set_free_ Set *set = NULL; + + assert_se(set_isempty(set)); + assert_se(set_ensure_put(&set, &path_hash_ops, "foo") == 1); + assert_se(set_ensure_put(&set, &path_hash_ops, "foo") == 0); + assert_se(set_ensure_put(&set, &path_hash_ops, "bar") == 1); + assert_se(set_ensure_put(&set, &path_hash_ops, "bar") == 0); + assert_se(set_ensure_put(&set, &path_hash_ops, "/foo") == 1); + assert_se(set_ensure_put(&set, &path_hash_ops, "/bar") == 1); + assert_se(set_ensure_put(&set, &path_hash_ops, "/foo/.") == 0); + assert_se(set_ensure_put(&set, &path_hash_ops, "/./bar/./.") == 0); + + assert_se(set_contains(set, "foo")); + assert_se(set_contains(set, "bar")); + assert_se(set_contains(set, "./foo")); + assert_se(set_contains(set, "./foo/.")); + assert_se(set_contains(set, "./bar")); + assert_se(set_contains(set, "./bar/.")); + assert_se(set_contains(set, "/foo")); + assert_se(set_contains(set, "/bar")); + assert_se(set_contains(set, "//./foo")); + assert_se(set_contains(set, "///./foo/.")); + assert_se(set_contains(set, "////./bar")); + assert_se(set_contains(set, "/////./bar/.")); + + assert_se(set_contains(set, "foo/")); + assert_se(set_contains(set, "bar/")); + assert_se(set_contains(set, "./foo/")); + assert_se(set_contains(set, "./foo/./")); + assert_se(set_contains(set, "./bar/")); + assert_se(set_contains(set, "./bar/./")); + assert_se(set_contains(set, "/foo/")); + assert_se(set_contains(set, "/bar/")); + assert_se(set_contains(set, "//./foo/")); + assert_se(set_contains(set, "///./foo/./")); + assert_se(set_contains(set, "////./bar/")); + assert_se(set_contains(set, "/////./bar/./")); + + assert_se(!set_contains(set, "foo.")); + assert_se(!set_contains(set, ".bar")); + assert_se(!set_contains(set, "./foo.")); + assert_se(!set_contains(set, "./.foo/.")); + assert_se(!set_contains(set, "../bar")); + assert_se(!set_contains(set, "./bar/..")); + assert_se(!set_contains(set, "./foo..")); + assert_se(!set_contains(set, "/..bar")); + assert_se(!set_contains(set, "//../foo")); + assert_se(!set_contains(set, "///../foo/.")); + assert_se(!set_contains(set, "////../bar")); + assert_se(!set_contains(set, "/////../bar/.")); + + assert_se(!set_contains(set, "foo./")); + assert_se(!set_contains(set, ".bar/")); + assert_se(!set_contains(set, "./foo./")); + assert_se(!set_contains(set, "./.foo/./")); + assert_se(!set_contains(set, "../bar/")); + assert_se(!set_contains(set, "./bar/../")); + assert_se(!set_contains(set, "./foo../")); + assert_se(!set_contains(set, "/..bar/")); + assert_se(!set_contains(set, "//../foo/")); + assert_se(!set_contains(set, "///../foo/./")); + assert_se(!set_contains(set, "////../bar/")); + assert_se(!set_contains(set, "/////../bar/./")); +} + +int main(int argc, char **argv) { + test_setup_logging(LOG_INFO); + + test_path_hash_set(); +} diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 50aacdb967..4c041cd57f 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -126,6 +126,8 @@ static void test_path_compare_one(const char *a, const char *b, int expected) { } static void test_path_compare(void) { + log_info("/* %s */", __func__); + test_path_compare_one("/goo", "/goo", 0); test_path_compare_one("/goo", "/goo", 0); test_path_compare_one("//goo", "/goo", 0); @@ -138,6 +140,12 @@ static void test_path_compare(void) { test_path_compare_one("/x", "x/", 1); test_path_compare_one("x/", "/", -1); test_path_compare_one("/x/./y", "x/y", 1); + test_path_compare_one("/x/./y", "/x/y", 0); + test_path_compare_one("/x/./././y", "/x/y/././.", 0); + test_path_compare_one("./x/./././y", "./x/y/././.", 0); + test_path_compare_one(".", "./.", 0); + test_path_compare_one(".", "././.", 0); + test_path_compare_one("./..", ".", 1); test_path_compare_one("x/.y", "x/y", -1); test_path_compare_one("foo", "/foo", -1); test_path_compare_one("/foo", "/foo/bar", -1);