From c6134d3e2f1d1d17b32b6e06556cd0c5429bc78a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 19 Jun 2019 15:20:13 +0200 Subject: [PATCH] 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 --- src/basic/conf-files.c | 8 +++--- src/basic/path-util.c | 38 ++------------------------- src/basic/path-util.h | 6 ++--- src/core/namespace.c | 2 +- src/cryptsetup/cryptsetup-generator.c | 2 +- src/nspawn/nspawn-cgroup.c | 6 ++--- src/nspawn/nspawn-mount.c | 16 +++++++---- src/nspawn/nspawn.c | 8 +++--- src/shared/dev-setup.c | 4 +-- src/shared/install.c | 6 ++--- src/shared/path-lookup.c | 6 ++--- src/test/test-conf-files.c | 10 +++---- src/test/test-path-util.c | 6 ++--- src/tmpfiles/tmpfiles.c | 2 +- 14 files changed, 44 insertions(+), 76 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 7c85022f08..a0d17e8a0e 100644 --- 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 55cb140d87..0ad1cce8a5 100644 --- 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 86c5a577cb..af4878b325 100644 --- 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 ec7af3ab1c..7aab2f7593 100644 --- 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 6d858f51ad..690ce0f8b0 100644 --- 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 f84bb39796..0462b46413 100644 --- 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 5a1bce4abc..e0740a6cc4 100644 --- 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 1c0187ae5c..e7f8cf389f 100644 --- 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 12e80e4b98..071ff7b30c 100644 --- 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 5391ac702b..b13b1f6d9e 100644 --- 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 442fde7b2d..1a005970f2 100644 --- 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 9fd8b6b590..07b681cf43 100644 --- 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 182991695a..7ef30ffc00 100644 --- 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 b859d94b92..334e3fb5f4 100644 --- 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();