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

Same finding as on v5 3/8, refuted there with a minimal Makefile:
https://lore.kernel.org/bpf/CAMtxOX3f-u0cnDiXuHyxUuiMWwosC9sj3xmTDy-ATaFAKuX4RA@mail.gmail.com/

On Sat, Aug 22, 2026 at 6:18 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing space in the SKIP message for the LINK step causes merged text output.
> --
>
> --- Patch [3]: [PATCH bpf-next v6 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper ---
> commit 0c0b26d03c8f590d99a07e3db037b9c093922751
> Author: Mykola Lysenko <[email protected]>
>
> selftests/bpf: factor the permissive-mode skip suffix into a helper
>
> With BPF_STRICT_BUILD=0, eleven recipes append the same tail. This patch
> factors the tail into skip_on_fail, parameterized by the SKIP tag, optional
> extra files to remove, and an optional note. Every call site is intended to
> keep its exact message and behavior.
>
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 8d71345c2e86c..04eea6ebc75cd 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; })
>
> [Severity: Low]
> Does this formatting behave exactly as intended when the note is appended?
>
> Because GNU Make strips the leading space after the comma in the then-part
> of the $(if) function, it appears that $(if $(3), $(3)) will strip the
> leading space.
>
> Would this result in appending the note directly to the target name without
> any whitespace separation?
>
> [ ... ]
>
> > @@ -1018,8 +1008,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]
> When this build fails in permissive mode on the bench target, the
> skip_on_fail macro is called with the note as the third argument.
>
> With the space-stripping behavior above, won't this result in the SKIP
> message being formatted incorrectly as:
>
> bench(some benchmarks may have been skipped)
>
> instead of the previous behavior:
>
> bench (some benchmarks may have been skipped)
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3