bin/71613: [PATCH] traceroute(8): cleanup of the usr.sbin/traceroute6 code

Bruce Evans brde at optusnet.com.au
Sat Mar 15 17:12:47 UTC 2008


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


More information about the freebsd-bugs mailing list