Date: Sun, 16 Mar 2008 04:12:41 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Dan Lukes <dan@obluda.cz> Cc: freebsd-bugs@freebsd.org, imp@freebsd.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: bin/71613: [PATCH] traceroute(8): cleanup of the usr.sbin/traceroute6 code Message-ID: <20080316035724.V40050@delplex.bde.org> In-Reply-To: <47DBC810.3030903@obluda.cz> References: <200803150200.m2F206rl078586@freefall.freebsd.org> <20080315151246.L35251@besplex.bde.org> <47DB7DA8.7050400@obluda.cz> <20080315192732.A38703@delplex.bde.org> <47DBC810.3030903@obluda.cz>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <sys/cdefs.h> 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 <sys/cdefs.h> should have gone away with __P(()). Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080316035724.V40050>