Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 13 Nov 2006 22:00:22 +1100 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        Poul-Henning Kamp <phk@phk.freebsd.dk>
Cc:        Joseph Koshy <jkoshy@FreeBSD.org>, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, cvs-src@FreeBSD.org
Subject:   Re: cvs commit: src/include ar.h 
Message-ID:  <20061113214928.P76443@delplex.bde.org>
In-Reply-To: <3801.1163410519@critter.freebsd.dk>
References:  <3801.1163410519@critter.freebsd.dk>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 13 Nov 2006, Poul-Henning Kamp wrote:

> In message <20061113173927.Q75708@delplex.bde.org>, Bruce Evans writes:
>> On Mon, 13 Nov 2006, Joseph Koshy wrote:
>>
>>> jkoshy      2006-11-13 04:28:29 UTC
>>>
>>>  FreeBSD src repository
>>>
>>>  Modified files:
>>>    include              ar.h
>>>  Log:
>>>  Attempt to improve application portability by marking `struct ar_hdr'
>>>  as `packed'.
>>>  ...
>
> I agree with bruce that __packed is not the way to go.
>
> For things that represent communication protocols, even if
> they happen through files, I advocate using the functions
> in sys/endian.h to explicitly decode and encode fields into
> bytestrings.

But <ar.h> already uses byte strings for everything.  It just requires
no padding between the byte strings (so that the offsets are constant),
and no padding at the end (so that offsets are constant if the struct
is contained in another struct that is at least as careful about padding
an aligment).

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.

Bruce



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