machinectl/machined: implement "rename", "clone", "read-only" verbs for machine images

10 files changed, 471 insertions, 24 deletions

diff --git a/configure.ac b/configure.ac
index 1df3a18d50..0496f5e3ed 100644
--- a/configure.ac
+++ b/configure.ac
@@ -310,7 +310,7 @@ LIBS="$save_LIBS"
 
 AC_CHECK_FUNCS([memfd_create])
 AC_CHECK_FUNCS([__secure_getenv secure_getenv])
-AC_CHECK_DECLS([gettid, pivot_root, name_to_handle_at, setns, getrandom, LO_FLAGS_PARTSCAN],
+AC_CHECK_DECLS([gettid, pivot_root, name_to_handle_at, setns, getrandom, renameat2, LO_FLAGS_PARTSCAN],
                [], [], [[
 #include <sys/types.h>
 #include <unistd.h>
diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c
index be2369953c..1030cd314f 100644
--- a/src/machine/image-dbus.c
+++ b/src/machine/image-dbus.c
@@ -236,6 +236,95 @@ static int method_remove(
         return sd_bus_reply_method_return(message, NULL);
 }
 
+static int method_rename(
+                sd_bus *bus,
+                sd_bus_message *message,
+                void *userdata,
+                sd_bus_error *error) {
+
+        _cleanup_(image_unrefp) Image *image = NULL;
+        const char *new_name;
+        int r;
+
+        assert(bus);
+        assert(message);
+
+        r = image_find_by_bus_path_with_error(sd_bus_message_get_path(message), &image, error);
+        if (r < 0)
+                return r;
+
+        r = sd_bus_message_read(message, "s", &new_name);
+        if (r < 0)
+                return r;
+
+        if (!image_name_is_valid(new_name))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", new_name);
+
+        r = image_rename(image, new_name);
+        if (r < 0)
+                return r;
+
+        return sd_bus_reply_method_return(message, NULL);
+}
+
+static int method_clone(
+                sd_bus *bus,
+                sd_bus_message *message,
+                void *userdata,
+                sd_bus_error *error) {
+
+        _cleanup_(image_unrefp) Image *image = NULL;
+        const char *new_name;
+        int r, read_only;
+
+        assert(bus);
+        assert(message);
+
+        r = image_find_by_bus_path_with_error(sd_bus_message_get_path(message), &image, error);
+        if (r < 0)
+                return r;
+
+        r = sd_bus_message_read(message, "sb", &new_name, &read_only);
+        if (r < 0)
+                return r;
+
+        if (!image_name_is_valid(new_name))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", new_name);
+
+        r = image_clone(image, new_name, read_only);
+        if (r < 0)
+                return r;
+
+        return sd_bus_reply_method_return(message, NULL);
+}
+
+static int method_mark_read_only(
+                sd_bus *bus,
+                sd_bus_message *message,
+                void *userdata,
+                sd_bus_error *error) {
+
+        _cleanup_(image_unrefp) Image *image = NULL;
+        int r, read_only;
+
+        assert(bus);
+        assert(message);
+
+        r = image_find_by_bus_path_with_error(sd_bus_message_get_path(message), &image, error);
+        if (r < 0)
+                return r;
+
+        r = sd_bus_message_read(message, "b", &read_only);
+        if (r < 0)
+                return r;
+
+        r = image_read_only(image, read_only);
+        if (r < 0)
+                return r;
+
+        return sd_bus_reply_method_return(message, NULL);
+}
+
 const sd_bus_vtable image_vtable[] = {
         SD_BUS_VTABLE_START(0),
         SD_BUS_PROPERTY("Name",                  "s", property_get_name,      0, 0),
@@ -245,6 +334,9 @@ const sd_bus_vtable image_vtable[] = {
         SD_BUS_PROPERTY("CreationTimestamp",     "t", property_get_crtime,    0, 0),
         SD_BUS_PROPERTY("ModificationTimestamp", "t", property_get_mtime,     0, 0),
         SD_BUS_METHOD("Remove", NULL, NULL, method_remove, 0),
+        SD_BUS_METHOD("Rename", "s", NULL, method_rename, 0),
+        SD_BUS_METHOD("Clone", "sb", NULL, method_clone, 0),
+        SD_BUS_METHOD("MarkeReadOnly", "b", NULL, method_mark_read_only, 0),
         SD_BUS_VTABLE_END
 };
 
diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
index 500e5b721a..893b429342 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -1286,13 +1286,11 @@ static int login_machine(int argc, char *argv[], void *userdata) {
 static int remove_image(int argc, char *argv[], void *userdata) {
         _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         sd_bus *bus = userdata;
-        int i;
+        int r, i;
 
         assert(bus);
 
         for (i = 1; i < argc; i++) {
-                int r;
-
                 r = sd_bus_call_method(
                                 bus,
                                 "org.freedesktop.machine1",
@@ -1311,6 +1309,80 @@ static int remove_image(int argc, char *argv[], void *userdata) {
         return 0;
 }
 
+static int rename_image(int argc, char *argv[], void *userdata) {
+        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+        sd_bus *bus = userdata;
+        int r;
+
+        r = sd_bus_call_method(
+                        bus,
+                        "org.freedesktop.machine1",
+                        "/org/freedesktop/machine1",
+                        "org.freedesktop.machine1.Manager",
+                        "RenameImage",
+                        &error,
+                        NULL,
+                        "ss", argv[1], argv[2]);
+        if (r < 0) {
+                log_error("Could not rename image: %s", bus_error_message(&error, -r));
+                return r;
+        }
+
+        return 0;
+}
+
+static int clone_image(int argc, char *argv[], void *userdata) {
+        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+        sd_bus *bus = userdata;
+        int r;
+
+        r = sd_bus_call_method(
+                        bus,
+                        "org.freedesktop.machine1",
+                        "/org/freedesktop/machine1",
+                        "org.freedesktop.machine1.Manager",
+                        "CloneImage",
+                        &error,
+                        NULL,
+                        "ssb", argv[1], argv[2], arg_read_only);
+        if (r < 0) {
+                log_error("Could not clone image: %s", bus_error_message(&error, -r));
+                return r;
+        }
+
+        return 0;
+}
+
+static int read_only_image(int argc, char *argv[], void *userdata) {
+        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+        sd_bus *bus = userdata;
+        int b = true, r;
+
+        if (argc > 2) {
+                b = parse_boolean(argv[2]);
+                if (b < 0) {
+                        log_error("Failed to parse boolean argument: %s", argv[2]);
+                        return -EINVAL;
+                }
+        }
+
+        r = sd_bus_call_method(
+                        bus,
+                        "org.freedesktop.machine1",
+                        "/org/freedesktop/machine1",
+                        "org.freedesktop.machine1.Manager",
+                        "MarkImageReadOnly",
+                        &error,
+                        NULL,
+                        "sb", argv[1], b);
+        if (r < 0) {
+                log_error("Could not mark image read-only: %s", bus_error_message(&error, -r));
+                return r;
+        }
+
+        return 0;
+}
+
 static int help(int argc, char *argv[], void *userdata) {
 
         printf("%s [OPTIONS...] {COMMAND} ...\n\n"
@@ -1336,15 +1408,18 @@ static int help(int argc, char *argv[], void *userdata) {
                "  login NAME                  Get a login prompt on a container\n"
                "  poweroff NAME...            Power off one or more containers\n"
                "  reboot NAME...              Reboot one or more containers\n"
-               "  kill NAME...                Send signal to processes of a VM/container\n"
                "  terminate NAME...           Terminate one or more VMs/containers\n"
-               "  bind NAME PATH [PATH]       Bind mount a path from the host into a container\n"
+               "  kill NAME...                Send signal to processes of a VM/container\n"
                "  copy-to NAME PATH [PATH]    Copy files from the host to a container\n"
-               "  copy-from NAME PATH [PATH]  Copy files from a container to the host\n\n"
+               "  copy-from NAME PATH [PATH]  Copy files from a container to the host\n"
+               "  bind NAME PATH [PATH]       Bind mount a path from the host into a container\n\n"
                "Image Commands:\n"
                "  list-images                 Show available images\n"
                "  image-status NAME...        Show image details\n"
                "  show-image NAME...          Show properties of image\n"
+               "  clone NAME NAME             Clone an image\n"
+               "  rename NAME NAME            Rename an image\n"
+               "  read-only NAME [BOOL]       Mark or unmark image read-only\n"
                "  remove NAME...              Remove an image\n",
                program_invocation_short_name);
 
@@ -1482,6 +1557,9 @@ static int machinectl_main(int argc, char *argv[], sd_bus *bus) {
                 { "copy-to",     3,        4,        0,            copy_files        },
                 { "copy-from",   3,        4,        0,            copy_files        },
                 { "remove",      2,        VERB_ANY, 0,            remove_image      },
+                { "rename",      3,        3,        0,            rename_image      },
+                { "clone",       3,        3,        0,            clone_image       },
+                { "read-only",   2,        3,        0,            read_only_image   },
                 {}
         };
 
diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c
index 9296377c6f..18b772d42d 100644
--- a/src/machine/machined-dbus.c
+++ b/src/machine/machined-dbus.c
@@ -588,6 +588,90 @@ static int method_remove_image(sd_bus *bus, sd_bus_message *message, void *userd
         return sd_bus_reply_method_return(message, NULL);
 }
 
+static int method_rename_image(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        _cleanup_(image_unrefp) Image* i = NULL;
+        const char *old_name, *new_name;
+        int r;
+
+        assert(bus);
+        assert(message);
+
+        r = sd_bus_message_read(message, "ss", &old_name, &new_name);
+        if (r < 0)
+                return r;
+
+        if (!image_name_is_valid(old_name))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", old_name);
+        if (!image_name_is_valid(new_name))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", new_name);
+
+        r = image_find(old_name, &i);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", old_name);
+
+        r = image_rename(i, new_name);
+        if (r < 0)
+                return r;
+
+        return sd_bus_reply_method_return(message, NULL);
+}
+
+static int method_clone_image(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        _cleanup_(image_unrefp) Image *i = NULL;
+        const char *old_name, *new_name;
+        int read_only, r;
+
+        assert(bus);
+        r = sd_bus_message_read(message, "ssb", &old_name, &new_name, &read_only);
+        if (r < 0)
+                return r;
+
+        if (!image_name_is_valid(old_name))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", old_name);
+        if (!image_name_is_valid(new_name))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", new_name);
+
+        r = image_find(old_name, &i);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", old_name);
+
+        r = image_clone(i, new_name, read_only);
+        if (r < 0)
+                return r;
+
+        return sd_bus_reply_method_return(message, NULL);
+}
+
+static int method_mark_image_read_only(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        _cleanup_(image_unrefp) Image *i = NULL;
+        const char *name;
+        int read_only, r;
+
+        assert(bus);
+        r = sd_bus_message_read(message, "sb", &name, &read_only);
+        if (r < 0)
+                return r;
+
+        if (!image_name_is_valid(name))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is invalid.", name);
+
+        r = image_find(name, &i);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_IMAGE, "No image '%s' known", name);
+
+        r = image_read_only(i, read_only);
+        if (r < 0)
+                return r;
+
+        return sd_bus_reply_method_return(message, NULL);
+}
+
 const sd_bus_vtable manager_vtable[] = {
         SD_BUS_VTABLE_START(0),
         SD_BUS_METHOD("GetMachine", "s", "o", method_get_machine, SD_BUS_VTABLE_UNPRIVILEGED),
@@ -606,6 +690,9 @@ const sd_bus_vtable manager_vtable[] = {
         SD_BUS_METHOD("OpenMachinePTY", "s", "hs", method_open_machine_pty, 0),
         SD_BUS_METHOD("OpenMachineLogin", "s", "hs", method_open_machine_login, SD_BUS_VTABLE_UNPRIVILEGED),
         SD_BUS_METHOD("RemoveImage", "s", NULL, method_remove_image, 0),
+        SD_BUS_METHOD("RenameImage", "ss", NULL, method_rename_image, 0),
+        SD_BUS_METHOD("CloneImage", "ssb", NULL, method_clone_image, 0),
+        SD_BUS_METHOD("MarkImageReadOnly", "sb", NULL, method_mark_image_read_only, 0),
         SD_BUS_SIGNAL("MachineNew", "so", 0),
         SD_BUS_SIGNAL("MachineRemoved", "so", 0),
         SD_BUS_VTABLE_END
diff --git a/src/shared/copy.c b/src/shared/copy.c
index 92f6e1e114..3df636704c 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -120,6 +120,7 @@ static int fd_copy_symlink(int df, const char *from, const struct stat *st, int
 
 static int fd_copy_regular(int df, const char *from, const struct stat *st, int dt, const char *to) {
         _cleanup_close_ int fdf = -1, fdt = -1;
+        struct timespec ts[2];
         int r, q;
 
         assert(from);
@@ -146,7 +147,10 @@ static int fd_copy_regular(int df, const char *from, const struct stat *st, int
         if (fchmod(fdt, st->st_mode & 07777) < 0)
                 r = -errno;
 
-        (void) copy_times(fdf, fdt);
+        ts[0] = st->st_atim;
+        ts[1] = st->st_mtim;
+        (void) futimens(fdt, ts);
+
         (void) copy_xattr(fdf, fdt);
 
         q = close(fdt);
@@ -243,14 +247,19 @@ static int fd_copy_directory(
         r = 0;
 
         if (created) {
+                struct timespec ut[2] = {
+                        st->st_atim,
+                        st->st_mtim
+                };
+
                 if (fchown(fdt, st->st_uid, st->st_gid) < 0)
                         r = -errno;
 
                 if (fchmod(fdt, st->st_mode & 07777) < 0)
                         r = -errno;
 
-                (void) copy_times(fdf, fdt);
-                (void) copy_xattr(fdf, fdt);
+                (void) futimens(fdt, ut);
+                (void) copy_xattr(dirfd(d), fdt);
         }
 
         FOREACH_DIRENT(de, d, return -errno) {
@@ -356,9 +365,11 @@ int copy_file(const char *from, const char *to, int flags, mode_t mode) {
         assert(from);
         assert(to);
 
-        fdt = open(to, flags|O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY, mode);
-        if (fdt < 0)
-                return -errno;
+        RUN_WITH_UMASK(0000) {
+                fdt = open(to, flags|O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY, mode);
+                if (fdt < 0)
+                        return -errno;
+        }
 
         r = copy_file_fd(from, fdt, true);
         if (r < 0) {
@@ -375,6 +386,29 @@ int copy_file(const char *from, const char *to, int flags, mode_t mode) {
         return 0;
 }
 
+int copy_file_atomic(const char *from, const char *to, mode_t mode, bool replace) {
+        _cleanup_free_ char *t;
+        int r;
+
+        assert(from);
+        assert(to);
+
+        r = tempfn_random(to, &t);
+        if (r < 0)
+                return r;
+
+        r = copy_file(from, t, O_NOFOLLOW|O_EXCL, mode);
+        if (r < 0)
+                return r;
+
+        if (renameat2(AT_FDCWD, t, AT_FDCWD, to, replace ? 0 : RENAME_NOREPLACE) < 0) {
+                unlink_noerrno(t);
+                return -errno;
+        }
+
+        return 0;
+}
+
 int copy_times(int fdf, int fdt) {
         struct timespec ut[2];
         struct stat st;
diff --git a/src/shared/copy.h b/src/shared/copy.h
index 6d725ef26d..58159a02cc 100644
--- a/src/shared/copy.h
+++ b/src/shared/copy.h
@@ -26,6 +26,7 @@
 
 int copy_file_fd(const char *from, int to, bool try_reflink);
 int copy_file(const char *from, const char *to, int flags, mode_t mode);
+int copy_file_atomic(const char *from, const char *to, mode_t mode, bool replace);
 int copy_tree(const char *from, const char *to, bool merge);
 int copy_tree_at(int fdf, const char *from, int fdt, const char *to, bool merge);
 int copy_directory_fd(int dirfd, const char *to, bool merge);
diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index fa06a0dc59..7c041fab76 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -20,11 +20,13 @@
 ***/
 
 #include <sys/statfs.h>
+#include <fcntl.h>
 
 #include "strv.h"
 #include "utf8.h"
 #include "btrfs-util.h"
 #include "path-util.h"
+#include "copy.h"
 #include "machine-image.h"
 
 static const char image_search_path[] =
@@ -317,18 +319,158 @@ void image_hashmap_free(Hashmap *map) {
 }
 
 int image_remove(Image *i) {
-        int r;
-
         assert(i);
 
         if (path_equal(i->path, "/") ||
             path_startswith(i->path, "/usr"))
                 return -EROFS;
 
-        if (i->type == IMAGE_SUBVOLUME)
+        switch (i->type) {
+
+        case IMAGE_SUBVOLUME:
                 return btrfs_subvol_remove(i->path);
-        else
+
+        case IMAGE_DIRECTORY:
+        case IMAGE_GPT:
                 return rm_rf_dangerous(i->path, false, true, false);
+
+        default:
+                return -ENOTSUP;
+        }
+}
+
+int image_rename(Image *i, const char *new_name) {
+        _cleanup_free_ char *new_path = NULL, *nn = NULL;
+        int r;
+
+        assert(i);
+
+        if (!image_name_is_valid(new_name))
+                return -EINVAL;
+
+        if (path_equal(i->path, "/") ||
+            path_startswith(i->path, "/usr"))
+                return -EROFS;
+
+        r = image_find(new_name, NULL);
+        if (r < 0)
+                return r;
+        if (r > 0)
+                return -EEXIST;
+
+        switch (i->type) {
+
+        case IMAGE_SUBVOLUME:
+        case IMAGE_DIRECTORY:
+                new_path = file_in_same_dir(i->path, new_name);
+                break;
+
+        case IMAGE_GPT: {
+                const char *fn;
+
+                fn = strappenda(new_name, ".gpt");
+                new_path = file_in_same_dir(i->path, fn);
+                break;
+        }
+
+        default:
+                return -ENOTSUP;
+        }
+
+        if (!new_path)
+                return -ENOMEM;
+
+        nn = strdup(new_name);
+        if (!nn)
+                return -ENOMEM;
+
+        if (renameat2(AT_FDCWD, i->path, AT_FDCWD, new_path, RENAME_NOREPLACE) < 0)
+                return -errno;
+
+        free(i->path);
+        i->path = new_path;
+        new_path = NULL;
+
+        free(i->name);
+        i->name = nn;
+        nn = NULL;
+
+        return 0;
+}
+
+int image_clone(Image *i, const char *new_name, bool read_only) {
+        const char *new_path;
+        int r;
+
+        assert(i);
+
+        if (!image_name_is_valid(new_name))
+                return -EINVAL;
+
+        r = image_find(new_name, NULL);
+        if (r < 0)
+                return r;
+        if (r > 0)
+                return -EEXIST;
+
+        switch (i->type) {
+
+        case IMAGE_SUBVOLUME:
+        case IMAGE_DIRECTORY:
+                new_path = strappenda("/var/lib/container/", new_name);
+
+                r = btrfs_subvol_snapshot(i->path, new_path, read_only, true);
+                break;
+
+        case IMAGE_GPT:
+                new_path = strappenda("/var/lib/container/", new_name, ".gpt");
+
+                r = copy_file_atomic(i->path, new_path, read_only ? 0444 : 0644, false);
+                break;
+
+        default:
+                return -ENOTSUP;
+        }
+
+        if (r < 0)
+                return r;
+
+        return 0;
+}
+
+int image_read_only(Image *i, bool b) {
+        int r;
+        assert(i);
+
+        if (path_equal(i->path, "/") ||
+            path_startswith(i->path, "/usr"))
+                return -EROFS;
+
+        switch (i->type) {
+
+        case IMAGE_SUBVOLUME:
+                r = btrfs_subvol_set_read_only(i->path, b);
+                if (r < 0)
+                        return r;
+                break;
+
+        case IMAGE_GPT: {
+                struct stat st;
+
+                if (stat(i->path, &st) < 0)
+                        return -errno;
+
+                if (chmod(i->path, (st.st_mode & 0444) | (b ? 0000 : 0200)) < 0)
+                        return -errno;
+                break;
+        }
+
+        case IMAGE_DIRECTORY:
+        default:
+                return -ENOTSUP;
+        }
+
+        return 0;
 }
 
 static const char* const image_type_table[_IMAGE_TYPE_MAX] = {
diff --git a/src/shared/machine-image.h b/src/shared/machine-image.h
index e17e32f4a5..9e0f6aedba 100644
--- a/src/shared/machine-image.h
+++ b/src/shared/machine-image.h
@@ -52,6 +52,9 @@ int image_find(const char *name, Image **ret);
 int image_discover(Hashmap *map);
 
 int image_remove(Image *i);
+int image_rename(Image *i, const char *new_name);
+int image_clone(Image *i, const char *new_name, bool read_only);
+int image_read_only(Image *i, bool b);
 
 const char* image_type_to_string(ImageType t) _const_;
 ImageType image_type_from_string(const char *s) _pure_;
diff --git a/src/shared/missing.h b/src/shared/missing.h
index dd7bef4d9d..08cf83647d 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -657,3 +657,13 @@ static inline int raw_clone(unsigned long flags, void *child_stack) {
 static inline pid_t raw_getpid(void) {
         return (pid_t) syscall(__NR_getpid);
 }
+
+#if !HAVE_DECL_RENAMEAT2
+static inline int renameat2(int oldfd, const char *oldname, int newfd, const char *newname, unsigned flags) {
+        return syscall(__NR_renameat2, oldfd, oldname, newfd, newname, flags);
+}
+#endif
+
+#ifndef RENAME_NOREPLACE
+#define RENAME_NOREPLACE (1 << 0)
+#endif
diff --git a/src/shared/util.c b/src/shared/util.c
index d04d73872e..92b4d498fe 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -1152,7 +1152,7 @@ char *delete_chars(char *s, const char *bad) {
 }
 
 char *file_in_same_dir(const char *path, const char *filename) {
-        char *e, *r;
+        char *e, *ret;
         size_t k;
 
         assert(path);
@@ -1165,17 +1165,17 @@ char *file_in_same_dir(const char *path, const char *filename) {
         if (path_is_absolute(filename))
                 return strdup(filename);
 
-        if (!(e = strrchr(path, '/')))
+        e = strrchr(path, '/');
+        if (!e)
                 return strdup(filename);
 
         k = strlen(filename);
-        if (!(r = new(char, e-path+1+k+1)))
+        ret = new(char, (e + 1 - path) + k + 1);
+        if (!ret)
                 return NULL;
 
-        memcpy(r, path, e-path+1);
-        memcpy(r+(e-path)+1, filename, k+1);
-
-        return r;
+        memcpy(mempcpy(ret, path, e + 1 - path), filename, k + 1);
+        return ret;
 }
 
 int rmdir_parents(const char *path, const char *stop) {