Re: New Defects reported by Coverity Scan for LibreOffice

On Monday 02 of February 2015, Stephan Bergmann wrote:
> On 02/02/2015 10:39 AM, Caolán McNamara wrote:
> > So, if we show coverity the asserts it removes a pile of warnings, but
> > introduces another pile of deadcode given the way we have stacks of
> > defensive "this shouldn't happen, but if it does" code :-) We either
> > ifdef off NDEBUG, just go back to hiding asserts from coverity, or
> > bravely claim that all our assert conditions never happen in release
> > mode.
>
> My take on it is simple:  There /is/ a flaw in the above code, and
> Coverity /does/ correctly identify it.  If the asserted condition cannot
> legitimately be false at that place, the ?: check is wrong and must go
> away.  If it can, the assert is wrong and must go away (or, depending on
> context, be replaced with a SAL_WARN_IF, say).

 That is indeed the theory, but what is the reality? Can somebody with such a 
monstrous codebase say for sure which case it is for every instance of the 
problem? If memory serves me well, we shipped a couple of releases that under 
some circumstances had VCL KFileDialog integration hitting asserts on 
improper locking, but release builds still managed to cope with it somehow 
(more often than not, anyway).

 Developer builds should of course fall flat on their face in such cases, but 
in practice it's probably better to value end product stability more than 
practically insignificant warnings from a tool.

-- 
 Lubos Lunak
 l.lunak@collabora.com