From owner-cvs-all@FreeBSD.ORG Fri Nov 17 09:48:17 2006 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DFCB916A4F1; Fri, 17 Nov 2006 09:48:17 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2-3.pacific.net.au [61.8.2.226]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5011343D5C; Fri, 17 Nov 2006 09:48:17 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.2.162]) by mailout2.pacific.net.au (Postfix) with ESMTP id 4A7E46E2AA; Fri, 17 Nov 2006 20:48:15 +1100 (EST) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy1.pacific.net.au (Postfix) with ESMTP id 4A1FA8C1B; Fri, 17 Nov 2006 20:48:14 +1100 (EST) Date: Fri, 17 Nov 2006 20:48:13 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Yar Tikhiy In-Reply-To: <20061117065555.GE49602@comp.chem.msu.su> Message-ID: <20061117201432.X11101@delplex.bde.org> References: <20061113214928.P76443@delplex.bde.org> <20061113.101958.-861030824.imp@bsdimp.com> <20061116090412.GB37133@comp.chem.msu.su> <20061116.165207.1661914048.imp@bsdimp.com> <20061117065555.GE49602@comp.chem.msu.su> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: src-committers@freebsd.org, jkoshy@freebsd.org, cvs-all@freebsd.org, phk@phk.freebsd.dk, cvs-src@freebsd.org, "M. Warner Losh" Subject: Re: cvs commit: src/include ar.h X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Nov 2006 09:48:18 -0000 On Fri, 17 Nov 2006, Yar Tikhiy wrote: > On Thu, Nov 16, 2006 at 04:52:07PM -0700, M. Warner Losh wrote: >> In message: <20061116090412.GB37133@comp.chem.msu.su> >> Yar Tikhiy writes: >> : Many years ago I was taught that comments in code could help to >> : avoid such clashes in software development. Is this true no more? ;-) >> >> You don't add comments like: >> >> i++; // Add one to i. >> >> This is a similar class. It is for any compiler that has differing >> alignment requirements than i386. But you do add comments like: i++; /* Don't add 1 to i (sic). */ when `i' is a const variable in magic device memory that needs to be used by read/add 1/write (where the non-atomicness of this doesn't matter for some reason). > This is an oversimplification of the case. If it were so simple, > no doubts about it would be raised. That's why I suggested adding > a comment explaining that historically struct ip was lucky to be > packed/aligned properly, but that wasn't backed by the C standard > in fact, and eventually architectures appeared, e.g., ARM, which > broke the false assumption. It's a rather edifying case. Then There is no luck about the alignment. Structs are always aligned, especially for arm, unless you break them using __packed. It never took much luck for the struct to be packed, since the struct has its largest members at the end so padding for alignment would affect most arches. It now doesn't take any luck to detect mispacking, since packing is now __CTASSERT()ed. Packing occurs naturally wthout using __packed for all arches supported by FreeBSD and Linux-2.6.10 (**), especially for arm since arm is on 32-bits so padding at the end to make the struct size a multiple of 64 bits would be weird. > you'll have a smaller chance of having to yell in capital letters > again, "DO NOT REMOVE IT. IT IS ABSOLUTELY REQUIRED FOR ARM TO > WORK RIGHT." -- hopefully, not only regarding struct ip. For that the comment should be something like: __packed; /* Align (sic) to work around bugs on arm (*). */ but I doubt that arm is that broken. (*) See this thread for more details. (**) Linux-2.6.10 seems to have only struct iphdr, and has gratuitous unportabilities in that like the ones that were removed in FreeBSD in the commit that added __packed: it uses bitfields to unportabalize the header length and version, and endianness ifdefs to work around the unportabilities; in addition, it uses bitfields of type char to additionally unportabalize the header length and version. Bruce