On Mon, Feb 01, 2010 at 12:03:07PM -0700, M. Warner Losh wrote:
> In message: <20100201183923.GA32901@onelab2.iet.unipi.it>
> Luigi Rizzo <email@example.com> writes:
> : On Mon, Feb 01, 2010 at 10:38:03AM -0700, M. Warner Losh wrote:
> : > I concur with Bruce's assessment. Leave well enough alone. We don't
> : > support those other compilers in the rest of the tree.
> : These are userland-visible system headers so the change is not for
> : building the system but for building generic userland programs on
> : FreeBSD with other compilers.
> : This said (sorry, i did not yet see Bruce's comment except in your
> : email) if i understand Bruce's comment well, the problem is that
> : no matter how we code it, bitfield layout is non specified, and we
> : need non-portable __packed and __align and actual verification
> : that things work as we want with a given compiler.
> Yes. The brief version is that the we're trying to capture the wire
> format in C, which provides no native way of doing that. The
> different byte ordering ifdefs, as well as the __packed and __aligned
> stuff tries to cope in a way we hope is the best.
> But my comment about not supporting these compilers is still relevant,
> I think. ...
not 100% sure.
> I think. We have special code in sys/defs.h to support the compilers
> we do... So if we don't support the compiler the __packed and
> __aligned macros are just defined away...
yes and no
> : > In particular, ARM generally gets broken when people naively monkey
> : > with these sorts of thing. I'll be quite put-out if this is another
> : > such change. Did you test it?
> : No i did not test it on ARM. But i cannot think how a compiler
> : would pack a u_int bitfield to 1 byte and refuse to do the same
> : with an u_char (or uint8_t if you like it better).
> Right. I cannot think of how the current ARM ABI does some of the
> things that it does.
fair enough. I realize i myself said that we need "actual verification
that things work as we want with a given compiler" but did not go
further after the 'grep' below returnes so many relevant hits.
> : At a quick test, bitfields declared using u_char or *int8_t
> : grep -Er 'int8_t|u_char' ~/FreeBSD/head/sys | grep ':[1-9]' | grep -v .svn
> : appear approximately 800 times in 60 files including a number of
> : device drivers, sys/dev/ciss/cissreg.h, sys/dev/ciss/cissio.h,
> ciss isn't known to work on ARM...
> : sys/dev/usb/usbdi.h and stuff that might be arm-related e.g.
> : sys/dev/usb/controller/avr32dci.h sys/dev/usb/controller/atmegadci.h
> but these are...
> : On these grounds (we already have 800 such instances, and my
> : changes are meant to improve compatibility of our system)
> : I'd like the changes to remain (possibly replacing u_char with
> : uint8_t if that looks better).
> I'd like to at least verify that ARM works by actually testing it
> rather than just speculating that it likely is OK. I'll queue up a
> build or two to make sure.
thanks a lot.
> My main concern here is to make sure that things work, and we've had
> better luck with u_int to date...
i cannot comment on this -- certainly this specific change 12-15 years
ago did not seem to be made to fix actual breakage.