Re: Why tvb_get_bits() assumes Big Endian?

[Filipe Laíns <lains () archlinux org>]

On Thu, 2020-07-30 at 08:06 +0200, Tomasz Moń wrote:
> Hello,
>
> The tvb_get_bits() function family contains following comment:
>     /* note that encoding has no meaning here, as the tvb is
> considered to contain an octet array */
>
> I don't understand the reason. What am I missing?
>
> The actual octets in tvb contain the bits ordered as expected, so the
> MSB/LSB-first problem within the octet itself does not apply (and I
> think this is what the comment refers to). However, when the bit
> field
> (e.g. 11 bits) spans across multiple octets, then the endianness does
> matter (i.e. which of the two octets contains the more significant
> part of the 11-bit value). Simply assuming Big Endian at the
> cross-octet boundary prevents USB HID dissection from using
> tvb_get_bits() directly because USB uses Little Endian.
>
> Best Regards,
> Tomasz Moń

I had planned to add support for little endian in tvb_get_bits*, the
reason I dropped it is because as Dr. Lars points out in [1] the
endianness can be at two levels here. It can be in the buffer, as Jaap
also points out, or in the value, what I need for HID. Or both, I
guess. With aligned data it doesn't matter, but here it does.

For this reason I decided to drop it in favor of tvb_get_bits_array
+ pletoh*. It is simple enough to be used standalone.

I would actually like to see tvb_get_bits* throw an error
on ENC_LITTLE_ENDIAN as it is clearly not supported.

I think it's better to force users to call two functions and choose
their behavior than to leave them scratching their heads when
ENC_LITTLE_ENDIAN is not behaving as they were expecting. These bugs
could also be hard to track down as in some cases the output for both
possible implementations would be the same. In HID for eg. we have
mainly aligned data in the reports, if we were using the network
implementation, we would only notice the bug when someone comes with a
capture where the data is unaligned.

[1] https://code.wireshark.org/review/#/c/37949/

Filipe Laíns

Attachment: signature.asc
Description: This is a digitally signed message part

___________________________________________________________________________
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