[PATCH bpf-next] selftests/bpf: Use snake_case for subtest descriptions in jit_insns_arena.c

[PATCH bpf-next] selftests/bpf: Use snake_case for subtest descriptions in jit_insns_arena.c

[Tiezhu Yang]

Normalize two inline assembly test descriptions in jit_insns_arena.c
from space-separated strings to snake_case for consistency and to fix
command-line filter parsing in test_progs. Otherwise:

$ sudo ./test_progs -t jit_insns/arena access with zero offset
Usage: test_progs [OPTION...]
Try `test_progs --help' or `test_progs --usage' for more information.

$ sudo ./test_progs -t jit_insns/cast to a user address
Usage: test_progs [OPTION...]
Try `test_progs --help' or `test_progs --usage' for more information.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
Note: This issue was introduced by the recent commit 54e8bde13a3a
("selftests/bpf: Add JIT instruction tests"). Since it's only in
bpf-next and hasn't been merged into mainline yet, no need to add
Fixes tag IMO.

 tools/testing/selftests/bpf/progs/jit_insns_arena.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/jit_insns_arena.c b/tools/testing/selftests/bpf/progs/jit_insns_arena.c
index b396d5aefb5a..16276ec192ac 100644
--- a/tools/testing/selftests/bpf/progs/jit_insns_arena.c
+++ b/tools/testing/selftests/bpf/progs/jit_insns_arena.c
@@ -34,7 +34,7 @@ struct {
 
 #if defined(__BPF_FEATURE_ADDR_SPACE_CAST)
 SEC("syscall")
-__description("arena access with zero offset")
+__description("arena_access_with_zero_offset")
 __success __retval(0)
 int arena_zero_off(void *ctx)
 {
@@ -82,7 +82,7 @@ int arena_zero_off(void *ctx)
 }
 
 SEC("syscall")
-__description("cast to a user address")
+__description("cast_to_a_user_address")
 __success __retval(0)
 int cast_to_user(void *ctx)
 {
-- 
2.42.0

Re: [PATCH bpf-next] selftests/bpf: Use snake_case for subtest descriptions in jit_insns_arena.c

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2547 bytes --]

> selftests/bpf: Use snake_case for subtest descriptions in jit_insns_arena.c

TL;DR: Commit message only: the snake_case rename is described as fixing a
test_progs filter parsing bug, but the failures come from unquoted shell
word-splitting; quoted names should already work, so the wording may be
inaccurate.

> selftests/bpf: Use snake_case for subtest descriptions in jit_insns_arena.c
>
> Normalize two inline assembly test descriptions in jit_insns_arena.c
> from space-separated strings to snake_case for consistency and to fix
> command-line filter parsing in test_progs. Otherwise:
>
> $ sudo ./test_progs -t jit_insns/arena access with zero offset
> Usage: test_progs [OPTION...]
> Try `test_progs --help' or `test_progs --usage' for more information.
>
> $ sudo ./test_progs -t jit_insns/cast to a user address
> Usage: test_progs [OPTION...]
> Try `test_progs --help' or `test_progs --usage' for more information.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

The commit message says the rename fixes "command-line filter parsing in
test_progs".  Is that description accurate?

In the quoted examples the shell word-splits the unquoted argument, so
test_progs receives extra positional arguments ("access", "with", "zero",
"offset").  parse_arg() in test_progs.c rejects those with:

    case ARGP_KEY_ARG:
        argp_usage(state);

and this happens before any filter is parsed.

The filter code itself handles spaces.  parse_test_list() splits only on
',', insert_test() splits on the first '/', and glob_match() compares
characters literally.  A quoted invocation such as:

    $ sudo ./test_progs -t 'jit_insns/arena access with zero offset'

should be accepted and match the subtest description through
match_subtest_desc().

Before this patch, -t jit_insns/arena_zero_off also worked, because
match_subtest_desc() additionally matches the BPF function name.

Other __description() strings in the tree contain spaces and show the same
shell-quoting behaviour when passed unquoted.

Could the commit message instead describe this as making the two names
convenient to pass unquoted on the command line, and consistent with the
snake_case names of the sibling subtests registered in test_jit_insns()
(alu_imm, alu_reg, ...), rather than as a fix for a parsing bug in
test_progs?


---
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/39190996107