From owner-freebsd-bugs@FreeBSD.ORG Sat Mar 15 17:12:47 2008 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2566A1065674; Sat, 15 Mar 2008 17:12:47 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail07.syd.optusnet.com.au (mail07.syd.optusnet.com.au [211.29.132.188]) by mx1.freebsd.org (Postfix) with ESMTP id C26818FC15; Sat, 15 Mar 2008 17:12:46 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c220-239-252-11.carlnfd3.nsw.optusnet.com.au (c220-239-252-11.carlnfd3.nsw.optusnet.com.au [220.239.252.11]) by mail07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id m2FHCfUL032385 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 16 Mar 2008 04:12:44 +1100 Date: Sun, 16 Mar 2008 04:12:41 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Dan Lukes In-Reply-To: <47DBC810.3030903@obluda.cz> Message-ID: <20080316035724.V40050@delplex.bde.org> References: <200803150200.m2F206rl078586@freefall.freebsd.org> <20080315151246.L35251@besplex.bde.org> <47DB7DA8.7050400@obluda.cz> <20080315192732.A38703@delplex.bde.org> <47DBC810.3030903@obluda.cz> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-bugs@freebsd.org, imp@freebsd.org, Bruce Evans Subject: Re: bin/71613: [PATCH] traceroute(8): cleanup of the usr.sbin/traceroute6 code X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Mar 2008 17:12:47 -0000 On Sat, 15 Mar 2008, Dan Lukes wrote: > OK > > There seems to be several ways to correct the problem. > > We can avoid the warning by adding "const" or "volatile" or __used or > by removing "static". > > The "remove static" way can't be used - non-static symbol is global > variable - so const char copyright[] from one source will collide with the > same name variable from another source. It's not problem only when there are > one source file only or we can guarantee unique variable name. __COPYRIGHT() in reduces this problem by concatenating __LINE__. It could also concatenate a file name (but not __FILE__, since that is probably not an identifier). > It seems we need 'static'. Unfortunately, static unused variable can > be optimized out. > > Adding 'const' and/or __used clear the warning, but doesn't prevent > "optimized-out" problem. The 'const' shall be used because the string is > constant. We can use __used, but it has limited portability. > > We still have the problem the variable may be optimized out. __used prevents this. > The 'volatile' is way to tell an ANSI C compiler "this variable may > be modified via mechanism you don't know about it" - it mean "count it as > used" and "don't optimize it". Note, the 'const' and 'static' are ANSI C > keywords also, so compiler knowing 'static' shall handle 'volatile' as well. volatile doesn't prevent the variable being optimized out for gcc-4.2. This makes some sense -- volatile sort of means "use it carefully", but when it is not used no care with it is needed. > Conclusion (for the case we can't guarantee the unique name of > variable): > static MUST > const SHALL > __used SHALL > volatile MUST > > So my recomentation is: > 0: static volatile const char __used copyright[]=... > > because of __unused the sys/cdefs.h must be included first. I prefer 'static char const __used copyright[]'. Not sure where __used belongs. > The other way to fix it is > 1: the sys/copyright.h way - e.g. plain char variable - but the variable must > be unique across the sources which sound not so easy for me Not too bad -- there is supposed to be only one copyright[] per executable. > 2. the __COPYRIGHT way, but > 2a: IDSTRING must be corrected first > 2b: the '\n' must be removed from the source. > > sys/cdefs.h must be included first. > > In my opinion the preference shall be 2a then 0 then 1 or 2b but it's > not strict. The commiter shall select the best way. 2b is no good. __COPYRIGHT() of course allows putting all the unportabilities in one place and changing the easily. It's just too ugly for me. Everything in should have gone away with __P(()). Bruce