From: sashiko-bot@kernel.org
To: "Mykola Lysenko" <nickolay.lysenko@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v8 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper
Date: Mon, 21 Sep 2026 08:09:51 +0000 [thread overview]
Message-ID: <20260921080951.EB4FF1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260921075855.2065871-4-nickolay.lysenko@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] build: missing space in skip_on_fail formatting
--
commit a3e18474fd48a50d84abcafc18805d61a8736ce1
Author: Mykola Lysenko <nickolay.lysenko@gmail.com>
selftests/bpf: factor the permissive-mode skip suffix into a helper
With BPF_STRICT_BUILD=0, eleven recipes append the same "|| { remove the
target, print a SKIP marker, report success }" tail, each spelled out
inline. Factor the tail into skip_on_fail; every call site keeps its
exact message and behavior.
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 3606ee8ab7949..11bd8997aea5d 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -46,6 +46,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 result in a missing space between the target name and the
note?
GNU Make strips unescaped leading whitespace following commas in function
arguments, so the space in `$(if $(3), $(3))` is discarded during expansion.
[ ... ]
> @@ -1014,8 +1004,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 the space is stripped, the invocation above doesn't provide a leading
space either. Will this log output as `bench(some benchmarks may have been
skipped)` instead of `bench (some benchmarks may have been skipped)`?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260921075855.2065871-1-nickolay.lysenko@gmail.com?part=3
next prev parent reply other threads:[~2026-09-21 8:09 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-21 7:58 [PATCH bpf-next v8 0/9] selftests/bpf: restructure the Makefile as a layered build Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 1/9] selftests/bpf: keep headers off the generic link command line Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 2/9] selftests/bpf: drop stale lines, restore two header dependencies Mykola Lysenko
2026-09-21 9:06 ` bot+bpf-ci
2026-09-21 9:21 ` Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper Mykola Lysenko
2026-09-21 8:09 ` sashiko-bot [this message]
2026-09-21 7:58 ` [PATCH bpf-next v8 4/9] selftests/bpf: generate the signing key and certificate once Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 5/9] selftests/bpf: generate verifier/tests.h in a regular recipe Mykola Lysenko
2026-09-21 9:06 ` bot+bpf-ci
2026-09-21 12:36 ` Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 6/9] selftests/bpf: derive the bench object list from the sources Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 7/9] selftests/bpf: extract BPF skeleton generation into a helper script Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 8/9] selftests/bpf: move shared build definitions into Makefile.buildvars Mykola Lysenko
2026-09-21 7:58 ` [PATCH bpf-next v8 9/9] selftests/bpf: build each test runner instance in its own sub-make Mykola Lysenko
2026-09-21 9:22 ` bot+bpf-ci
2026-09-21 12:08 ` Mykola Lysenko
2026-09-21 9:28 ` [PATCH bpf-next v8 0/9] selftests/bpf: restructure the Makefile as a layered build Kumar Kartikeya Dwivedi
2026-09-21 9:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260921080951.EB4FF1F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=nickolay.lysenko@gmail.com \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox