[1/1] bpftool: Be more portable by using POSIX's basename()

Commit Message

Arnaldo Carvalho de Melo Jan. 29, 2024, 2:33 p.m. UTC

musl libc had the basename() prototype in string.h, but this is a
glibc-ism, now they removed the _GNU_SOURCE bits in their devel distro,
Alpine Linux edge:

  https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7

So lets use the POSIX version, the whole rationale is spelled out at:

  https://gitlab.alpinelinux.org/alpine/aports/-/issues/15643

Acked-by: Jiri Olsa <[e-mail]>
Acked-by: Quentin Monnet <[e-mail]>
Cc: Adrian Hunter <[e-mail]>
Cc: Andrii Nakryiko <[e-mail]>
Cc: Ian Rogers <[e-mail]>
Cc: Namhyung Kim <[e-mail]>
Link: https://lore.kernel.org/lkml/[e-mail]
Signed-off-by: Arnaldo Carvalho de Melo <[e-mail]>
---
 tools/bpf/bpftool/gen.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Daniel Borkmann Jan. 29, 2024, 3:17 p.m. UTC | #1

On 1/29/24 3:33 PM, Arnaldo Carvalho de Melo wrote:
> musl libc had the basename() prototype in string.h, but this is a
> glibc-ism, now they removed the _GNU_SOURCE bits in their devel distro,
> Alpine Linux edge:
> 
>    https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7
> 
> So lets use the POSIX version, the whole rationale is spelled out at:
> 
>    https://gitlab.alpinelinux.org/alpine/aports/-/issues/15643
> 
> Acked-by: Jiri Olsa <[e-mail]>
> Acked-by: Quentin Monnet <[e-mail]>
> Cc: Adrian Hunter <[e-mail]>
> Cc: Andrii Nakryiko <[e-mail]>
> Cc: Ian Rogers <[e-mail]>
> Cc: Namhyung Kim <[e-mail]>
> Link: https://lore.kernel.org/lkml/[e-mail]
> Signed-off-by: Arnaldo Carvalho de Melo <[e-mail]>
> ---
>   tools/bpf/bpftool/gen.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index ee3ce2b8000d75d2..a5cc5938c3d7951e 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -7,6 +7,7 @@
>   #include <ctype.h>
>   #include <errno.h>
>   #include <fcntl.h>
> +#include <libgen.h>
>   #include <linux/err.h>
>   #include <stdbool.h>
>   #include <stdio.h>
> @@ -56,9 +57,10 @@ static bool str_has_suffix(const char *str, const char *suffix)
>   
>   static void get_obj_name(char *name, const char *file)
>   {
> -	/* Using basename() GNU version which doesn't modify arg. */
> -	strncpy(name, basename(file), MAX_OBJ_NAME_LEN - 1);
> -	name[MAX_OBJ_NAME_LEN - 1] = '\0';
> +	char file_copy[PATH_MAX];

Added a newline in here while applying, otherwise lgtm, thanks!

> +	/* Using basename() POSIX version to be more portable. */
> +	strncpy(file_copy, file, PATH_MAX - 1)[PATH_MAX - 1] = '\0';
> +	strncpy(name, basename(file_copy), MAX_OBJ_NAME_LEN - 1)[MAX_OBJ_NAME_LEN - 1] = '\0';
>   	if (str_has_suffix(name, ".o"))
>   		name[strlen(name) - 2] = '\0';
>   	sanitize_identifier(name);
>

Patch

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index ee3ce2b8000d75d2..a5cc5938c3d7951e 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -7,6 +7,7 @@ 
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <libgen.h>
 #include <linux/err.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -56,9 +57,10 @@  static bool str_has_suffix(const char *str, const char *suffix)
 
 static void get_obj_name(char *name, const char *file)
 {
-	/* Using basename() GNU version which doesn't modify arg. */
-	strncpy(name, basename(file), MAX_OBJ_NAME_LEN - 1);
-	name[MAX_OBJ_NAME_LEN - 1] = '\0';
+	char file_copy[PATH_MAX];
+	/* Using basename() POSIX version to be more portable. */
+	strncpy(file_copy, file, PATH_MAX - 1)[PATH_MAX - 1] = '\0';
+	strncpy(name, basename(file_copy), MAX_OBJ_NAME_LEN - 1)[MAX_OBJ_NAME_LEN - 1] = '\0';
 	if (str_has_suffix(name, ".o"))
 		name[strlen(name) - 2] = '\0';
 	sanitize_identifier(name);