Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Nov 2006 09:55:55 +0300
From:      Yar Tikhiy <yar@comp.chem.msu.su>
To:        "M. Warner Losh" <imp@bsdimp.com>
Cc:        src-committers@FreeBSD.ORG, bde@zeta.org.au, jkoshy@FreeBSD.ORG, cvs-all@FreeBSD.ORG, phk@phk.freebsd.dk, cvs-src@FreeBSD.ORG
Subject:   Re: cvs commit: src/include ar.h
Message-ID:  <20061117065555.GE49602@comp.chem.msu.su>
In-Reply-To: <20061116.165207.1661914048.imp@bsdimp.com>
References:  <20061113214928.P76443@delplex.bde.org> <20061113.101958.-861030824.imp@bsdimp.com> <20061116090412.GB37133@comp.chem.msu.su> <20061116.165207.1661914048.imp@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Nov 16, 2006 at 04:52:07PM -0700, M. Warner Losh wrote:
> In message: <20061116090412.GB37133@comp.chem.msu.su>
>             Yar Tikhiy <yar@comp.chem.msu.su> writes:
> : On Mon, Nov 13, 2006 at 10:19:58AM -0700, M. Warner Losh wrote:
> : > : 
> : > : BTW, you are responsible for the __packed in <netinet/ip.h>.  Please remove
> : > : it.  The __CTASSERT() is enough to detect if heroic packing is ever needed.
> : > : The only danger is if something has grown to depend on __packed reducing
> : > : alignment as a side effect.  E.g., suppose we had a byte string containing
> : > : a bytewise copy of a struct ip.  If the copy might be misaligned, then it
> : > : should be copied to an actual struct ip before accessing it as a struct,
> : > : but code that accesses it directly using ((struct ip *)&bs[N]) would work
> : > : now due to the reduced alignment.  Places that really need __packed should
> : > : probably use __aligned() to restore the natural alignment.
> : > 
> : > DO NOT REMOVE IT.  IT IS ABSOLUTELY REQUIRED FOR ARM TO WORK RIGHT.
> : > If you want to remove it, then you must make sure arm works right
> : > after it because I'll add it back.
> : 
> : 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.

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
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.

-- 
Yar



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061117065555.GE49602>