svn commit: r274086 - head/sbin/route

[Bruce Evans]

On Tue, 4 Nov 2014, David Chisnall wrote:

> On 4 Nov 2014, at 10:28, Stefan Farfeleder <stefanf at FreeBSD.org> wrote:
>
>> Shouldn't Coverity understand that err doesn't return?
>
> err() is marked as __dead2, which expands to __attribute__((__noreturn__)).  If Coverity doesn't know that __attribute__((__noreturn__)) functions don't return, then that's a Coverity bug and they should fix it (if we're not expanding __dead3 to __attribute__((__noreturn__)) for Coverity, then that's a sys/cdefs.h bug and should be fixed there).

__dead3 would be the gcc-3 syntax for __dead2 if that were different.  Since
gcc only changed the syntax for non-returning functions once, __dead3 doesn't
exist.  You probably mean __dead2.

<sys/cdefs.h> is indeed broken for lint and some other cases.  It defines
__dead2 as nothing for lint.  This shouldn't be a problem for primitive
lints since __dead2 is only a hint (not so for some other things that
are defined away), but it prevents any line that supports gcc extensions
from seeing the defined-away attributes.

__dead2 is also defined away unless the compiler is gcc >= 2.5 or any
__INTEL_COMPILER (do any __INTEL_COMPILERs still exist?).  Coverity
would have to pretend to be gcc >= 2.5 to see the gcc attributes.  clang
pretends this, but INTEL_COMPILER doesn't.

Some other attributes are ifdefed more orthogonally but still messily
using macros like __CC_SUPPORTS_INLINE.  You still need a mess of ifdefs
to determine if the compiler supports the feature.  The mess is especially
ugly though not very large for 'inline'.  There are similar macros for
__inline and __inline__.  Plain inline has only been standard for 15 years
now.  __inline is gcc's 20+ year old workaround for inline not being
standard.  __inline__ is an alternative spelling of this.  Its use is
just a style bug.  All of these are assumed to exist if the compiler is
gcc or __INTEL_COMPILER.  The ifdefs are not messy enough to be correct
even for gcc, since 25+ year old gcc didn't support inlining.  Only a
few places actually uses the __CC_SUPPORTS_*INLINE feature tests, so these
are worse than useless.  Many places use __inline instead of inline, so
they don't depend on the compiler supporting C99 inline or being gcc with
support for inline not killed using -std=c89, etc.

> Putting a break after a noreturn function makes the code less readable and will cause errors in non-buggy static analysers (dead code warning - why do you have a break on an unreachable line?).

Similarly for lint comments like /* NOTREACHED */.  Even lint shouldn't need
help to know that standard functions like exit() don't return.

Bruce