Re: [PATCH bpf-next v1 2/4] selftests/bpf: Add tests for bpftool btf dump format c

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The comment describing the `holey` struct layout contradicts the actual code, creating a 64-bit hole instead of the documented 32-bit hole.
--

--- Patch [2]: [PATCH bpf-next v1 2/4] selftests/bpf: Add tests for bpftool btf dump format c ---
commit 6c049c9e731fda95ed237645c2367be42ed67768
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

selftests/bpf: Add tests for bpftool btf dump format c

This commit adds a selftest for "bpftool btf dump format c" to ensure that
the generated vmlinux.h header contains the correct macros, pragmas, and
padding widths. It verifies these features across both sorted and unsorted
BTF output formats.

> diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c b/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c
> new file mode 100644
> index 0000000000000..53bb7065b3cae
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c

[ ... ]

> +/*
> + * struct holey { int c; <32 bit hole> int tail; };
> + *
> + * One record with a hole, and a 4-byte long to pad it with. How records
> + * themselves are rendered is already covered by the build, so the fixture does
> + * not need to be more elaborate than that.
> + */
> +static struct btf *mk_btf(void)
> +{

[ ... ]

> +	id = btf__add_struct(btf, "holey", 16);
> +	if (!ASSERT_GT(id, 0, "struct_holey"))
> +		goto err_out;
> +
> +	err = btf__add_field(btf, "c", 1, 0, 0);
> +	if (!ASSERT_OK(err, "holey_c"))
> +		goto err_out;
> +
> +	err = btf__add_field(btf, "tail", 1, 96, 0);

[Severity: Low]
This isn't a bug, but does this create a 64-bit hole instead of the 32-bit hole
mentioned in the comment above?

Since "c" is a 32-bit integer ending at bit 32 and "tail" starts at bit offset
96, the gap between them appears to be 64 bits.

Could the comment be updated to accurately reflect the actual struct layout
generated here?

> +	if (!ASSERT_OK(err, "holey_tail"))
> +		goto err_out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820000627.3826188-1-ihor.solodrai@linux.dev?part=2