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