Re: [PATCH bpf-next v7 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing space in "SKIP-LINK" message for skipped benchmarks.
--

--- Patch [3]: [PATCH bpf-next v7 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper ---
commit 390f83151721ccaeb21976f48c2d352d76658f37
Author: Mykola Lysenko <[email protected]>

selftests/bpf: factor the permissive-mode skip suffix into a helper

This commit factors out the duplicated permissive-mode skip suffix across
multiple Makefile recipes into a single skip_on_fail helper macro. This
reduces duplication while maintaining the original behavior and messages.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index c7c7f27dd8605..e4c8efd1e6f1b 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -48,6 +48,14 @@ SKIP_CRYPTO	?=
>  BPF_STRICT_BUILD ?= 1
>  PERMISSIVE := $(filter 0,$(BPF_STRICT_BUILD))
>  
> +# Permissive-mode recipe suffix: on failure, remove the target, emit a
> +# SKIP marker and report success so the rest of the build continues.
> +# $(1) - SKIP tag (BPF, TEST, BENCH, SKEL, LINK, ...)
> +# $(2) - optional extra files to remove alongside the target
> +# $(3) - optional note appended to the SKIP message
> +skip_on_fail = $(if $(PERMISSIVE),|| { $(RM) $@ $(2); \
> +	printf '  %-12s %s\n' 'SKIP-$(1)' '$(notdir $@)$(if $(3), $(3))' 1>&2; })

[ ... ]

> @@ -1016,8 +1006,7 @@ $(OUTPUT)/bench: $(OUTPUT)/bench.o \
>  		 $(OUTPUT)/usdt_2.o \
>  		 #
>  	$(call msg,BINARY,,$@)
> -	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.a %.o,$^) $(LDLIBS) -o $@ $(if $(PERMISSIVE),|| \
> -		($(RM) $@; printf '  %-12s %s\n' 'SKIP-LINK' '$(notdir $@) (some benchmarks may have been skipped)' 1>&2))
> +	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.a %.o,$^) $(LDLIBS) -o $@ $(call skip_on_fail,LINK,,(some benchmarks may have been skipped))

[Severity: Low]
This isn't a bug, but does the resulting string lack a space between the
target name and the note?

GNU Make ignores whitespace following a comma in function arguments. Because
of this, $(if $(3), $(3)) in the skip_on_fail macro will evaluate without
a leading space, even if one is provided in the invocation or in the string
itself.

Could this cause the printed message to become "bench(some benchmarks may
have been skipped)" instead of the original "bench (some benchmarks may have
been skipped)"?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3