tree-wide: use path_join() instead of prefix_roota() in various cases

prefix_roota() is something we should stop using. It is bad for three
reasons:

1. As it names suggests it's supposed to be used when working relative
   to some root directory, but given it doesn't follow symlinks (and
   instead just stupidly joins paths) it is not a good choice for that.

2. More often than not it is currently used with inputs under control of
   the user, and that is icky given it typically allocates memory on the
   stack.

3. It's a redundant interface, where chase_symlinks() and path_join()
   already exist as better, safer interfaces.

Hence, let's start moving things from prefix_roota() to path_join() for
the cases where that's appropriate.

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 8ecbd69031b80eed3877830162f79973382bdae0..746c7cdfed0a87cd1c85644cffdf043833ebc93d 100644 (file)
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -2295,6 +2295,7 @@ static int unit_attach_pid_to_cgroup_via_bus(Unit *u, pid_t pid, const char *suf
 }
 
 int unit_attach_pids_to_cgroup(Unit *u, Set *pids, const char *suffix_path) {
+        _cleanup_free_ char *joined = NULL;
         CGroupMask delegated_mask;
         const char *p;
         void *pidp;
@@ -2320,8 +2321,13 @@ int unit_attach_pids_to_cgroup(Unit *u, Set *pids, const char *suffix_path) {
 
         if (isempty(suffix_path))
                 p = u->cgroup_path;
-        else
-                p = prefix_roota(u->cgroup_path, suffix_path);
+        else {
+                joined = path_join(u->cgroup_path, suffix_path);
+                if (!joined)
+                        return -ENOMEM;
+
+                p = joined;
+        }
 
         delegated_mask = unit_get_delegate_mask(u);
 

diff --git a/src/delta/delta.c b/src/delta/delta.c
index aa5a546bce58f4ec56b6782f5f68383fdfea9bd6..a08d35e43c392cbec67e054a1a752d7c2c88a62e 100644 (file)
--- a/src/delta/delta.c
+++ b/src/delta/delta.c
@@ -369,10 +369,12 @@ static int enumerate_dir(
 
 static int should_skip_path(const char *prefix, const char *suffix) {
 #if HAVE_SPLIT_USR
-        _cleanup_free_ char *target = NULL;
-        const char *dirname, *p;
+        _cleanup_free_ char *target = NULL, *dirname = NULL;
+        const char *p;
 
-        dirname = prefix_roota(prefix, suffix);
+        dirname = path_join(prefix, suffix);
+        if (!dirname)
+                return -ENOMEM;
 
         if (chase_symlinks(dirname, NULL, 0, &target, NULL) < 0)
                 return false;

diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c
index fa56a8322d4d34bd67200d03f17dafe9283634b8..a95f384ecbce0420be9fb8c75ea36ccbe44b3729 100644 (file)
--- a/src/gpt-auto-generator/gpt-auto-generator.c
+++ b/src/gpt-auto-generator/gpt-auto-generator.c
@@ -415,9 +415,9 @@ static int add_automount(
                 const char *description,
                 usec_t timeout) {
 
-        _cleanup_free_ char *unit = NULL;
+        _cleanup_free_ char *unit = NULL, *p = NULL;
         _cleanup_fclose_ FILE *f = NULL;
-        const char *opt = "noauto", *p;
+        const char *opt = "noauto";
         int r;
 
         assert(id);
@@ -443,7 +443,10 @@ static int add_automount(
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit name: %m");
 
-        p = prefix_roota(arg_dest, unit);
+        p = path_join(arg_dest, unit);
+        if (!p)
+                return log_oom();
+
         f = fopen(p, "wxe");
         if (!f)
                 return log_error_errno(errno, "Failed to create unit file %s: %m", unit);

diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
index 8574337bda9670a04fa7bcd7ea1ba47ce1a8c7b0..6bc4e6a019fa131b0d373b07f1aefa40fef496d7 100644 (file)
--- a/src/libsystemd/sd-device/sd-device.c
+++ b/src/libsystemd/sd-device/sd-device.c
@@ -2128,8 +2128,8 @@ int device_get_cached_sysattr_value(sd_device *device, const char *key, const ch
 /* We cache all sysattr lookups. If an attribute does not exist, it is stored
  * with a NULL value in the cache, otherwise the returned string is stored */
 _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **ret_value) {
-        _cleanup_free_ char *value = NULL;
-        const char *path, *syspath;
+        _cleanup_free_ char *value = NULL, *path = NULL;
+        const char *syspath;
         struct stat statbuf;
         int r;
 
@@ -2145,7 +2145,10 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr,
         if (r < 0)
                 return r;
 
-        path = prefix_roota(syspath, sysattr);
+        path = path_join(syspath, sysattr);
+        if (!path)
+                return -ENOMEM;
+
         if (lstat(path, &statbuf) < 0) {
                 int k;
 
@@ -2227,8 +2230,8 @@ static void device_remove_cached_sysattr_value(sd_device *device, const char *_k
 }
 
 _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr, const char *_value) {
-        _cleanup_free_ char *value = NULL;
-        const char *syspath, *path;
+        _cleanup_free_ char *value = NULL, *path = NULL;
+        const char *syspath;
         size_t len;
         int r;
 
@@ -2247,7 +2250,9 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
         if (r < 0)
                 return r;
 
-        path = prefix_roota(syspath, sysattr);
+        path = path_join(syspath, sysattr);
+        if (!path)
+                return -ENOMEM;
 
         len = strlen(_value);
 

diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c
index 3318f9217d9621589885abc20281f36ba2eb7c15..2a46f11d8ad87aeba3806aeefa2490187a33f5d7 100644 (file)
--- a/src/libsystemd/sd-journal/sd-journal.c
+++ b/src/libsystemd/sd-journal/sd-journal.c
@@ -1375,7 +1375,7 @@ static int add_file_by_name(
                 const char *prefix,
                 const char *filename) {
 
-        const char *path;
+        _cleanup_free_ char *path = NULL;
 
         assert(j);
         assert(prefix);
@@ -1387,28 +1387,35 @@ static int add_file_by_name(
         if (!file_type_wanted(j->flags, filename))
                 return 0;
 
-        path = prefix_roota(prefix, filename);
+        path = path_join(prefix, filename);
+        if (!path)
+                return -ENOMEM;
+
         return add_any_file(j, -1, path);
 }
 
-static void remove_file_by_name(
+static int remove_file_by_name(
                 sd_journal *j,
                 const char *prefix,
                 const char *filename) {
 
-        const char *path;
+        _cleanup_free_ char *path = NULL;
         JournalFile *f;
 
         assert(j);
         assert(prefix);
         assert(filename);
 
-        path = prefix_roota(prefix, filename);
+        path = path_join(prefix, filename);
+        if (!path)
+                return -ENOMEM;
+
         f = ordered_hashmap_get(j->files, path);
         if (!f)
-                return;
+                return 0;
 
         remove_file_real(j, f);
+        return 1;
 }
 
 static void remove_file_real(sd_journal *j, JournalFile *f) {
@@ -2620,7 +2627,7 @@ static void process_inotify_event(sd_journal *j, const struct inotify_event *e)
                         if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB))
                                 (void) add_file_by_name(j, d->path, e->name);
                         else if (e->mask & (IN_DELETE|IN_MOVED_FROM|IN_UNMOUNT))
-                                remove_file_by_name(j, d->path, e->name);
+                                (void) remove_file_by_name(j, d->path, e->name);
 
                 } else if (!d->is_root && e->len == 0) {
 

diff --git a/src/portable/portable.c b/src/portable/portable.c
index c6e74e9c27da2dd927ab9b23612073a3d299855a..256362355c08717dc22acd0d20a8dd1ed795a1cc 100644 (file)
--- a/src/portable/portable.c
+++ b/src/portable/portable.c
@@ -873,6 +873,8 @@ static int portable_changes_add_with_prefix(
                 const char *path,
                 const char *source) {
 
+        _cleanup_free_ char *path_buf = NULL, *source_buf = NULL;
+
         assert(path);
         assert(!changes == !n_changes);
 
@@ -880,10 +882,19 @@ static int portable_changes_add_with_prefix(
                 return 0;
 
         if (prefix) {
-                path = prefix_roota(prefix, path);
+                path_buf = path_join(prefix, path);
+                if (!path_buf)
+                        return -ENOMEM;
+
+                path = path_buf;
+
+                if (source) {
+                        source_buf = path_join(prefix, source);
+                        if (!source_buf)
+                                return -ENOMEM;
 
-                if (source)
-                        source = prefix_roota(prefix, source);
+                        source = source_buf;
+                }
         }
 
         return portable_changes_add(changes, n_changes, type_or_errno, path, source);
@@ -1098,7 +1109,8 @@ static int attach_unit_file(
 
         _cleanup_(unlink_and_freep) char *chroot_dropin = NULL, *profile_dropin = NULL;
         _cleanup_(rmdir_and_freep) char *dropin_dir = NULL;
-        const char *where, *path;
+        _cleanup_free_ char *path = NULL;
+        const char *where;
         int r;
 
         assert(paths);
@@ -1115,7 +1127,10 @@ static int attach_unit_file(
         } else
                 (void) portable_changes_add(changes, n_changes, PORTABLE_MKDIR, where, NULL);
 
-        path = prefix_roota(where, m->name);
+        path = path_join(where, m->name);
+        if (!path)
+                return -ENOMEM;
+
         dropin_dir = strjoin(path, ".d");
         if (!dropin_dir)
                 return -ENOMEM;

diff --git a/src/shared/generator.c b/src/shared/generator.c
index b4efcf6d0bd6e54c01dc79985a046e62b9bdf245..681b97c6bd5f5b5cef5f105b8e1d4aad878a1340 100644 (file)
--- a/src/shared/generator.c
+++ b/src/shared/generator.c
@@ -29,11 +29,13 @@ int generator_open_unit_file(
                 const char *name,
                 FILE **file) {
 
-        const char *unit;
+        _cleanup_free_ char *unit = NULL;
         FILE *f;
         int r;
 
-        unit = prefix_roota(dest, name);
+        unit = path_join(dest, name);
+        if (!unit)
+                return log_oom();
 
         r = fopen_unlocked(unit, "wxe", &f);
         if (r < 0) {
@@ -352,8 +354,8 @@ int generator_hook_up_mkswap(
                 const char *what) {
 
         _cleanup_free_ char *node = NULL, *unit = NULL, *escaped = NULL, *where_unit = NULL;
+        _cleanup_free_ char *unit_file = NULL;
         _cleanup_fclose_ FILE *f = NULL;
-        const char *unit_file;
         int r;
 
         node = fstab_node_to_udev_node(what);
@@ -371,7 +373,10 @@ int generator_hook_up_mkswap(
                 return log_error_errno(r, "Failed to make unit instance name from path \"%s\": %m",
                                        node);
 
-        unit_file = prefix_roota(dir, unit);
+        unit_file = path_join(dir, unit);
+        if (!unit_file)
+                return log_oom();
+
         log_debug("Creating %s", unit_file);
 
         escaped = cescape(node);
@@ -421,9 +426,8 @@ int generator_hook_up_mkfs(
                 const char *where,
                 const char *type) {
 
-        _cleanup_free_ char *node = NULL, *unit = NULL, *escaped = NULL, *where_unit = NULL;
+        _cleanup_free_ char *node = NULL, *unit = NULL, *unit_file = NULL, *escaped = NULL, *where_unit = NULL;
         _cleanup_fclose_ FILE *f = NULL;
-        const char *unit_file;
         int r;
 
         node = fstab_node_to_udev_node(what);
@@ -446,7 +450,10 @@ int generator_hook_up_mkfs(
                 return log_error_errno(r, "Failed to make unit instance name from path \"%s\": %m",
                                        node);
 
-        unit_file = prefix_roota(dir, unit);
+        unit_file = path_join(dir, unit);
+        if (!unit_file)
+                return log_oom();
+
         log_debug("Creating %s", unit_file);
 
         escaped = cescape(node);
@@ -499,9 +506,8 @@ int generator_hook_up_growfs(
                 const char *where,
                 const char *target) {
 
-        _cleanup_free_ char *unit = NULL, *escaped = NULL, *where_unit = NULL;
+        _cleanup_free_ char *unit = NULL, *escaped = NULL, *where_unit = NULL, *unit_file = NULL;
         _cleanup_fclose_ FILE *f = NULL;
-        const char *unit_file;
         int r;
 
         assert(dir);
@@ -521,7 +527,10 @@ int generator_hook_up_growfs(
                 return log_error_errno(r, "Failed to make unit name from path \"%s\": %m",
                                        where);
 
-        unit_file = prefix_roota(dir, unit);
+        unit_file = path_join(dir, unit);
+        if (!unit_file)
+                return log_oom();
+
         log_debug("Creating %s", unit_file);
 
         f = fopen(unit_file, "wxe");

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 14ae873dc05c8ba167f52a07fd3f5b5481331410..3c5df6c3ec86cfec790b1ec2e3569111c17f7019 100644 (file)
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -80,16 +80,16 @@ static void free_sysvstub_hashmapp(Hashmap **h) {
 }
 
 static int add_alias(const char *service, const char *alias) {
-        const char *link;
-        int r;
+        _cleanup_free_ char *link = NULL;
 
         assert(service);
         assert(alias);
 
-        link = prefix_roota(arg_dest, alias);
+        link = path_join(arg_dest, alias);
+        if (!link)
+                return -ENOMEM;
 
-        r = symlink(service, link);
-        if (r < 0) {
+        if (symlink(service, link) < 0) {
                 if (errno == EEXIST)
                         return 0;
 
@@ -100,9 +100,8 @@ static int add_alias(const char *service, const char *alias) {
 }
 
 static int generate_unit_file(SysvStub *s) {
-        _cleanup_free_ char *path_escaped = NULL;
+        _cleanup_free_ char *path_escaped = NULL, *unit = NULL;
         _cleanup_fclose_ FILE *f = NULL;
-        const char *unit;
         int r;
 
         assert(s);
@@ -114,7 +113,9 @@ static int generate_unit_file(SysvStub *s) {
         if (!path_escaped)
                 return log_oom();
 
-        unit = prefix_roota(arg_dest, s->name);
+        unit = path_join(arg_dest, s->name);
+        if (!unit)
+                return log_oom();
 
         /* We might already have a symlink with the same name from a Provides:,
          * or from backup files like /etc/init.d/foo.bak. Real scripts always win,