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

On Tue, Aug 4, 2026 at 10:21 AM <[email protected]> wrote:
>
> 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)"?

does not seems to be the case

prozak@powertower:/tmp/tmp.U6wCzgIl5v$ make --version
GNU Make 4.3
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

prozak@powertower:/tmp/tmp.U6wCzgIl5v$ cat Makefile
PERMISSIVE := 1

skip_on_fail = $(if $(PERMISSIVE),||  \
{ printf "  %-12s %s\n" "SKIP-$(1)" "$(notdir $@)$(if $(3), $(3))" 1>&2; })

bench:
@false \
$(call skip_on_fail,LINK,,(some benchmarks may have been skipped))

prozak@powertower:/tmp/tmp.U6wCzgIl5v$ make bench
  SKIP-LINK    bench (some benchmarks may have been skipped)

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