Re: [PATCH bpf-next 2/3] libbpf: Fix dumping big-endian bitfields

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Tue, 2021-10-12 at 06:03 +0200, Andrii Nakryiko wrote:
> On Tue, Oct 12, 2021 at 4:32 AM Ilya Leoshkevich <iii@linux.ibm.com>
> wrote:
> > 
> > On big-endian arches not only bytes, but also bits are numbered in
> > reverse order (see e.g. S/390 ELF ABI Supplement, but this is also
> > true
> > for other big-endian arches as well).
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  tools/lib/bpf/btf_dump.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> > index ad6df97295ae..ab45771d0cb4 100644
> > --- a/tools/lib/bpf/btf_dump.c
> > +++ b/tools/lib/bpf/btf_dump.c
> > @@ -1577,14 +1577,15 @@ static int
> > btf_dump_get_bitfield_value(struct btf_dump *d,
> >         /* Bitfield value retrieval is done in two steps; first
> > relevant bytes are
> >          * stored in num, then we left/right shift num to eliminate
> > irrelevant bits.
> >          */
> > -       nr_copy_bits = bit_sz + bits_offset;
> >         nr_copy_bytes = t->size;
> >  #if __BYTE_ORDER == __LITTLE_ENDIAN
> >         for (i = nr_copy_bytes - 1; i >= 0; i--)
> >                 num = num * 256 + bytes[i];
> > +       nr_copy_bits = bit_sz + bits_offset;
> >  #elif __BYTE_ORDER == __BIG_ENDIAN
> >         for (i = 0; i < nr_copy_bytes; i++)
> >                 num = num * 256 + bytes[i];
> > +       nr_copy_bits = nr_copy_bytes * 8 - bits_offset;
> 
> oh, I remember dealing with this in the context of pahole. Just one
> nit, please use t->size instead of nr_copy_bytes, I think it will
> make
> it a bit more explicit (nr_copy_bytes is logically mutable, though
> only in little-endian case, but still).

Both sz and num_copy_bytes look redundant to be honest. What do you
think about dropping both completely and just using t->size everywhere
instead?

> 
> >  #else
> >  # error "Unrecognized __BYTE_ORDER__"
> >  #endif
> > --
> > 2.31.1
> >