Date: Mon, 1 Feb 2010 20:34:22 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: "M. Warner Losh" <imp@bsdimp.com> Cc: svn-src-head@FreeBSD.org, luigi@FreeBSD.org, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, brde@optusnet.com.au Subject: Re: svn commit: r203343 - head/sys/netinet Message-ID: <20100201193422.GA33864@onelab2.iet.unipi.it> In-Reply-To: <20100201.120307.4959786928951104.imp@bsdimp.com> References: <20100202012830.B1230@besplex.bde.org> <20100201.103803.850602504823287588.imp@bsdimp.com> <20100201183923.GA32901@onelab2.iet.unipi.it> <20100201.120307.4959786928951104.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Feb 01, 2010 at 12:03:07PM -0700, M. Warner Losh wrote: > In message: <20100201183923.GA32901@onelab2.iet.unipi.it> > Luigi Rizzo <rizzo@iet.unipi.it> 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. cheers luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100201193422.GA33864>