[ndctl PATCH v2] daxctl: replace basename() usage with new path_basename()

[ndctl PATCH v2] daxctl: replace basename() usage with new path_basename()

[Alison Schofield]

A user reports that ndctl fails to compile on MUSL systems:

daxctl/device.c: In function 'parse_device_options':
daxctl/device.c:377:26: error: implicit declaration of function 'basename' [-Wimplicit-function-declaration]
  377 |                 device = basename(argv[0]);
      |                          ^~~~~~~~

There are two versions of basename() with different behaviors:
	GNU basename() from <string.h>: doesn't modify its argument
	POSIX basename() from <libgen.h>: may modify its argument

glibc provides both versions, while MUSL libc only provides the POSIX
version. Previous code relied on the GNU extension without a header
or used the POSIX version inconsistently.

Introduce a new helper path_basename() that returns the portion of a
path after the last '/', the full string if no '/' is present, and a
trailing '/' returns an empty string. This avoids libc-specific
basename() behavior and is safe for argv style and arbitrary paths.

Closes: https://github.com/pmem/ndctl/issues/283
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

Changes in v2: 
- Replace open coded strrchr() logic with new helper (Marc, Dan)
- Comment that new helper (Marc)
- Update commit msg


 daxctl/device.c        |  4 ++--
 daxctl/lib/libdaxctl.c |  7 ++++---
 util/util.h            | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/daxctl/device.c b/daxctl/device.c
index e3993b17c260..a4e36b130a09 100644
--- a/daxctl/device.c
+++ b/daxctl/device.c
@@ -362,11 +362,11 @@ static const char *parse_device_options(int argc, const char **argv,
 	};
 	unsigned long long units = 1;
 	int i, rc = 0;
-	char *device = NULL;
+	const char *device = NULL;
 
 	argc = parse_options(argc, argv, options, u, 0);
 	if (argc > 0)
-		device = basename(argv[0]);
+		device = path_basename(argv[0]);
 
 	/* Handle action-agnostic non-option arguments */
 	if (argc == 0 &&
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index b7fa0de0b73d..02ae7e50b123 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -3,7 +3,6 @@
 #include <stdio.h>
 #include <errno.h>
 #include <limits.h>
-#include <libgen.h>
 #include <stdlib.h>
 #include <dirent.h>
 #include <unistd.h>
@@ -15,6 +14,7 @@
 #include <ccan/array_size/array_size.h>
 
 #include <util/log.h>
+#include <util/util.h>
 #include <util/sysfs.h>
 #include <util/iomem.h>
 #include <daxctl/libdaxctl.h>
@@ -389,7 +389,8 @@ DAXCTL_EXPORT int daxctl_dev_is_system_ram_capable(struct daxctl_dev *dev)
 {
 	const char *devname = daxctl_dev_get_devname(dev);
 	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
-	char *mod_path, *mod_base;
+	const char *mod_base;
+	char *mod_path;
 	char path[200];
 	const int len = sizeof(path);
 
@@ -408,7 +409,7 @@ DAXCTL_EXPORT int daxctl_dev_is_system_ram_capable(struct daxctl_dev *dev)
 	if (!mod_path)
 		return false;
 
-	mod_base = basename(mod_path);
+	mod_base = path_basename(mod_path);
 	if (strcmp(mod_base, dax_modules[DAXCTL_DEV_MODE_RAM]) == 0) {
 		free(mod_path);
 		return true;
diff --git a/util/util.h b/util/util.h
index 58db06530c37..b7913b499d82 100644
--- a/util/util.h
+++ b/util/util.h
@@ -93,6 +93,21 @@ static inline int is_absolute_path(const char *path)
 	return path[0] == '/';
 }
 
+/*
+ * path_basename() is a basename() style helper for paths that may not
+ * contain a '/'. Unlike devpath_to_devname() in util/sysfs.h, which
+ * assumes a valid sysfs-style path and requires at least one '/', this
+ * helper can return the full string when no path separator is present.
+ * It avoids libc-specific basename() behavior and is safe for argv style
+ * inputs.
+ */
+static inline const char *path_basename(const char *path)
+{
+	const char *p = strrchr(path, '/');
+
+	return p ? p + 1 : path;
+}
+
 void usage(const char *err) NORETURN;
 void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2)));
 int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
-- 
2.37.3

Re: [ndctl PATCH v2] daxctl: replace basename() usage with new path_basename()

[Marc Herbert]

Hearing the punishment for inputs with a trailing slash is not too
harsh:

Reviewed-by: Marc Herbert <marc.herbert@linux.intel.com>


Alison Schofield <alison.schofield@intel.com> writes:

> A user reports that ndctl fails to compile on MUSL systems:
>
> daxctl/device.c: In function 'parse_device_options':
> daxctl/device.c:377:26: error: implicit declaration of function 'basename' [-Wimplicit-function-declaration]
>   377 |                 device = basename(argv[0]);
>       |                          ^~~~~~~~
>
> There are two versions of basename() with different behaviors:
> 	GNU basename() from <string.h>: doesn't modify its argument
> 	POSIX basename() from <libgen.h>: may modify its argument
>
> glibc provides both versions, while MUSL libc only provides the POSIX
> version. Previous code relied on the GNU extension without a header
> or used the POSIX version inconsistently.
>
> Introduce a new helper path_basename() that returns the portion of a
> path after the last '/', the full string if no '/' is present, and a
> trailing '/' returns an empty string. This avoids libc-specific
> basename() behavior and is safe for argv style and arbitrary paths.
>
> Closes: https://github.com/pmem/ndctl/issues/283
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>
> Changes in v2: 
> - Replace open coded strrchr() logic with new helper (Marc, Dan)
> - Comment that new helper (Marc)
> - Update commit msg
>
>
>  daxctl/device.c        |  4 ++--
>  daxctl/lib/libdaxctl.c |  7 ++++---
>  util/util.h            | 15 +++++++++++++++
>  3 files changed, 21 insertions(+), 5 deletions(-)
>