Wireshark-dev: Re: [Wireshark-dev] C Bitfield types and alignment

From: Guy Harris <guy@xxxxxxxxxxxx>
Date: Mon, 5 Dec 2011 14:01:10 -0800
On Dec 5, 2011, at 1:41 PM, Bill Meier wrote:

> My curiosity was piqued by Guy's comment in SVN #40078:
> 
> "Bitfields indicate how many bits they are; "guint8 foo:4" is
> self-contradictory (it's 4 bits, not 8).  Furthermore, the C language
> doesn't support "unsigned char" as a bitfield type;some compilers might
> accept that, but if you crank up the warning levels, even GCC will warn
> about that."
> 
> So: I decided to do some research.

Shorter everything about C bitfields:

	C bitfields are a tool of Satan.

tcpdump used to rely on OS headers for various protocol data structures, including the IPv4 header; some of them used bitfields for the version and header length fields, and I think some others used shift-and-mask.  There was a fair bit of crap trying to deal with that, including making sure the order of the version and header length fields was correct.

I finally gave up (the problem that provoked it was DNS, where HP-UX had old headers that didn't define all the flags that the tcpdump dissector wanted) and changed tcpdump to have its own copies of all those headers (by lifting copies from various BSDs); I did *NOT* go with bitfields in the IPv4 header - life's too short for that.

> What I found:
> 
> 1. The only valid types for a C bitfield (at least for C89) are [unsigned|signed] int (as implied by Guy).
> 
> However, supposedly "many" C compilers allow other types.
> Given that AFAIK we've not had any complaints about the use of
> guint8 & etc as a bitfield type, presumably the compilers commonly used to compile Wireshark accept same.

They accept it but may warn about it; we were getting warnings from it.

My inclination here would be to stop using bitfields in the radiotap parser.