Date: Wed, 16 Oct 2002 18:03:24 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Jeffrey Hsu <hsu@FreeBSD.ORG> Cc: Poul-Henning Kamp <phk@FreeBSD.ORG>, <net@FreeBSD.ORG>, <arch@FreeBSD.ORG> Subject: Re: RFC: eliminating the _IP_VHL hack. Message-ID: <20021016172019.W4358-100000@gamplex.bde.org> In-Reply-To: <0H41001UNVU5KX@mta5.snfc21.pbi.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 15 Oct 2002, Jeffrey Hsu wrote:
> > The side effect of having some source-files using the _IP_VHL hack and
> > some not is that sizeof(struct ip) varies from file to file, which at
> > best is confusing an at worst the source of some really evil bugs.
There is no such effect, or ip would not work. Bitfields are inherently
unportable, but here everything is carefully packed so that there is a
good chance that both versions give the same struct layout for all
non-broken compilers (the layout needs to be compiler-independent too).
> > I would therefore propose to eliminate the _IP_VHL hack from the kernel
> > to end this state of (potential) confusion
This would remove the least unportable version.
> This problem could be solved more easily by changing the u_int back
> to an u_char, as it used to be before rev 1.15:
>
> Index: ip.h
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/ip.h,v
> retrieving revision 1.19
> diff -u -r1.19 ip.h
> --- ip.h 14 Dec 2001 19:37:32 -0000 1.19
> +++ ip.h 16 Oct 2002 01:15:48 -0000
> @@ -51,11 +51,11 @@
> u_char ip_vhl; /* version << 4 | header length >> 2 */
> #else
> #if BYTE_ORDER == LITTLE_ENDIAN
> - u_int ip_hl:4, /* header length */
> + u_char ip_hl:4, /* header length */
> ip_v:4; /* version */
> #endif
> #if BYTE_ORDER == BIG_ENDIAN
> - u_int ip_v:4, /* version */
> + u_char ip_v:4, /* version */
> ip_hl:4; /* header length */
> #endif
> #endif /* not _IP_VHL */
>
> But, if we were to pick one or the other to discard, I would keep the
> IP_VHL because that field really is a byte in the IP header
This would just guarantee unportable behaviour by untranslating from C.
C compilers reject bit-fields that are declared to have u_char, since
u_char is not a permitted type for a bit-field. E.g., from the C99
draft n869.txt:
6.7.2.1 Language 6.7.2.1
WG14/N869 Committee Draft -- January 18, 1999 115
[#8] A bit-field shall have a type that is a qualified or
unqualified version of _Bool, signed int, or unsigned int.
...
Little is lost by restricting the type of a bitfield in this way, since
the width of a bitfield can be determined without lokking at its type.
Bit-fields that are declared to have types other than the standard
types for them are a poorly documented gcc extension. The type of a
bit-field has little or no affect on packing of bits near the bitfield,
at least if everything is packed manually. However, the types of
bit-fields within a struct affects padding at the end of a struct in
a (usually) less than useful way: if a standard standard type is used,
then structs get padded if necessary to give alignment of that type.
E.g., on i386's the following structs have size 4 although you probably
want them to have size 1:
struct foo1 { int bar: 1; };
struct foo2 { int bar: 8; };
TenDRA is bug-for-bug compatible with gcc here.
This can be worked around if the compiler is gcc by declaring the
struct as __packed (__aligned(1) doesn't seem to work), or by declaring
the bit-field type as char, or on some systems by using -mno-bit-align.
These workarounds are even more unportable than bit-fields. However,
there is no problem with struct ip yet, because everything is manually
packed to work with "all" C compilers on "all" systems. This will
break on systems with 64-bit ints, since sizeof(struct ip) == 20 is
not a multiple of 8.
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021016172019.W4358-100000>