Merge pull request #25828 from poettering/no-more-basename

convert a number of basename() invocations to path_extract_filename()

8 files changed, 100 insertions, 60 deletions

diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c
index 0a4bd48b88..0ef3c608bb 100644
--- a/src/shared/btrfs-util.c
+++ b/src/shared/btrfs-util.c
@@ -51,20 +51,22 @@ static int validate_subvolume_name(const char *name) {
         return 0;
 }
 
-static int extract_subvolume_name(const char *path, const char **subvolume) {
-        const char *fn;
+static int extract_subvolume_name(const char *path, char **ret) {
+        _cleanup_free_ char *fn = NULL;
         int r;
 
         assert(path);
-        assert(subvolume);
+        assert(ret);
 
-        fn = basename(path);
+        r = path_extract_filename(path, &fn);
+        if (r < 0)
+                return r;
 
         r = validate_subvolume_name(fn);
         if (r < 0)
                 return r;
 
-        *subvolume = fn;
+        *ret = TAKE_PTR(fn);
         return 0;
 }
 
@@ -119,8 +121,8 @@ int btrfs_subvol_make_fd(int fd, const char *subvolume) {
 }
 
 int btrfs_subvol_make(const char *path) {
+        _cleanup_free_ char *subvolume = NULL;
         _cleanup_close_ int fd = -EBADF;
-        const char *subvolume;
         int r;
 
         assert(path);
@@ -1180,8 +1182,8 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
 }
 
 int btrfs_subvol_remove(const char *path, BtrfsRemoveFlags flags) {
+        _cleanup_free_ char *subvolume = NULL;
         _cleanup_close_ int fd = -EBADF;
-        const char *subvolume;
         int r;
 
         assert(path);
@@ -1578,8 +1580,8 @@ int btrfs_subvol_snapshot_fd_full(
                 copy_progress_bytes_t progress_bytes,
                 void *userdata) {
 
+        _cleanup_free_ char *subvolume = NULL;
         _cleanup_close_ int new_fd = -EBADF;
-        const char *subvolume;
         int r;
 
         assert(old_fd >= 0);
diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c
index cd922b84d4..f5283f9df4 100644
--- a/src/shared/exec-util.c
+++ b/src/shared/exec-util.c
@@ -18,6 +18,7 @@
 #include "hashmap.h"
 #include "macro.h"
 #include "missing_syscall.h"
+#include "path-util.h"
 #include "process-util.h"
 #include "rlimit-util.h"
 #include "serialize.h"
@@ -131,7 +132,13 @@ static int do_execute(
                         return log_oom();
 
                 if (callbacks) {
-                        fd = open_serialization_fd(basename(*path));
+                        _cleanup_free_ char *bn = NULL;
+
+                        r = path_extract_filename(*path, &bn);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to extract filename from path '%s': %m", *path);
+
+                        fd = open_serialization_fd(bn);
                         if (fd < 0)
                                 return log_error_errno(fd, "Failed to open serialization file: %m");
                 }
@@ -199,15 +206,16 @@ int execute_directories(
                 ExecDirFlags flags) {
 
         char **dirs = (char**) directories;
+        _cleanup_free_ char *name = NULL;
         _cleanup_close_ int fd = -EBADF;
-        char *name;
         int r;
         pid_t executor_pid;
 
         assert(!strv_isempty(dirs));
 
-        name = basename(dirs[0]);
-        assert(!isempty(name));
+        r = path_extract_filename(dirs[0], &name);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract file name from '%s': %m", dirs[0]);
 
         if (callbacks) {
                 assert(callback_args);
diff --git a/src/shared/format-table.c b/src/shared/format-table.c
index 1c1db2f43a..4795e1ad80 100644
--- a/src/shared/format-table.c
+++ b/src/shared/format-table.c
@@ -281,6 +281,7 @@ static size_t table_data_size(TableDataType type, const void *data) {
 
         case TABLE_STRING:
         case TABLE_PATH:
+        case TABLE_PATH_BASENAME:
         case TABLE_FIELD:
         case TABLE_HEADER:
                 return strlen(data) + 1;
@@ -513,7 +514,7 @@ int table_add_cell_stringf_full(Table *t, TableCell **ret_cell, TableDataType dt
         int r;
 
         assert(t);
-        assert(IN_SET(dt, TABLE_STRING, TABLE_PATH, TABLE_FIELD, TABLE_HEADER));
+        assert(IN_SET(dt, TABLE_STRING, TABLE_PATH, TABLE_PATH_BASENAME, TABLE_FIELD, TABLE_HEADER));
 
         va_start(ap, format);
         r = vasprintf(&buffer, format, ap);
@@ -873,6 +874,7 @@ int table_add_many_internal(Table *t, TableDataType first_type, ...) {
 
                 case TABLE_STRING:
                 case TABLE_PATH:
+                case TABLE_PATH_BASENAME:
                 case TABLE_FIELD:
                 case TABLE_HEADER:
                         data = va_arg(ap, const char *);
@@ -1281,6 +1283,7 @@ static int cell_data_compare(TableData *a, size_t index_a, TableData *b, size_t
                         return strcmp(a->string, b->string);
 
                 case TABLE_PATH:
+                case TABLE_PATH_BASENAME:
                         return path_compare(a->string, b->string);
 
                 case TABLE_STRV:
@@ -1453,15 +1456,24 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas
 
         case TABLE_STRING:
         case TABLE_PATH:
+        case TABLE_PATH_BASENAME:
         case TABLE_FIELD:
-        case TABLE_HEADER:
+        case TABLE_HEADER: {
+                _cleanup_free_ char *bn = NULL;
+                const char *s;
+
+                if (d->type == TABLE_PATH_BASENAME)
+                        s = path_extract_filename(d->string, &bn) < 0 ? d->string : bn;
+                else
+                        s = d->string;
+
                 if (d->uppercase && !avoid_uppercasing) {
-                        d->formatted = new(char, strlen(d->string) + (d->type == TABLE_FIELD) + 1);
+                        d->formatted = new(char, strlen(s) + (d->type == TABLE_FIELD) + 1);
                         if (!d->formatted)
                                 return NULL;
 
                         char *q = d->formatted;
-                        for (char *p = d->string; *p; p++)
+                        for (const char *p = s; *p; p++)
                                 *(q++) = (char) toupper((unsigned char) *p);
 
                         if (d->type == TABLE_FIELD)
@@ -1470,14 +1482,20 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas
                         *q = 0;
                         return d->formatted;
                 } else if (d->type == TABLE_FIELD) {
-                        d->formatted = strjoin(d->string, ":");
+                        d->formatted = strjoin(s, ":");
                         if (!d->formatted)
                                 return NULL;
 
                         return d->formatted;
                 }
 
+                if (bn) {
+                        d->formatted = TAKE_PTR(bn);
+                        return d->formatted;
+                }
+
                 return d->string;
+        }
 
         case TABLE_STRV:
                 if (strv_isempty(d->strv))
@@ -2544,6 +2562,7 @@ static int table_data_to_json(TableData *d, JsonVariant **ret) {
 
         case TABLE_STRING:
         case TABLE_PATH:
+        case TABLE_PATH_BASENAME:
         case TABLE_FIELD:
         case TABLE_HEADER:
                 return json_variant_new_string(ret, d->string);
diff --git a/src/shared/format-table.h b/src/shared/format-table.h
index e835692c6f..0e8aecffe6 100644
--- a/src/shared/format-table.h
+++ b/src/shared/format-table.h
@@ -17,6 +17,7 @@ typedef enum TableDataType {
         TABLE_STRV,
         TABLE_STRV_WRAPPED,
         TABLE_PATH,
+        TABLE_PATH_BASENAME,       /* like TABLE_PATH, but display only last path element (i.e. the "basename") in regular output */
         TABLE_BOOLEAN,
         TABLE_BOOLEAN_CHECKMARK,
         TABLE_TIMESTAMP,
diff --git a/src/shared/generator.c b/src/shared/generator.c
index da3eaadbf4..6d95aa72f8 100644
--- a/src/shared/generator.c
+++ b/src/shared/generator.c
@@ -60,13 +60,19 @@ int generator_open_unit_file(
 }
 
 int generator_add_symlink(const char *dir, const char *dst, const char *dep_type, const char *src) {
+        _cleanup_free_ char *bn = NULL;
+        const char *from, *to;
+        int r;
+
         /* Adds a symlink from <dst>.<dep_type>/ to <src> (if src is absolute)
          * or ../<src> (otherwise). */
 
-        const char *from, *to;
+        r = path_extract_filename(src, &bn);
+        if (r < 0)
+                return log_error_errno(r, "Failed to extract filename from '%s': %m", src);
 
         from = path_is_absolute(src) ? src : strjoina("../", src);
-        to = strjoina(dir, "/", dst, ".", dep_type, "/", basename(src));
+        to = strjoina(dir, "/", dst, ".", dep_type, "/", bn);
 
         (void) mkdir_parents_label(to, 0755);
         if (symlink(from, to) < 0)
diff --git a/src/shared/lockfile-util.c b/src/shared/lockfile-util.c
index b920f3fe9b..df099d7d05 100644
--- a/src/shared/lockfile-util.c
+++ b/src/shared/lockfile-util.c
@@ -20,15 +20,15 @@ int make_lock_file(const char *p, int operation, LockFile *ret) {
         _cleanup_free_ char *t = NULL;
         int r;
 
+        assert(p);
+        assert(ret);
+
         /*
-         * We use UNPOSIX locks if they are available. They have nice
-         * semantics, and are mostly compatible with NFS. However,
-         * they are only available on new kernels. When we detect we
-         * are running on an older kernel, then we fall back to good
-         * old BSD locks. They also have nice semantics, but are
-         * slightly problematic on NFS, where they are upgraded to
-         * POSIX locks, even though locally they are orthogonal to
-         * POSIX locks.
+         * We use UNPOSIX locks if they are available. They have nice semantics, and are mostly compatible
+         * with NFS. However, they are only available on new kernels. When we detect we are running on an
+         * older kernel, then we fall back to good old BSD locks. They also have nice semantics, but are
+         * slightly problematic on NFS, where they are upgraded to POSIX locks, even though locally they are
+         * orthogonal to POSIX locks.
          */
 
         t = strdup(p);
@@ -48,23 +48,18 @@ int make_lock_file(const char *p, int operation, LockFile *ret) {
 
                 r = fcntl(fd, (operation & LOCK_NB) ? F_OFD_SETLK : F_OFD_SETLKW, &fl);
                 if (r < 0) {
-
                         /* If the kernel is too old, use good old BSD locks */
-                        if (errno == EINVAL)
+                        if (errno == EINVAL || ERRNO_IS_NOT_SUPPORTED(errno))
                                 r = flock(fd, operation);
-
                         if (r < 0)
                                 return errno == EAGAIN ? -EBUSY : -errno;
                 }
 
-                /* If we acquired the lock, let's check if the file
-                 * still exists in the file system. If not, then the
-                 * previous exclusive owner removed it and then closed
-                 * it. In such a case our acquired lock is worthless,
-                 * hence try again. */
+                /* If we acquired the lock, let's check if the file still exists in the file system. If not,
+                 * then the previous exclusive owner removed it and then closed it. In such a case our
+                 * acquired lock is worthless, hence try again. */
 
-                r = fstat(fd, &st);
-                if (r < 0)
+                if (fstat(fd, &st) < 0)
                         return -errno;
                 if (st.st_nlink > 0)
                         break;
@@ -72,29 +67,33 @@ int make_lock_file(const char *p, int operation, LockFile *ret) {
                 fd = safe_close(fd);
         }
 
-        ret->path = t;
-        ret->fd = fd;
-        ret->operation = operation;
-
-        fd = -EBADF;
-        t = NULL;
+        *ret = (LockFile) {
+                .path = TAKE_PTR(t),
+                .fd = TAKE_FD(fd),
+                .operation = operation,
+        };
 
         return r;
 }
 
 int make_lock_file_for(const char *p, int operation, LockFile *ret) {
-        const char *fn;
-        char *t;
+        _cleanup_free_ char *fn = NULL, *dn = NULL, *t = NULL;
+        int r;
 
         assert(p);
         assert(ret);
 
-        fn = basename(p);
-        if (!filename_is_valid(fn))
-                return -EINVAL;
+        r = path_extract_filename(p, &fn);
+        if (r < 0)
+                return r;
+
+        r = path_extract_directory(p, &dn);
+        if (r < 0)
+                return r;
 
-        t = newa(char, strlen(p) + 2 + 4 + 1);
-        stpcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), ".#"), fn), ".lck");
+        t = strjoin(dn, "/.#", fn, ".lck");
+        if (!t)
+                return -ENOMEM;
 
         return make_lock_file(t, operation, ret);
 }
@@ -119,7 +118,7 @@ void release_lock_file(LockFile *f) {
                         };
 
                         r = fcntl(f->fd, F_OFD_SETLK, &fl);
-                        if (r < 0 && errno == EINVAL)
+                        if (r < 0 && (errno == EINVAL || ERRNO_IS_NOT_SUPPORTED(errno)))
                                 r = flock(f->fd, LOCK_EX|LOCK_NB);
 
                         if (r >= 0)
diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c
index 603eb7b780..8aad531a4d 100644
--- a/src/shared/mount-util.c
+++ b/src/shared/mount-util.c
@@ -952,7 +952,7 @@ static int mount_in_namespace(
         if (r < 0)
                 goto finish;
         if (r == 0) {
-                const char *mount_inside;
+                _cleanup_free_ char *mount_outside_fn = NULL, *mount_inside = NULL;
 
                 errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]);
 
@@ -965,8 +965,19 @@ static int mount_in_namespace(
                 }
 
                 /* Fifth, move the mount to the right place inside */
-                mount_inside = strjoina(incoming_path, basename(mount_outside));
-                r = mount_nofollow_verbose(LOG_ERR, mount_inside, dest, NULL, MS_MOVE, NULL);
+                r = path_extract_filename(mount_outside, &mount_outside_fn);
+                if (r < 0) {
+                        log_debug_errno(r, "Failed to extract filename from propagation file or directory '%s': %m", mount_outside);
+                        goto child_fail;
+                }
+
+                mount_inside = path_join(incoming_path, mount_outside_fn);
+                if (!mount_inside) {
+                        r = log_oom_debug();
+                        goto child_fail;
+                }
+
+                r = mount_nofollow_verbose(LOG_DEBUG, mount_inside, dest, NULL, MS_MOVE, NULL);
                 if (r < 0)
                         goto child_fail;
 
diff --git a/src/shared/tests.c b/src/shared/tests.c
index 54e80decd1..8884c8e90e 100644
--- a/src/shared/tests.c
+++ b/src/shared/tests.c
@@ -7,12 +7,6 @@
 #include <sys/mount.h>
 #include <sys/wait.h>
 
-/* When we include libgen.h because we need dirname() we immediately
- * undefine basename() since libgen.h defines it as a macro to the POSIX
- * version which is really broken. We prefer GNU basename(). */
-#include <libgen.h>
-#undef basename
-
 #include "sd-bus.h"
 
 #include "alloc-util.h"