> diff --git a/tools/testing/selftests/bpf/README.rst > b/tools/testing/selftests/bpf/README.rst > index 07c834433b38..75ff1142c185 100644 > --- a/tools/testing/selftests/bpf/README.rst > +++ b/tools/testing/selftests/bpf/README.rst > @@ -187,7 +187,7 @@ The symptom for ``bpf_iter/netlink`` looks like > ... > 15: (bf) r2 = r7 > 16: (0f) r2 += r1 > - ; BPF_SEQ_PRINTF(seq, "%pK %-3d ", s, s->sk_protocol); > + ; BPF_SEQ_PRINTF(seq, "%p %-3d ", s, s->sk_protocol); > 17: (7b) *(u64 *)(r7 +0) = r2 > only read is supported
This excerpt is a historical clang-10 verifier log illustrating an old failure case. The patch rewrites the quoted source to "%p %-3d ", but the actual code now reads BPF_SEQ_PRINTF(seq, "0 %-3d ", s->sk_protocol) in bpf_iter_netlink.c - the sock pointer argument is gone entirely. Should the historical log remain untouched, or should it be updated to reflect that the netlink iterator no longer passes a pointer to BPF_SEQ_PRINTF? > diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf.c > b/tools/testing/selftests/bpf/prog_tests/snprintf.c > index 4e4a82d54f79..f2cd442c4683 100644 > --- a/tools/testing/selftests/bpf/prog_tests/snprintf.c > +++ b/tools/testing/selftests/bpf/prog_tests/snprintf.c > @@ -16,7 +16,7 @@ > #define MIN_SYM_RET sizeof(EXP_SYM_OUT) > > /* The third specifier, %p, is a hashed pointer which changes on every > reboot */ > -#define EXP_ADDR_OUT "0000000000000000 ffff00000add4e55 " > +#define EXP_ADDR_OUT "ffff00000add4e55 " > #define EXP_ADDR_RET sizeof(EXP_ADDR_OUT "unknownhashedptr") The comment says %p is the third specifier, but after removing %pK from the BPF program (test_snprintf.c:59), %p is now the second specifier in the format string "%px %p". Should this comment be updated to say "The second specifier"? [ ... ] > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_unix.c > b/tools/testing/selftests/bpf/progs/bpf_iter_unix.c > index a2652c8c3616..36690478992f 100644 > --- a/tools/testing/selftests/bpf/progs/bpf_iter_unix.c > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_unix.c > @@ -44,8 +44,7 @@ int dump_unix(struct bpf_iter__unix *ctx) > if (seq_num == 0) > BPF_SEQ_PRINTF(seq, "Num RefCount Protocol Flags > Type St Inode Path\n"); > > - BPF_SEQ_PRINTF(seq, "%pK: %08X %08X %08X %04X %02X %8lu", > - unix_sk, > + BPF_SEQ_PRINTF(seq, "%08u: %08X %08X %08X %04X %02X %8lu", 0, > sk->sk_refcnt.refs.counter, > 0, > sk->sk_state == TCP_LISTEN ? __SO_ACCEPTCON : 0, The header shows "Num RefCount" (18 columns before RefCount). The old format "%pK: " produced 18 columns (16 hex digits + ':' + ' '), matching the header width. The new format "%08u: " with argument 0 produces "00000000: " (10 columns), so every column from RefCount onward is now shifted 8 characters left of its header label. Should the header be updated to match the new field width, similar to how bpf_iter_netlink.c was adjusted from "sk Eth" to "sk Eth"? Regarding the commit message: > Networking recently replaced the socket output with a plain 0 because the > socket pointer added no value to the output and the %pK should be removed > fom library handling. The format remained otherwise unchanged in order not > to break any tools which are parsing this information. There's a typo: "fom library handling" should be "from library handling". Also, the statement "The format remained otherwise unchanged" is contradicted by the netlink header change from "sk Eth Pid" to "sk Eth Pid", and by the field width changes across all modified files (varying from 1 to 10 columns vs the original 16-17 columns). If maintaining format stability for parsers is the stated goal, should the width-preserving spelling be used consistently? --- 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/34614571384
