All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Mykola Lysenko" <nickolay.lysenko@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v7 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper
Date: Sun, 23 Aug 2026 19:51:28 +0000	[thread overview]
Message-ID: <20260823195129.2FF801F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260823194241.3415152-4-nickolay.lysenko@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing space in "SKIP-LINK" message for skipped benchmarks.
--

--- Patch [3]: [PATCH bpf-next v7 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper ---
commit 390f83151721ccaeb21976f48c2d352d76658f37
Author: Mykola Lysenko <nickolay.lysenko@gmail.com>

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

This commit factors out the duplicated permissive-mode skip suffix across
multiple Makefile recipes into a single skip_on_fail helper macro. This
reduces duplication while maintaining the original behavior and messages.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index c7c7f27dd8605..e4c8efd1e6f1b 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; })

[ ... ]

> @@ -1016,8 +1006,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]
This isn't a bug, but does the resulting string lack a space between the
target name and the note?

GNU Make ignores whitespace following a comma in function arguments. Because
of this, $(if $(3), $(3)) in the skip_on_fail macro will evaluate without
a leading space, even if one is provided in the invocation or in the string
itself.

Could this cause the printed message to become "bench(some benchmarks may
have been skipped)" instead of the original "bench (some benchmarks may have
been skipped)"?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823194241.3415152-1-nickolay.lysenko@gmail.com?part=3

  reply	other threads:[~2026-08-23 19:51 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-23 19:42 [PATCH bpf-next v7 0/9] selftests/bpf: restructure the Makefile as a layered build Mykola Lysenko
2026-08-23 19:42 ` [PATCH bpf-next v7 1/9] selftests/bpf: keep headers off the generic link command line Mykola Lysenko
2026-08-23 19:42 ` [PATCH bpf-next v7 2/9] selftests/bpf: drop stale lines, restore two header dependencies Mykola Lysenko
2026-08-23 20:41   ` bot+bpf-ci
2026-08-24 22:47   ` Eduard Zingerman
2026-08-23 19:42 ` [PATCH bpf-next v7 3/9] selftests/bpf: factor the permissive-mode skip suffix into a helper Mykola Lysenko
2026-08-23 19:51   ` sashiko-bot [this message]
2026-08-23 19:42 ` [PATCH bpf-next v7 4/9] selftests/bpf: generate the signing key and certificate once Mykola Lysenko
2026-08-23 19:42 ` [PATCH bpf-next v7 5/9] selftests/bpf: generate verifier/tests.h in a regular recipe Mykola Lysenko
2026-08-23 19:42 ` [PATCH bpf-next v7 6/9] selftests/bpf: derive the bench object list from the sources Mykola Lysenko
2026-08-24 23:42   ` Eduard Zingerman
2026-08-23 19:42 ` [PATCH bpf-next v7 7/9] selftests/bpf: extract BPF skeleton generation into a helper script Mykola Lysenko
2026-08-23 19:42 ` [PATCH bpf-next v7 8/9] selftests/bpf: move shared build definitions into Makefile.buildvars Mykola Lysenko
2026-08-23 20:53   ` bot+bpf-ci
2026-08-25  6:45   ` Eduard Zingerman
2026-08-23 19:42 ` [PATCH bpf-next v7 9/9] selftests/bpf: build each test runner instance in its own sub-make Mykola Lysenko
2026-08-23 21:05   ` bot+bpf-ci
2026-08-25 23:55   ` Eduard Zingerman
2026-08-26  4:58     ` Eduard Zingerman

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=20260823195129.2FF801F000E9@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.