Re: [RFC PATCH bpf-next v5 3/8] 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 message output due to GNU Make's function argument whitespace stripping.
--

--- Patch [3]: [RFC PATCH bpf-next v5 3/8] selftests/bpf: factor the permissive-mode skip suffix into a helper ---
commit 5a41b62fa6a5d8d794a18de41a7ff9bb6b7087f6
Author: Mykola Lysenko <[email protected]>

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

This commit factors out the duplicated skip logic used for permissive builds
into a common skip_on_fail macro in the Makefile.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -49,6 +49,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; })

[Severity: Low]
Will GNU Make strip the leading space before $(3) in this if statement?

Since Make strips leading whitespace after commas in function arguments, it
appears the optional note gets appended directly to the target name without
any separation space.

[ ... ]

> @@ -1012,8 +1002,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]
Because of the stripped whitespace mentioned above, will the console output
for this rule now print as "bench(some benchmarks may have been skipped)"
instead of "bench (some benchmarks may have been skipped)"?

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