path-util: get rid of prefix_root()

prefix_root() is equivalent to path_join() in almost all ways, hence
let's remove it.

There are subtle differences though: prefix_root() will try shorten
multiple "/" before and after the prefix. path_join() doesn't do that.
This means prefix_root() might return a string shorter than both its
inputs combined, while path_join() never does that. I like the
path_join() semantics better, hence I think dropping prefix_root() is
totally OK. In the end the strings generated by both functon should
always be identical in terms of path_equal() if not streq().

This leaves prefix_roota() in place. Ideally we'd have path_joina(), but
I don't think we can reasonably implement that as a macro. or maybe we
can? (if so, sounds like something for a later PR)

Also add in a few missing OOM checks

diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c
index 7c85022f08ee859080254b4110ff5d0a0447359a..a0d17e8a0e73c147b0195373c8f8c08649af90e2 100644 (file)
--- a/src/basic/conf-files.c
+++ b/src/basic/conf-files.c
@@ -207,7 +207,7 @@ int conf_files_insert(char ***strv, const char *root, char **dirs, const char *p
                                 _cleanup_free_ char *rdir = NULL;
                                 char *p1, *p2;
 
-                                rdir = prefix_root(root, *dir);
+                                rdir = path_join(root, *dir);
                                 if (!rdir)
                                         return -ENOMEM;
 
@@ -221,7 +221,7 @@ int conf_files_insert(char ***strv, const char *root, char **dirs, const char *p
                                 if (p2) {
                                         /* Our new entry has higher priority */
 
-                                        t = prefix_root(root, path);
+                                        t = path_join(root, path);
                                         if (!t)
                                                 return log_oom();
 
@@ -238,7 +238,7 @@ int conf_files_insert(char ***strv, const char *root, char **dirs, const char *p
         }
 
         /* The new file has lower priority than all the existing entries */
-        t = prefix_root(root, path);
+        t = path_join(root, path);
         if (!t)
                 return -ENOMEM;
 
@@ -313,7 +313,7 @@ int conf_files_list_with_replacement(
                 if (r < 0)
                         return log_error_errno(r, "Failed to extend config file list: %m");
 
-                p = prefix_root(root, replacement);
+                p = path_join(root, replacement);
                 if (!p)
                         return log_oom();
         }

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 55cb140d873508f2a330083ec3e2feef183b0605..0ad1cce8a5ed202ccb3c30255ee23ad208759c7d 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -256,7 +256,7 @@ char **path_strv_resolve(char **l, const char *root) {
 
                 if (root) {
                         orig = *s;
-                        t = prefix_root(root, orig);
+                        t = path_join(root, orig);
                         if (!t) {
                                 enomem = true;
                                 continue;
@@ -686,40 +686,6 @@ int mkfs_exists(const char *fstype) {
         return binary_is_good(mkfs);
 }
 
-char *prefix_root(const char *root, const char *path) {
-        char *n, *p;
-        size_t l;
-
-        /* If root is passed, prefixes path with it. Otherwise returns
-         * it as is. */
-
-        assert(path);
-
-        /* First, drop duplicate prefixing slashes from the path */
-        while (path[0] == '/' && path[1] == '/')
-                path++;
-
-        if (empty_or_root(root))
-                return strdup(path);
-
-        l = strlen(root) + 1 + strlen(path) + 1;
-
-        n = new(char, l);
-        if (!n)
-                return NULL;
-
-        p = stpcpy(n, root);
-
-        while (p > n && p[-1] == '/')
-                p--;
-
-        if (path[0] != '/')
-                *(p++) = '/';
-
-        strcpy(p, path);
-        return n;
-}
-
 int parse_path_argument_and_warn(const char *path, bool suppress_root, char **arg) {
         char *p;
         int r;
@@ -1027,7 +993,7 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version)
                 _cleanup_free_ char *path = NULL;
                 char *c, **name;
 
-                path = prefix_root(root, pattern);
+                path = path_join(root, pattern);
                 if (!path)
                         return -ENOMEM;
 

diff --git a/src/basic/path-util.h b/src/basic/path-util.h
index 86c5a577cb46d2db8ad0cf3e1256fe91a5878c15..af4878b3255b92ce4e85fe66e6438041f873e2cf 100644 (file)
--- a/src/basic/path-util.h
+++ b/src/basic/path-util.h
@@ -116,10 +116,8 @@ int mkfs_exists(const char *fstype);
              _slash && ((*_slash = 0), true);                           \
              _slash = strrchr((prefix), '/'))
 
-char *prefix_root(const char *root, const char *path);
-
-/* Similar to prefix_root(), but returns an alloca() buffer, or
- * possibly a const pointer into the path parameter */
+/* Similar to path_join(), but only works for two components, and only the first one may be NULL and returns
+ * an alloca() buffer, or possibly a const pointer into the path parameter */
 #define prefix_roota(root, path)                                        \
         ({                                                              \
                 const char* _path = (path), *_root = (root), *_ret;     \

diff --git a/src/core/namespace.c b/src/core/namespace.c
index ec7af3ab1c0204bb81dc8d9fdb0a1b6cd702a7cf..7aab2f7593c33fbb51bad8284612316b54e8ec6a 100644 (file)
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -444,7 +444,7 @@ static int prefix_where_needed(MountEntry *m, size_t n, const char *root_directo
                 if (m[i].has_prefix)
                         continue;
 
-                s = prefix_root(root_directory, mount_entry_path(m+i));
+                s = path_join(root_directory, mount_entry_path(m+i));
                 if (!s)
                         return -ENOMEM;
 

diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index 6d858f51ad086a5e67c7b860bba73a6f41ed22f7..690ce0f8b0976a8968f6785c8261bdce42c298e6 100644 (file)
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -235,7 +235,7 @@ static int create_disk(
                 if (r < 0)
                         return log_error_errno(r, "Failed to generate keydev mount unit: %m");
 
-                p = prefix_root(keydev_mount, password_escaped);
+                p = path_join(keydev_mount, password_escaped);
                 if (!p)
                         return log_oom();
 

diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c
index f84bb3979628a8bdb4560844c7b78c5ffcef8d9c..0462b4641307f1fb84a5813c7db4300d50579013 100644 (file)
--- a/src/nspawn/nspawn-cgroup.c
+++ b/src/nspawn/nspawn-cgroup.c
@@ -370,7 +370,7 @@ static int mount_legacy_cgns_supported(
                         if (streq(controller, tok))
                                 break;
 
-                        target = prefix_root("/sys/fs/cgroup/", tok);
+                        target = path_join("/sys/fs/cgroup/", tok);
                         if (!target)
                                 return log_oom();
 
@@ -451,7 +451,7 @@ static int mount_legacy_cgns_unsupported(
                 if (!controller)
                         break;
 
-                origin = prefix_root("/sys/fs/cgroup/", controller);
+                origin = path_join("/sys/fs/cgroup/", controller);
                 if (!origin)
                         return log_oom();
 
@@ -468,7 +468,7 @@ static int mount_legacy_cgns_unsupported(
                 else {
                         _cleanup_free_ char *target = NULL;
 
-                        target = prefix_root(dest, origin);
+                        target = path_join(dest, origin);
                         if (!target)
                                 return log_oom();
 

diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c
index 5a1bce4abc400fc273b82bb014a034fcb947e7d1..e0740a6cc46257791bd3d179b0d6ee9caa63098c 100644 (file)
--- a/src/nspawn/nspawn-mount.c
+++ b/src/nspawn/nspawn-mount.c
@@ -97,7 +97,7 @@ static char *resolve_source_path(const char *dest, const char *source) {
                 return NULL;
 
         if (source[0] == '+')
-                return prefix_root(dest, source + 1);
+                return path_join(dest, source + 1);
 
         return strdup(source);
 }
@@ -433,11 +433,11 @@ int mount_sysfs(const char *dest, MountSettingsMask mount_settings) {
         FOREACH_STRING(x, "block", "bus", "class", "dev", "devices", "kernel") {
                 _cleanup_free_ char *from = NULL, *to = NULL;
 
-                from = prefix_root(full, x);
+                from = path_join(full, x);
                 if (!from)
                         return log_oom();
 
-                to = prefix_root(top, x);
+                to = path_join(top, x);
                 if (!to)
                         return log_oom();
 
@@ -1190,7 +1190,9 @@ int setup_pivot_root(const char *directory, const char *pivot_root_new, const ch
          * Requires all file systems at directory and below to be mounted
          * MS_PRIVATE or MS_SLAVE so they can be moved.
          */
-        directory_pivot_root_new = prefix_root(directory, pivot_root_new);
+        directory_pivot_root_new = path_join(directory, pivot_root_new);
+        if (!directory_pivot_root_new)
+                return log_oom();
 
         /* Remount directory_pivot_root_new to make it movable. */
         r = mount_verbose(LOG_ERR, directory_pivot_root_new, directory_pivot_root_new, NULL, MS_BIND, NULL);
@@ -1204,7 +1206,11 @@ int setup_pivot_root(const char *directory, const char *pivot_root_new, const ch
                 }
 
                 remove_pivot_tmp = true;
-                pivot_tmp_pivot_root_old = prefix_root(pivot_tmp, pivot_root_old);
+                pivot_tmp_pivot_root_old = path_join(pivot_tmp, pivot_root_old);
+                if (!pivot_tmp_pivot_root_old) {
+                        r = log_oom();
+                        goto done;
+                }
 
                 r = mount_verbose(LOG_ERR, directory_pivot_root_new, pivot_tmp, NULL, MS_MOVE, NULL);
                 if (r < 0)

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 1c0187ae5c1122db788dd517849a646952c0478a..e7f8cf389f0f9ed9f0199d784904f862bc2c9d5c 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1895,11 +1895,11 @@ static int copy_devnodes(const char *dest) {
                 _cleanup_free_ char *from = NULL, *to = NULL;
                 struct stat st;
 
-                from = strappend("/dev/", d);
+                from = path_join("/dev/", d);
                 if (!from)
                         return log_oom();
 
-                to = prefix_root(dest, from);
+                to = path_join(dest, from);
                 if (!to)
                         return log_oom();
 
@@ -1945,7 +1945,7 @@ static int copy_devnodes(const char *dest) {
                         if (asprintf(&sl, "%s/%u:%u", dn, major(st.st_rdev), minor(st.st_rdev)) < 0)
                                 return log_oom();
 
-                        prefixed = prefix_root(dest, sl);
+                        prefixed = path_join(dest, sl);
                         if (!prefixed)
                                 return log_oom();
 
@@ -1972,7 +1972,7 @@ static int make_extra_nodes(const char *dest) {
                 _cleanup_free_ char *path = NULL;
                 DeviceNode *n = arg_extra_nodes + i;
 
-                path = prefix_root(dest, n->path);
+                path = path_join(dest, n->path);
                 if (!path)
                         return log_oom();
 

diff --git a/src/shared/dev-setup.c b/src/shared/dev-setup.c
index 12e80e4b98f128cba183eea36edc687778990a57..071ff7b30cff002e53f80ebc8ab2d3c4735029cc 100644 (file)
--- a/src/shared/dev-setup.c
+++ b/src/shared/dev-setup.c
@@ -36,7 +36,7 @@ int dev_setup(const char *prefix, uid_t uid, gid_t gid) {
                 }
 
                 if (prefix) {
-                        link_name = prefix_root(prefix, k);
+                        link_name = path_join(prefix, k);
                         if (!link_name)
                                 return -ENOMEM;
 
@@ -91,7 +91,7 @@ int make_inaccessible_nodes(const char *root, uid_t uid, gid_t gid) {
         for (i = 0; i < ELEMENTSOF(table); i++) {
                 _cleanup_free_ char *path = NULL;
 
-                path = prefix_root(root, table[i].name);
+                path = path_join(root, table[i].name);
                 if (!path)
                         return log_oom();
 

diff --git a/src/shared/install.c b/src/shared/install.c
index 5391ac702beac7dcc5873a06840f4f9051dba5c1..b13b1f6d9ea2073f759cb05cd6450480e0a33f90 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -794,7 +794,7 @@ static int find_symlinks_fd(
                         if (!path_is_absolute(dest)) {
                                 char *x;
 
-                                x = prefix_root(root_dir, dest);
+                                x = path_join(root_dir, dest);
                                 if (!x)
                                         return -ENOMEM;
 
@@ -1377,7 +1377,7 @@ static int unit_file_load_or_readlink(
 
                 if (path_is_absolute(target))
                         /* This is an absolute path, prefix the root so that we always deal with fully qualified paths */
-                        info->symlink_target = prefix_root(root_dir, target);
+                        info->symlink_target = path_join(root_dir, target);
                 else
                         /* This is a relative path, take it relative to the dir the symlink is located in. */
                         info->symlink_target = file_in_same_dir(path, target);
@@ -2188,7 +2188,7 @@ int unit_file_link(
                 if (!unit_name_is_valid(fn, UNIT_NAME_ANY))
                         return -EINVAL;
 
-                full = prefix_root(paths.root_dir, *i);
+                full = path_join(paths.root_dir, *i);
                 if (!full)
                         return -ENOMEM;
 

diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index 442fde7b2d2db732158a0e0974c7183b091f3b34..1a005970f2d8d595bd9d9a63bc9b660b45aae8bb 100644 (file)
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -456,13 +456,11 @@ static int patch_root_prefix(char **p, const char *root_dir) {
         if (!*p)
                 return 0;
 
-        c = prefix_root(root_dir, *p);
+        c = path_join(root_dir, *p);
         if (!c)
                 return -ENOMEM;
 
-        free(*p);
-        *p = c;
-
+        free_and_replace(*p, c);
         return 0;
 }
 

diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c
index 9fd8b6b59055532c2fd925d2be5efaa3b06401c4..07b681cf43c187efe63af6e404dbf5dc6c8fa215 100644 (file)
--- a/src/test/test-conf-files.c
+++ b/src/test/test-conf-files.c
@@ -102,11 +102,11 @@ static void test_conf_files_insert(const char *root) {
         char **dirs = STRV_MAKE("/dir1", "/dir2", "/dir3");
 
         _cleanup_free_ const char
-                *foo1 = prefix_root(root, "/dir1/foo.conf"),
-                *foo2 = prefix_root(root, "/dir2/foo.conf"),
-                *bar2 = prefix_root(root, "/dir2/bar.conf"),
-                *zzz3 = prefix_root(root, "/dir3/zzz.conf"),
-                *whatever = prefix_root(root, "/whatever.conf");
+                *foo1 = path_join(root, "/dir1/foo.conf"),
+                *foo2 = path_join(root, "/dir2/foo.conf"),
+                *bar2 = path_join(root, "/dir2/bar.conf"),
+                *zzz3 = path_join(root, "/dir3/zzz.conf"),
+                *whatever = path_join(root, "/whatever.conf");
 
         assert_se(conf_files_insert(&s, root, dirs, "/dir2/foo.conf") == 0);
         assert_se(strv_equal(s, STRV_MAKE(foo2)));

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 182991695a4d72dc91856137233da43e8c489fc5..7ef30ffc00ef171866e0c2ba2fe9e88ad97eb855 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -388,12 +388,12 @@ static void test_prefix_root_one(const char *r, const char *p, const char *expec
         _cleanup_free_ char *s = NULL;
         const char *t;
 
-        assert_se(s = prefix_root(r, p));
-        assert_se(streq_ptr(s, expected));
+        assert_se(s = path_join(r, p));
+        assert_se(path_equal_ptr(s, expected));
 
         t = prefix_roota(r, p);
         assert_se(t);
-        assert_se(streq_ptr(t, expected));
+        assert_se(path_equal_ptr(t, expected));
 }
 
 static void test_prefix_root(void) {

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index b859d94b92100654e14e6701f699fbb8acdf1688..334e3fb5f463267f3cb69647416cc7292e107633 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -2711,7 +2711,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool
         if (arg_root) {
                 char *p;
 
-                p = prefix_root(arg_root, i.path);
+                p = path_join(arg_root, i.path);
                 if (!p)
                         return log_oom();