--oJ71EGRlYNjSvfq7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Jun 06, 2006 at 12:13:11PM -0500, David Young wrote:
> On Tue, Jun 06, 2006 at 05:49:39PM +0100, Rui Paulo wrote:
> > struct ip {
> >=20
> > 	[...]
> >=20
> > 	union {
> > 		struct ip_ds {		/* differentiated services */
> > #if BYTE_ORDER =3D=3D LITTLE_ENDIAN
> > 			u_int8_t  ds_ecn:2;	/* ecn field */
> > 			u_int8_t  ds_cp:6;	/* diffserv codepoint */
> > #endif
> > #if BYTE_ORDER =3D=3D BIG_ENDIAN
> > 			u_int8_t  ds_cp:6;	/* diffserv codepoint */
> > 			u_int8_t  ds_ecn:2;	/* ecn field */
> > #endif
> > 		} ip_ds __attribute__((__packed__));
>=20
> I think bitfields are just fine for flags and other tiny fields, but
> I object to the use of bitfields to define any binary format.  I don't
> think the C standard guarantees any particular bit order, and I would
> have to pull out GCC's manual to interpret your ip_ds definition, above.
> At a glance, the LITTLE_ENDIAN definition looks backwards.

Actually, the LITTLE_ENDIAN one is right. It took me a while to figure it=
=20
out (netatalk does this too).

My understanding is that bitfields are packed in bytes in such a way that
if you grow the storage (char to short to long to long long), the
previously-defined bit fields stay in the same bits in memory. Likewise,=20
if you add bit fields, the old ones stay put. And bit fields are=20
contiguous in the storage word (ds_cp is extracted using some sort of bit=
=20
shift and mask; you don't move different parts of the word around and OR=20
the parts together).

Let's look at the LITTLE_ENDIAN case. For ds_ecn to not move regardless of=
=20
what comes after, it has to be in byte 0 in memory, which is the LSB of a=
=20
short or a long. So it has to be in the 2^0 and 2^1 bits.

For contiguous packing, ds_cp then has to be in the 2^7 -> 2^2 bits. And=20
because of size, we just happen to fill only 1 byte.

Likewise for the BIG_ENDIAN case, for ds_cp to not move, it has to be in=20
byte 0 of memory, which is the MSB of a short or long. For an 8-bit value,=
=20
that's 2^7 -> 2^2. For contiguous packing, ds_ecn then has to be in the=20
2^1 and 2^0 bits.

So it all works out.

That said, I'd much prefer bit masks and shifts. The fact it took four=20
paragraphs to describe this to a very savvy developer indicates that this=
=20
is an unclear area to venture into. Bit masks and shifts are much easier=20
for everyone to understand. Also, bitmask_snprintf() can use one common=20
string to format the output for printing (the "new" bitmask format can=20
print fields).

Take care,

Bill

--oJ71EGRlYNjSvfq7
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (NetBSD)

iD8DBQFEhcAmWz+3JHUci9cRAvIoAJwM/6FcqNaBlhQG/mKbbXTyuqoUvwCfSznD
gwKtfG8GyGwloSUKENigvd8=
=M9Ti
-----END PGP SIGNATURE-----

--oJ71EGRlYNjSvfq7--