* Re: [PATCH] bpf: add printf attributes
2026-09-15 20:21 [PATCH] bpf: add printf attributes Arnd Bergmann
@ 2026-09-15 21:11 ` bot+bpf-ci
0 siblings, 0 replies; 2+ messages in thread
From: bot+bpf-ci @ 2026-09-15 21:11 UTC (permalink / raw)
To: arnd, ast, daniel, andrii, eddyz87, memxor
Cc: arnd, martin.lau, song, yonghong.song, jolsa, emil,
ihor.solodrai, ameryhung, kees+treewide, bpf, linux-kernel, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 6411 bytes --]
> commit 5d37f3f1dd68e8bbee421b3d0871994907f9e35d
> Author: Arnd Bergmann <arnd@arndb.de>
> Date: Tue Sep 15 22:21:55 2026 +0200
>
> bpf: add printf attributes
>
> Building with extra warnings enabled shows some missing printf attributes:
>
> kernel/bpf/diagnostics.c: In function 'bpf_diag_source':
> kernel/bpf/diagnostics.c:828:9: error: function 'bpf_diag_source' might be a candidate for 'gnu_printf' format attribute [-Werror=suggest-attribute=format]
> 828 | msg = bpf_diag_vfmt(env, fmt, args);
>
> Add these to shut up the warning and allow the compiler to check the
> format strings.
>
> Fixes: a8f427835394 ("bpf: Report Program Structure CFG errors")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Does the Fixes tag reference the commit that introduced the warning?
Looking at the compiler warning, bpf_diag_source() triggers the warning
because it calls va_start(args, fmt) and passes those to bpf_diag_vfmt(),
which already carries __printf(2, 0) in kernel/bpf/diagnostics.h:50.
Both halves of that trigger existed before commit a8f427835394. The
parent commit a8f427835394~1 already had:
kernel/bpf/diagnostics.c:795
static void bpf_diag_source(struct bpf_verifier_env *env, u32 insn_idx, const char *label,
const char *fmt, ...)
with va_start(args, fmt) and msg = bpf_diag_vfmt(env, fmt, args) in the
body, and kernel/bpf/diagnostics.h:50 already had __printf(2, 0) on
bpf_diag_vfmt.
Commit a8f427835394 changed only kernel/bpf/diagnostics.c by 19
insertions, adding bpf_diag_program_structure() without touching
bpf_diag_source() or any format attribute, so the warning already
existed before that commit.
Should the Fixes tag instead name the commit that introduced
bpf_diag_source() together with the __printf(2, 0) annotation on
bpf_diag_vfmt?
> diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
> index a2cac59c6639..b069fe73d3dc 100644
> --- a/kernel/bpf/diagnostics.c
> +++ b/kernel/bpf/diagnostics.c
> @@ -291,7 +291,7 @@ char *bpf_diag_fmt_buf(struct bpf_verifier_env *env, size_t size)
> return buf;
> }
>
> -const char *bpf_diag_vfmt(struct bpf_verifier_env *env, const char *fmt, va_list args)
> +__printf(2, 0) const char *bpf_diag_vfmt(struct bpf_verifier_env *env, const char *fmt, va_list args)
> {
> va_list copy;
> char *buf;
Is this attribute needed here? bpf_diag_vfmt() already carries
__printf(2, 0) on its prototype in kernel/bpf/diagnostics.h:49-50:
const char *bpf_diag_vfmt(struct bpf_verifier_env *env, const char *fmt, va_list args)
__printf(2, 0);
kernel/bpf/diagnostics.c includes that header at line 17, so the
compiler was already applying the format attribute before this patch.
The attribute was present in the parent of the Fixes-referenced commit
(a8f427835394~1:kernel/bpf/diagnostics.h:50), and the compiler output
in the changelog reports exactly one warning, for bpf_diag_source()
only.
This hunk silences no warning and enables no new format checking, but it
pushes the line from 86 to 101 columns, exceeding the kernel's
100-column limit, so checkpatch will now emit "WARNING: line length of
101 exceeds 100 columns" on a line that was previously clean.
> @@ -309,7 +309,7 @@ const char *bpf_diag_vfmt(struct bpf_verifier_env *env, const char *fmt, va_list
> return buf ?: "";
> }
>
> -const char *bpf_diag_fmt(struct bpf_verifier_env *env, const char *fmt, ...)
> +__printf(2, 3) const char *bpf_diag_fmt(struct bpf_verifier_env *env, const char *fmt, ...)
> {
> const char *buf;
> va_list args;
Same redundancy here. bpf_diag_fmt() already carries __printf(2, 3) on
its prototype in kernel/bpf/diagnostics.h:51:
const char *bpf_diag_fmt(struct bpf_verifier_env *env, const char *fmt, ...) __printf(2, 3);
Both translation units that call it include that header
(kernel/bpf/diagnostics.c:17, kernel/bpf/verifier.c:38), so all call
sites were already format-checked. The attribute is present in the
parent of the Fixes-referenced commit (a8f427835394~1), so this hunk
adds no checking and silences no warning.
The changelog says "shows some missing printf attributes" and "Add these
to shut up the warning", but the quoted compiler output reports a single
warning and only bpf_diag_source() was actually missing an attribute.
Could the patch be reduced to the bpf_diag_source() change alone?
> @@ -794,7 +794,7 @@ static void diag_print_insn_context(struct bpf_verifier_env *env, u32 insn_idx,
> }
> }
>
> -static void bpf_diag_source(struct bpf_verifier_env *env, u32 insn_idx, const char *label,
> +__printf(4, 5) static void bpf_diag_source(struct bpf_verifier_env *env, u32 insn_idx, const char *label,
> const char *fmt, ...)
> {
> struct bpf_diag_scratch *scratch;
This is the hunk that is actually needed, but was the prefix inserted
without re-wrapping the declaration?
Adding the 15-character __printf(4, 5) prefix causes two formatting
issues:
1. Line 797 grows from 90 to 105 columns, exceeding the kernel's
100-column limit, so checkpatch will emit "WARNING: line length of 105
exceeds 100 columns".
2. The continuation line 798 is no longer aligned with the open
parenthesis. Before the patch, the first argument sat at column 29 and
line 798 (3 tabs + 4 spaces) also started at column 29, matching
exactly. Adding the prefix moves the open parenthesis so the first
argument now sits at column 44, while line 798 still starts at column 29
-> checkpatch "CHECK: Alignment should match open parenthesis".
The five nearest in-tree uses of this exact __printf(N, M) static form
all avoid both problems by re-wrapping the parameter list:
kernel/bpf/btf.c:1536-1537 len=75, continuation at col 47
kernel/bpf/btf.c:1560-1561 len=80, continuation at col 52
kernel/bpf/btf.c:8334-8335 len=79, continuation at col 41
kernel/bpf/verifier.c:7656-7657 len=92, continuation at col 50
kernel/bpf/verifier.c:8457-8458 len=92, continuation at col 50
Should the parameters be split across the two lines (or move label down)
to bring line 797 under 100 columns and re-indent line 798 to the new
parenthesis column?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35020865259
^ permalink raw reply [flat|nested] 2+ messages in thread