svn commit: r250037 - head/bin/hostname

Bruce Evans brde at optusnet.com.au
Mon Apr 29 13:10:43 UTC 2013


On Sun, 28 Apr 2013, Eitan Adler wrote:

> Log:
>  Mark usage() __dead2

This just adds a style bug.

> Modified: head/bin/hostname/hostname.c
> ==============================================================================
> --- head/bin/hostname/hostname.c	Sun Apr 28 22:12:40 2013	(r250036)
> +++ head/bin/hostname/hostname.c	Sun Apr 28 22:52:43 2013	(r250037)
> @@ -49,7 +49,7 @@ __FBSDID("$FreeBSD$");
> #include <string.h>
> #include <unistd.h>
>
> -static void usage(void);
> +static void usage(void) __dead2;

Static functions should never be marked __dead2.  The purpose of the
markup is to inform the compiler that the function doesn't return.
But for static functions, the compiler can see this for itself, at
least with -funit-at-a-time in CFLAGS.  (-funit-at-a-time is not
required (doesn't exist) in C standards, but is now the default.
Compilers even use it to look to closely at static functions and
automatically inline them even when they are declared after they are
used, as is common for usage(), provided they are only used once and
-fno-inline-functions-called-once is not used.  This mainly breaks
debugging and profiling of non-inline functions.  The breakage is
largest for clang, collaterally with the bug that clang doesn't support
either of these flags, so it seems to have no way to turn off this
excessive inlining.)

The style bug is missing in the example of using usage() in style(9).

This style bug is missing in about 259 out of 284 declarations of static
void usage(void) in /usr/src/*bin (.c files).  Except:
- bin/pkill/pkill.c has it in a worse form, with __dead2 hard-coded
   using __attribute(())
- usr.bin/rlogin/rlogin.c has it in a gratuitously different form, as
   'static _Noreturn void	usage(void);'.  This is bogus since
   _Noreturn is a wrapper for a new C++ feature, but rlogin is an old C
   application; for C, it is defined as __dead2, but it isn't clear that
   it does the same as a normal use of __dead2 since it is used in a
   syntactically different way.  __dead2 only exist because this syntax,
   which was used for the old __dead (== volatile return type instead of
   __attribute((__noreturn__))) didn't work.  Apparently the syntax of
   __attribute__() has become less strict, so the more natural (old)
   syntax works again, and the use of the old syntax here is just a
   style bug and not a syntax error.
I counted these style bugs by grepping for "static.*usage" (318 lines)
and then filtering out most that didn't match the normal usage of
usage().  There are too many gratuitous differences like taking a 'char
*' arg or returning an int.  Another popular style bug is a gnu-style
declaration with a space after the function name.  One declaration is
even missing the arg type, so it isn't a prototype.

Not all instances of the __dead[2] style bug are new.  In 4.4BSD-Lite2,
there were 17 instances of __dead in /usr/src/*bin.  Only 4 of these
were on static functions.  Only 2 of them were on usage(), and usage()
wasn't declared static in these 2.  These 2 were rlogin and mount_nfs.
The bug in rlogin persists in a worse form, and the bug in mount_nfs
was cloned to many other mount utilities.

In FreeBSD-~5.2, there were 43 instances of __dead2 in /usr/src/*bin.
30 of these were on static functions.  22 of these were on usage().
Almost half of these 22 were in mount utilities.

Further statistics for -current in /usr/src/*bin:
- 60 instances of __dead2
- 57 of these on static functions
- at least 26 of these on static.*usage.*
- some false/misleading hits for the sloppy patterns:
   - 1 nonsense declaration in echo/echo.c.  __dead2 is placed on the
     function definition of errexit(), where it ican have no effect except
     possibly to cause a warning when the function does return.  The function
     actually ends with exit(), so the compiler can see that it doesn't
     return.  There is no forward prototype for this function, but it is
     placed before it is used, so the compiler can see that it doesn't
     return even without -funit-at-a-time.
   - same nonsense declaration in sh/arith_yacc.c:yyerror().  It returns
     with error() instad of exit(), so its non-returningness is given by
     __dead2 on error().
   - same nonsense declaration in killall/killall.c:usage().  usage() is
     unsorted at the beginning of the file, so it doesn't need the usual
     forward prototype, but it has the __dead2 declaration which is not
     even needed for the prototype.
   - i2c/i2c.c:usage().  Like killall, except the unsorting is not so
     close to the beginning of the file.
   - gbde/gbde.c:usage().  Like i2c.
   - hastd/primary.c.  Like sh (3 functions).
   - hastctl/hastctl.c.  Like i2c.
   These false hits were only harder to understand because the function
   definitions are formatted normally, so the function name isn't on the
   same line as __dead2 so grepping with simple patterns doesn't show what
   it is.  The syntax for all the nonsense declarations is
   'static __dead2 <type>\n<function name>'.  This would have been a
   syntax error for the original implementation of __dead2.  __dead2 is
   named as it is (with '2' meaning gcc version2 instead of 1) because
   simply changing the definition of __dead wouldn't have worked due to
   syntactical problems.

The above statistics are all for .c files.  __dead2 is used a further 28
times in .h files in /usr/src/*bin in -current.  Now all the uses are
hopefully useful.  But there aren't enough of them to make much difference.
System headers are hopefully more careful about this.

Bruce


More information about the svn-src-all mailing list