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

[Bruce Evans]

On Sat, 15 Mar 2008, Dan Lukes wrote:

> Bruce Evans napsal/wrote, On 03/15/08 06:06:
>> gcc broke its guarantee about "const" stopping this warning, and even
>> more importantly, of the variable not being removed, in gcc-4 or
>> earlier.  The #if 0 in the above was committed on 2003/04/30 with a
> ...
>> Using a volatile qualifier instead of a const qualifier gives inconsistent
>> results.  gcc-4.3 always removes "static char volatile copyright[] but
>> doesn't warn about this removal at WARNS > 1.  Not warning is clearly
>> just a bug.  gcc-3.3.3 always keeps this variable, and warns about it
>> being unused at WARNS > 1.
>
> 	Hm. It seems the GCCs are not consistent between versions. In 
> advance, we didn't speak about intel's icc for now.
>
> 	I found something that make both proposed patches void. There is 
> defined __COPYRIGHT macro in sys/cdef.h exactly just for such purpose. The 
> cdefs.h logic tried to deal with icc/gcc and gcc.x/gcc.y differences.

No, __COPYRIGHT() should never be used.  In FreeBSD, it is even more
broken than "static char const copyright[]", and this combined with
its uglyness has prevented significant use.  It is actually __IDSTRING()
that is broken (__COPYRIGHT() just invokes __IDSTRING() with a fairly
unique name.)  __IDSTRING() is used a lot, e.g., for __FBSDID(), and
its brokenness is mostly hamless except for __COPYRIGHT() since its
main brokenness is for strings with newlines and such strings are only
common for copyright strings.  Bugs in __IDSTRING() include:

For non-gcc non-intel compilers (and maybe once with an ifdef for very old
versions of gcc, but support for very old versions of gcc is just broken
here, while other parts of cdefs.h still have considerable uglyness to
support such versions):
- it uses the old "const" method.  This might still work.
- it uses the __unused qualifier bogusly.  I think this has no effect,
   since __unused is #defined as nothing for non-gcc non-intel compilers.
   It is also illogical, and wouldn't work with gcc now, since gcc quite
   logically removes static unused variables if they are declared as
   __unused (__unused only suppresses the warning).  It should use the
   __used qualifier or just nothing.

For gcc and intel compilers:
- if the string has any \n's in it, then __IDSTRING() reduces to an invalid
   asm directive with the \n's replaced by hard newlines.  The desired
   object code is generated, but the assembler warns once for each hard
   newline and the build fails iff WERROR is enabled.

These bugs have limited __COPYRIGHT() being used in only 7 places in
/usr/src, all in code obtained from other systems where __IDSTRING()
presumably isn't so broken:

./contrib/lukemftp/src/main.c
./contrib/lukemftpd/src/ftpd.c
./lib/libedit/TEST/test.c
./sbin/routed/main.c
./sbin/routed/rtquery/rtquery.c
./usr.bin/nl/nl.c
./usr.bin/gzip/gzip.c

Not even one of these compiles cleanly -- they all have soft newlines in
the sources, and none has been hacked to remove the newlines (but ISTR
some sources being hacked to avoid this bug).  Thus warning(s) are
emitted for all of them.  WARNS is undefined or 0 for most of them, but
even when WARNS is 6 (for gzip), the build doesn't fail, since the
warnings are generated by the assembler and -Werror only applies to the
main compiler pass (another bug).

> 	Even new gcc versions or brand new compiler can be handled in one 
> place.
>
> 	So all the 'copyright' places shall be patched using __COPYRIGHT 
> macro.

Ugh.

To use __IDSTRING() on strings with \n's in them, one ugly fix is to quote
the quotes (\n -> \\n).  Doing this at the source level would break the
non-gcc non-intel case.

__IDSTRING normally uses .ident.  A more portable fix would be to
always use "static const char __used UNIQUIZE(copyright)[]".  This
would lose putting of the string in a non-loaded section.  Not too bad
for just copyrights.  At the source level, removing static declarations
is not too bad.

There are also the copyright[] declarations and COPYRIGHT_* macros in
<sys/copyright.h>.  These know better than to use __COPYRIGHT().  They
avoid the portability problems by just not declaring copyright[] as
static (or even as const).

Bruce