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

[bot+bpf-ci@kernel.org]

[-- 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