Re: [PATCH bpf-next] bpftool: Preserve unsigned enum64 values in data dumps

[bot+bpf-ci]

> bpftool: Preserve unsigned enum64 values in data dumps

TL;DR: Possible incomplete fix: as requested in the v1 review,
btf_dumper_enum() was left unchanged, so an unmatched unsigned 4-byte enum
value like 0xffffffff may still print as -1 in bpftool dumps.

> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> index 3267163119dc..88c0da29f246 100644
> --- a/tools/bpf/bpftool/btf_dumper.c
> +++ b/tools/bpf/bpftool/btf_dumper.c

[ ... ]

> @@ -204,7 +204,10 @@ static int btf_dumper_enum64(const struct btf_dumper *d,
>  		}
>  	}
>
> -	jsonw_int(d->jw, value);
> +	if (btf_kflag(t))
> +		jsonw_int(d->jw, value);
> +	else
> +		jsonw_uint(d->jw, value);
>  	return 0;
>  }
>

Alexei Starovoitov replied to v1 that btf_dumper_enum() has the same
problem and asked for both functions to be fixed in one patch, and he
supplied a diff for it.  I could not find a v2 on lore, and this patch still
only touches btf_dumper_enum64().

Was the request to cover btf_dumper_enum() intentionally left out?

btf_dumper_enum() still sign-extends the 1, 2 and 4 byte loads and always
prints an unmatched value with jsonw_int():

static int btf_dumper_enum(const struct btf_dumper *d,
			   const struct btf_type *t,
			   const void *data)
{
	...
	switch (t->size) {
	case 4:
		value = *(__s32 *)data;
		break;
	...
	}
	...
	jsonw_int(d->jw, value);
	return 0;
}

For example, an unsigned 4-byte enum holding 0xffffffff that matches no
enumerator would still be printed as -1.  Should btf_dumper_enum() pick the
zero- or sign-extending load based on btf_kflag(t), compare enumerators with
matching signedness, and call jsonw_uint() for unsigned enums, as in the
diff from the v1 review?


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