Date: Wed, 5 Nov 2014 03:50:20 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: David Chisnall <theraven@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Stefan Farfeleder <stefanf@freebsd.org>, "Alexander V. Chernikov" <melifaro@freebsd.org>, src-committers@freebsd.org Subject: Re: svn commit: r274086 - head/sbin/route Message-ID: <20141105032132.Y1105@besplex.bde.org> In-Reply-To: <C17BDEA3-7BF7-4EB7-A8E2-A8D202425576@FreeBSD.org> References: <201411041021.sA4ALZ4m001202@svn.freebsd.org> <20141104102828.GB1215@mole.fafoe.narf.at> <C17BDEA3-7BF7-4EB7-A8E2-A8D202425576@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 4 Nov 2014, David Chisnall wrote: > On 4 Nov 2014, at 10:28, Stefan Farfeleder <stefanf@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141105032132.Y1105>