Re: [RFC PATCH bpf-next v5 3/8] selftests/bpf: factor the permissive-mode skip suffix into a helper
Mykola Lysenko <[email protected]> Tue, 4 Aug 2026 17:13:37 -0700
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <CAMtxOX3f-u0cnDiXuHyxUuiMWwosC9sj3xmTDy-ATaFAKuX4RA@mail.gmail.com> |
On Tue, Aug 4, 2026 at 10:21=E2=80=AFAM <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issu= e(s) to consider: > - [Low] Missing space in SKIP message output due to GNU Make's function a= rgument whitespace stripping. > -- > > --- Patch [3]: [RFC PATCH bpf-next v5 3/8] selftests/bpf: factor the perm= issive-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 bui= lds > into a common skip_on_fail macro in the Makefile. > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selft= ests/bpf/Makefile > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -49,6 +49,14 @@ SKIP_CRYPTO ?=3D > > BPF_STRICT_BUILD ?=3D 1 > > PERMISSIVE :=3D $(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 =3D $(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 witho= ut > 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 outp= ut > 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.htm= l> 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 :=3D 1 skip_on_fail =3D $(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 =C2=B7 https://sashiko.dev/#/patchset/20260804170156.17= [email protected]?part=3D3