Re: Why tvb_get_bits() assumes Big Endian?

[Tomasz Moń <desowin () gmail com>]

On Thu, Jul 30, 2020 at 6:22 PM Filipe Laíns <lains () archlinux org> wrote:
> I think it would be reasonable to say that cases which need such
> special handling can do it themselves. Wireshark should still provide
> all the routines required to make this work, but I don't think it needs
> to necessarily provide a function for very specific use cases.

I would consider the special case to be swapping/reversing the bits
within a byte. Which is different from the issue discussed here.

> With that said, if we want to indeed support this then we need new
> functions. We can't touch the current signature as that will break the
> ABI.

tvb_get_bits() functions do take the encoding parameter already, but
simply ignore it. If someone calls it with ENC_LITTLE_ENDIAN while
expecting it to work like with ENC_BIG_ENDIAN, then I would say that's
a bug. Everyone who indeed wanted the little endian handling would
notice that it doesn't really do the job.

> My proposal would be to make tvb_get_bits* respect little endian for
> the value itself and add correspondent tvb_get_network_bits* methods
> that would treat the buffer as little endian. What do you think?

tvb_get_network_bits to treat buffer as little endian seems weird,
because the network byte order is big endian.

> Would adjusting the behavior of tvb_get_bits* be considered a breaking
> change? I mean, this behavior is not documented.

I wouldn't consider finally handling the ENC_LITTLE_ENDIAN in
tvb_get_bits() as a breaking change. Such code path used to be
DISSECTOR_ASSERT_NOT_REACHED() after all. It might just be a bit late
to implement, but it wouldn't be the first thing that finally got
implemented after many years.

Header fields with bitmasks are already working just fine with
ENC_LITTLE_ENDIAN on byte aligned data. Implementing ENC_LITTLE_ENDIAN
in tvb_get_bits() would just allow doing essentially the same for
unaligned data.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe