From owner-freebsd-amd64@FreeBSD.ORG Wed Dec 29 12:09:24 2004 Return-Path: Delivered-To: freebsd-amd64@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 5146916A4CE; Wed, 29 Dec 2004 12:09:24 +0000 (GMT) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id A22FE43D49; Wed, 29 Dec 2004 12:09:23 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au [61.8.0.87])iBTC9IHn007748; Wed, 29 Dec 2004 23:09:18 +1100 Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) iBTC9DIm024986; Wed, 29 Dec 2004 23:09:15 +1100 Date: Wed, 29 Dec 2004 23:09:14 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Peter Wemm In-Reply-To: <200412281526.48335.peter@wemm.org> Message-ID: <20041229221042.X92684@delplex.bde.org> References: <1936407230.20041214120051@bk.ru> <200412281432.56309.peter@wemm.org> <41D1E0B2.3020500@dellroad.org> <200412281526.48335.peter@wemm.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: freebsd-amd64@freebsd.org cc: "James R. Van Artsalen" cc: archie@freebsd.org cc: Archie Cobbs Subject: Re: mpd@amd64 X-BeenThere: freebsd-amd64@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Porting FreeBSD to the AMD64 platform List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Dec 2004 12:09:24 -0000 On Tue, 28 Dec 2004, Peter Wemm wrote: > On Tuesday 28 December 2004 02:39 pm, Archie Cobbs wrote: > > Peter Wemm wrote: > > >>Attached is a slightly different patch. This is against the source > > >>currently in ports, and it fixes two additional questionable uses > > >> of va_start (). > > > > > > Just as a BTW; powerpc platforms need this series of changes too, > > > not just amd64. > > > > Thanks.. this change should work for all. > > > > > BTW2: I wonder if some of the attached changes really could be > > > done more simply with va_copy()... Not that it matters of course. > > > > Probably.. that's new though? No va_copy() on 4.x. > > Yes, its formerly part of C99, but gcc has had it for a while. In > gcc-2.95 it is implemented in the gnu version of stdarg.h as > __va_copy(), but we neglected to add it to our includes in 4.x: Probably not. va_copy() is only needed for restarting at a place other than the start, and possibly for acting on multiple active copies of the args (is this required to work?), but the patch seemed to only need restarting at the start. va_start() is ideal for restarting at the start; using va_copy() for this is just illogical, gratuitously unportable to systems that don't implement this part of C99 (e.g., RELENG_4), and takes slightly more code (for declaring the variable that holds the copy): dump/optr.c still gives an example of how not to use va_copy() (rev.1.27). Fix: % Index: optr.c % =================================================================== % RCS file: /home/ncvs/src/sbin/dump/optr.c,v % retrieving revision 1.30 % diff -u -2 -r1.30 optr.c % --- optr.c 19 Jun 2004 22:41:18 -0000 1.30 % +++ optr.c 20 Jun 2004 07:05:14 -0000 % @@ -234,5 +234,4 @@ % { % va_list ap; % - va_list ap2; % % (void) fprintf(stderr," DUMP: "); The patch removes the extra declaration which is the only extra code (by line count) needed to use va_copy. (But having an extra line of code is another style bug -- in KNF, local declarations of the same type should be on the same line if possible.) % @@ -241,11 +240,11 @@ % #endif % va_start(ap, fmt); % - va_copy(ap2, ap); The patch removes the va_copy() which gives 2 active copies throughout the function. We only need 1 active copy at a time. % (void) vfprintf(stderr, fmt, ap); % + va_end(ap); va_end() as soon as we have finished with a pass over the args is more natural than at the end of the function, and is needed to reuse the ap variable. % (void) fflush(stdout); % (void) fflush(stderr); % - (void) vsnprintf(lastmsg, sizeof(lastmsg), fmt, ap2); % + va_start(ap, fmt); % + (void) vsnprintf(lastmsg, sizeof(lastmsg), fmt, ap); To make another pass over the args restarting at the start, just start with va_start(). % va_end(ap); % - va_end(ap2); Using va_copy() doesn't even simplify cleaning up, since a va_end() is needed for each va_copy() just like it is needed for each va_start(). % } % > > default implementation: > gcc/ginclude/stdarg.h:#define __va_copy(dest, src) (dest) = (src) > gcc/ginclude/varargs.h:#define __va_copy(dest, src) (dest) = (src) > > platform override for ppc: > gcc/ginclude/va-ppc.h:#define __va_copy(dest, src) *(dest) = *(src) > > The same variation for ppc there would work for amd64 as well. I have > run into quite a number of ports that have this knowledge built in so > that they can compile on ppc with older gcc's without the includes. Apparently va_copy() wasn't much missed, since it wasn't in C90. Did the ports really need va_copy() or just not understand va_start()? Bruce