Re: [PATCH bpf-next] bpftool: Fix handling enum64 in btf dump sorting

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 02/09/2024 17:22, Daniel Borkmann wrote:
> On 9/1/24 11:30 PM, "Mykyta Yatsenko mykyta.yatsenko5"@gmail.com wrote:
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> Wrong function is used to access the first enum64 element.
>> Substituting btf_enum(t) with btf_enum64(t) for BTF_KIND_ENUM64.
>>
>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>> ---
>>   tools/bpf/bpftool/btf.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
>> index 6789c7a4d5ca..b0f12c511bb3 100644
>> --- a/tools/bpf/bpftool/btf.c
>> +++ b/tools/bpf/bpftool/btf.c
>> @@ -557,16 +557,23 @@ static const char *btf_type_sort_name(const 
>> struct btf *btf, __u32 index, bool f
>>       const struct btf_type *t = btf__type_by_id(btf, index);
>>         switch (btf_kind(t)) {
>> -    case BTF_KIND_ENUM:
>> -    case BTF_KIND_ENUM64: {
>> +    case BTF_KIND_ENUM: {
>>           int name_off = t->name_off;
>>             /* Use name of the first element for anonymous enums if 
>> allowed */
>> -        if (!from_ref && !t->name_off && btf_vlen(t))
>> +        if (!from_ref && !name_off && btf_vlen(t))
>>               name_off = btf_enum(t)->name_off;
>>             return btf__name_by_offset(btf, name_off);
>>       }
>
> Small nit, could we consolidate the logic into the below? Still 
> somewhat nicer
> than duplicating all of the rest.
Thanks for the suggestion, this makes sense,  I will apply it.
>
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 6789c7a4d5ca..aae6f5262c6a 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -562,8 +562,10 @@ static const char *btf_type_sort_name(const 
> struct btf *btf, __u32 index, bool f
>                 int name_off = t->name_off;
>
>                 /* Use name of the first element for anonymous enums 
> if allowed */
> -               if (!from_ref && !t->name_off && btf_vlen(t))
> -                       name_off = btf_enum(t)->name_off;
> +               if (!from_ref && !name_off && btf_vlen(t))
> +                       name_off = btf_kind(t) == BTF_KIND_ENUM64 ?
> +                                  btf_enum64(t)->name_off :
> +                                  btf_enum(t)->name_off;
>
>                 return btf__name_by_offset(btf, name_off);
>         }
>
> Thanks,
> Daniel
>