From 884a609a77a6ddb7f0e0ba8789e0f0fc0e899dab Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 18 Apr 2024 18:57:08 +0200 Subject: [PATCH] btrfs-progs: add basename wrappers for unified semantics What basename(3) does with the argument depends on _GNU_SOURCE and inclusion of libgen.h. This is problematic on Musl (1.2.5) as reported. We want the GNU semantics that does not modify the argument. Common way to make it portable is to add own helper. This is now implemented in path_basename() that does not use the libc provided basename but preserves the semantics. The path_dirname() is just for parity, otherwise same as dirname(). Sources: - https://bugs.gentoo.org/926288 - https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7 Issue: #778 Signed-off-by: David Sterba --- cmds/subvolume.c | 26 +++++++++++++------------- common/device-utils.c | 4 ++-- common/path-utils.c | 28 ++++++++++++++++++++++++++++ common/path-utils.h | 2 ++ 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/cmds/subvolume.c b/cmds/subvolume.c index 5d53efe6..869d7077 100644 --- a/cmds/subvolume.c +++ b/cmds/subvolume.c @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include @@ -149,7 +148,7 @@ static int create_one_subvolume(const char *dst, struct btrfs_qgroup_inherit *in int fddst = -1; char *dupname = NULL; char *dupdir = NULL; - char *newname; + const char *newname; char *dstdir; ret = path_is_dir(dst); @@ -170,7 +169,7 @@ static int create_one_subvolume(const char *dst, struct btrfs_qgroup_inherit *in ret = -ENOMEM; goto out; } - newname = basename(dupname); + newname = path_basename(dupname); dupdir = strdup(dst); if (!dupdir) { @@ -178,7 +177,7 @@ static int create_one_subvolume(const char *dst, struct btrfs_qgroup_inherit *in ret = -ENOMEM; goto out; } - dstdir = dirname(dupdir); + dstdir = path_dirname(dupdir); if (!test_issubvolname(newname)) { error("invalid subvolume name: %s", newname); @@ -364,7 +363,8 @@ static int cmd_subvolume_delete(const struct cmd_struct *cmd, int argc, char **a int res, ret = 0; int cnt; int fd = -1; - char *dname, *vname, *cpath; + char *dname, *cpath; + const char *vname; char *dupdname = NULL; char *dupvname = NULL; char *path = NULL; @@ -482,9 +482,9 @@ again: goto out; } dupdname = strdup(cpath); - dname = dirname(dupdname); + dname = path_dirname(dupdname); dupvname = strdup(cpath); - vname = basename(dupvname); + vname = path_basename(dupvname); free(cpath); /* When subvolid is passed, will point to the mount point */ @@ -670,7 +670,7 @@ static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char * bool readonly = false; char *dupname = NULL; char *dupdir = NULL; - char *newname; + const char *newname; char *dstdir; enum btrfs_util_error err; struct btrfs_ioctl_vol_args_v2 args; @@ -727,13 +727,13 @@ static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char * if (res > 0) { dupname = strdup(subvol); - newname = basename(dupname); + newname = path_basename(dupname); dstdir = dst; } else { dupname = strdup(dst); - newname = basename(dupname); + newname = path_basename(dupname); dupdir = strdup(dst); - dstdir = dirname(dupdir); + dstdir = path_dirname(dupdir); } if (!test_issubvolname(newname)) { @@ -1557,7 +1557,7 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg struct btrfs_util_subvolume_iterator *iter; struct btrfs_util_subvolume_info subvol; char *subvol_path = NULL; - char *subvol_name = NULL; + const char *subvol_name = NULL; enum btrfs_util_error err; struct btrfs_qgroup_stats stats; unsigned int unit_mode; @@ -1669,7 +1669,7 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg subvol_path = strdup("/"); subvol_name = ""; } else { - subvol_name = basename(subvol_path); + subvol_name = path_basename(subvol_path); } if (bconf.output_format == CMD_FORMAT_JSON) { diff --git a/common/device-utils.c b/common/device-utils.c index 36108ec4..d086e9ea 100644 --- a/common/device-utils.c +++ b/common/device-utils.c @@ -343,14 +343,14 @@ static u64 device_get_partition_size_sysfs(const char *dev) char path[PATH_MAX] = {}; char sysfs[PATH_MAX] = {}; char sizebuf[128] = {}; - char *name = NULL; + const char *name = NULL; int sysfd; unsigned long long size = 0; name = realpath(dev, path); if (!name) return 0; - name = basename(path); + name = path_basename(path); ret = path_cat3_out(sysfs, "/sys/class/block", name, "size"); if (ret < 0) diff --git a/common/path-utils.c b/common/path-utils.c index 181737c4..929e5c8f 100644 --- a/common/path-utils.c +++ b/common/path-utils.c @@ -28,6 +28,11 @@ #include #include #include +/* + * For dirname() and basename(), but never use basename directly, there's + * path_basename() with unified GNU behaviour regardless of the includes and + * conditional defines. See basename(3) for more. + */ #include #include #include "common/path-utils.h" @@ -482,3 +487,26 @@ int test_issubvolname(const char *name) strcmp(name, ".") && strcmp(name, ".."); } +/* + * Unified GNU semantics basename helper, never changing the argument. Always + * use this instead of basename(). + */ +const char *path_basename(const char *path) +{ + const char *tmp = strrchr(path, '/'); + + /* Special case when the whole path is just "/". */ + if (path[0] == '/' && path[1] == 0) + return path; + + return tmp ? tmp + 1 : path; +} + +/* + * Return dirname component of path, may change the argument. + * Own helper for parity with path_basename(). + */ +char *path_dirname(char *path) +{ + return dirname(path); +} diff --git a/common/path-utils.h b/common/path-utils.h index 08ae0ff1..697fa6b4 100644 --- a/common/path-utils.h +++ b/common/path-utils.h @@ -39,6 +39,8 @@ int path_is_dir(const char *path); int is_same_loop_file(const char *a, const char *b); int path_is_reg_or_block_device(const char *filename); int path_is_in_dir(const char *parent, const char *path); +const char *path_basename(const char *path); +char *path_dirname(char *path); int test_issubvolname(const char *name);