svn commit: r328430 - head/sbin/devd

[Bruce Evans]

On Thu, 25 Jan 2018, Warner Losh wrote:

> On Thu, Jan 25, 2018 at 9:40 PM, Eitan Adler <eadler at freebsd.org> wrote:
>
>> Author: eadler
>> Date: Fri Jan 26 04:40:41 2018
>> New Revision: 328430
>> URL: https://svnweb.freebsd.org/changeset/base/328430
>>
>> Log:
>>   devd: minor nits
>>
>>   - mark usage as noreturn
>>   - config does not need a virtual destructor
>>
>
> Everything needs a virtual destructor...  Please back that part of this
> out...

Everything here should be backed out.

usage() is static, so it doesn't need any magic declarations even for lint.
Compilers shouldn't warn about it if they do even less checking that lint.
In practice gcc-4 and later compilers default to using -funit-at-a-time
which gives the full file-scope scope analysis needed to obfuscate functions
like usage() by inlining them especially when you intentionally kept them
separate.  Noticing that usage() doesn't return is a trivial part of the
preparation for possibly inlining it.

(The obfuscations from auto-inlining mainly affect debugging including
things like running nm and objdump.  It can be reduced or avoided using
-fno-inline or -fno-inline-functions-called-once or the __noinline
attribute, but -fno-inline is too global,
-fno-inline-functions-called-onces is broken for clang, and adding
__noinline to most static functions is painful.)

style(9) even gives usage() as an example, and of course this is missing
the style bug of declaring it as __dead2.

However, style(9) is not completly trustworthy for usage.  It still uses
a K&R or C++ definition for usage() (missing 'void').  The bug is this
definition, not the correct prototype.

>> Modified: head/sbin/devd/devd.cc
>> ============================================================
>> ==================
>> --- head/sbin/devd/devd.cc      Fri Jan 26 04:32:31 2018        (r328429)
>> +++ head/sbin/devd/devd.cc      Fri Jan 26 04:40:41 2018        (r328430)
>> @@ -161,7 +161,7 @@ static const char *configfile = CF;
>>  static void devdlog(int priority, const char* message, ...)
>>         __printflike(2, 3);
>>  static void event_loop(void);
>> -static void usage(void);
>> +static void usage(void) __dead2;

Since this is C++, it is arguably also a style bug to not be missing 'void'.

It is unarguably a style bug not be missing 'void' in the prototype but to
be missing it in the definition (the combination is the same style bug as
in style(9), made worse since style(9) was correct for K&R but the
difference is just gratuitous for C++).

The style for 'void' is random in this program:
- all prototypes in devd.cc use (void) (not all static functions have forward
   prototypes)
- 4 function definitions in devd.cc use (void)
- 12 function definitions in devd.cc use ()
- 2 (yacc) prototypes in devd.h use (void).  There are no 'extern "C"'
   declarations in the program so I don't know how yacc can work.
- 0 prototypes in devd.hh use (void)
- many prototypes in devd.hh use ().  These mostly match the 12 functions
   in devd.cc.

Bruce