Wireshark · Ethereal-dev: [ethereal-dev] ANSI bitfields

["Gilbert Ramirez Jr." <gram@xxxxxxxxxxxxxxxxxxx>]

I am attempting to compile ethereal and wiretap on AIX 4.3 using IBM's
ANSI C compiler instead of gcc. IBM's compiler complains about the
following:

cc -DHAVE_CONFIG_H -I. -I. -I.   -g -I/usr/local/lib/glib/include
-I/usr/local/include -c capture.c
"packet.h", line 164.11: 1506-159 (E) Bit-field type specified for igmp_v
is not valid. Type unsigned assumed.
"packet.h", line 165.11: 1506-159 (E) Bit-field type specified for igmp_t
is not valid. Type unsigned assumed.
"packet.h", line 188.11: 1506-159 (E) Bit-field type specified for ip_v is
not valid. Type unsigned assumed.
"packet.h", line 189.11: 1506-159 (E) Bit-field type specified for ip_hl is
not valid. Type unsigned assumed.
"packet.h", line 310.11: 1506-159 (E) Bit-field type specified for th_off
is not valid. Type unsigned assumed.
"packet.h", line 311.11: 1506-159 (E) Bit-field type specified for th_x2 is
not valid. Type unsigned assumed.

It compiles ethereal, but it does not run correctly.

In checking the first bitfield error, I see this:
/* IGMP structs and definitions */
typedef struct _e_igmp {
#if BYTE_ORDER == BIG_ENDIAN
  guint8  igmp_v:4;
  guint8  igmp_t:4;
#else /* Little endian */
  guint8  igmp_t:4;
  guint8  igmp_v:4;
#endif
  guint8  igmp_unused;
  guint16 igmp_cksum;
  guint32 igmp_gaddr;
} e_igmp;

IBM's C compiler does not want to make bitfields inside of a guint8
(unsigned char) field.  I checked in K&R (2nd edition, the ANSI-version)
and found this in section A8, page 213:

	A field member (which need not have a declarator and thus may be unnamed)
	has type int, unsigned int, or signed int, and is interpreted as an object
	of integral type of the specified length in bits.

So, gcc allows bitfields inside of char fields, but this does not
follow ANSI rules. The next paragraph states:

	It is advisable to read the language rules for storing bit-fields
	as "implementation-dependent" without qualification. Structures with
	bit-fields may be used as a portable way of attempting to reduce the
	storage required for a structure (with the probable cost of increasing the
	instruction space, and time, needed to access the fields), or as a non-
	portable way to describe a storage layout known at the bit level.

We see the nibbles being re-arranged in the e_igmp struct in an attempt
to overcome the non-portability (by checking the endianness of the
CPU), but the ability to use bitfields inside an unsigned char is
itself non-portable.  We should probably not rely on gcc's ability to
address bits inside unsigned chars.

Furthermore, I don't think ethereal should use the structs we see in
packet.h as templates where you do a memcpy of the entire structure
from the packet data to your structure.  When using a compiler other
than gcc, you cannot guarantee that your fields will not be padded to
fit in 4-byte boudnaries.  So, for example, one cannot guarantee that
igmp_unused and igmp_cksum are next to each other, or are 3 bytes apart.

Unless I'm wrong, we should copy each field of the struct from the packet
data into the struct, instead of memcpy'ing the entire struct at once. As
an example, see packet-tr.c. I copy each field, one by one.

Is the padding issue a valid concern? I'd love for someone to tell me that
I'm wrong.

--gilbert

-- 
Gilbert Ramirez                Voice:  +1 210 358 4032
Technical Services             Fax:    +1 210 358 1122
University Health System       San Antonio, Texas, USA