From owner-svn-src-all@FreeBSD.ORG Mon Feb 1 19:25:45 2010 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 24FE6106568B; Mon, 1 Feb 2010 19:25:45 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id A62B18FC0A; Mon, 1 Feb 2010 19:25:44 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 775D6730A1; Mon, 1 Feb 2010 20:34:22 +0100 (CET) Date: Mon, 1 Feb 2010 20:34:22 +0100 From: Luigi Rizzo To: "M. Warner Losh" Message-ID: <20100201193422.GA33864@onelab2.iet.unipi.it> References: <20100202012830.B1230@besplex.bde.org> <20100201.103803.850602504823287588.imp@bsdimp.com> <20100201183923.GA32901@onelab2.iet.unipi.it> <20100201.120307.4959786928951104.imp@bsdimp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100201.120307.4959786928951104.imp@bsdimp.com> User-Agent: Mutt/1.4.2.3i 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 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Feb 2010 19:25:45 -0000 On Mon, Feb 01, 2010 at 12:03:07PM -0700, M. Warner Losh wrote: > In message: <20100201183923.GA32901@onelab2.iet.unipi.it> > Luigi Rizzo 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