Date: Fri, 26 Jan 2018 21:28:05 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Warner Losh <imp@bsdimp.com> Cc: Eitan Adler <eadler@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r328430 - head/sbin/devd Message-ID: <20180126205058.X1040@besplex.bde.org> In-Reply-To: <CANCZdfosuC0CTig=6_p3D5g0fDPhCVW3%2B4HAEbD4EQ4%2B3TetpA@mail.gmail.com> References: <201801260440.w0Q4efhg008105@repo.freebsd.org> <CANCZdfosuC0CTig=6_p3D5g0fDPhCVW3%2B4HAEbD4EQ4%2B3TetpA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 25 Jan 2018, Warner Losh wrote: > On Thu, Jan 25, 2018 at 9:40 PM, Eitan Adler <eadler@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180126205058.X1040>