Re: [PATCH] tools/sched_ext: use btf_vlen() helper in compat.h

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch fails to resolve the `-Wsign-compare` warning it claims to fix because `btf_vlen()` returns `__u32`, not `__u16`.
--

commit 4880227aa1b0e483fd0500c9f49fa0639cd32d8b
Author: luoliang <luoliang@kylinos.cn>

tools/sched_ext: use btf_vlen() helper in compat.h

This commit replaces open-coded BTF_INFO_VLEN() lookups with btf_vlen() in 
compat.h, intending to fix a -Wsign-compare compiler warning.

>     libbpf already exposes btf_vlen() for exactly this purpose; it
>     returns __u16, the natural width of the vlen field, and the usual
>     integer promotions turn the 'int < __u16' comparison into a plain
>     'int < int' so the warning goes away without any cast. This matches

[Severity: Medium]
Is this description accurate?

Looking at the definitions in include/linux/btf.h and tools/lib/bpf/btf.h,
btf_vlen() is defined to return an unsigned 32-bit integer (u32 or __u32),
not a __u16:

static inline u32 btf_vlen(const struct btf_type *t)
{
	return btf_type_vlen(t);
}

Because it returns an unsigned 32-bit integer, won't comparing it against
a signed int still trigger the -Wsign-compare warning?

> diff --git a/tools/sched_ext/include/scx/compat.h b/tools/sched_ext/include/scx/compat.h
> index 602f07061ee39..5b92e9c85d03d 100644
> --- a/tools/sched_ext/include/scx/compat.h
> +++ b/tools/sched_ext/include/scx/compat.h
> @@ -42,7 +42,7 @@ static inline bool __COMPAT_read_enum(const char *type, const char *name, u64 *v
>  	if (btf_is_enum(t)) {
>  		struct btf_enum *e = btf_enum(t);
>  
> -		for (i = 0; i < BTF_INFO_VLEN(t->info); i++) {
> +		for (i = 0; i < btf_vlen(t); i++) {

[Severity: Medium]
Does this code still compare a signed int 'i' against the unsigned 32-bit
return value of btf_vlen(t)?

Other places in the kernel (like kernel/bpf/inode.c) and bpftool typically
avoid this warning by either declaring their loop counter as a __u32, or by
explicitly assigning the result of btf_vlen() to an int variable before the
loop. Should one of those approaches be used here instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630031246.2618216-1-luoliang@kylinos.cn?part=1