From owner-cvs-all@FreeBSD.ORG Mon Nov 13 17:31:23 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 1F93416A523; Mon, 13 Nov 2006 17:31:23 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (vc4-2-0-87.dsl.netrack.net [199.45.160.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id 48DEE43EA2; Mon, 13 Nov 2006 17:22:22 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.13.4/8.13.4) with ESMTP id kADHJLTK097632; Mon, 13 Nov 2006 10:19:21 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Mon, 13 Nov 2006 10:19:58 -0700 (MST) Message-Id: <20061113.101958.-861030824.imp@bsdimp.com> To: bde@zeta.org.au From: "M. Warner Losh" In-Reply-To: <20061113214928.P76443@delplex.bde.org> References: <3801.1163410519@critter.freebsd.dk> <20061113214928.P76443@delplex.bde.org> X-Mailer: Mew version 4.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (harmony.bsdimp.com [127.0.0.1]); Mon, 13 Nov 2006 10:19:21 -0700 (MST) Cc: jkoshy@FreeBSD.ORG, phk@phk.freebsd.dk, src-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG, cvs-src@FreeBSD.ORG 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: Mon, 13 Nov 2006 17:31:23 -0000 In message: <20061113214928.P76443@delplex.bde.org> Bruce Evans writes: : 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 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 . 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. Warner