Re: New Defects reported by Coverity Scan for LibreOffice

On 02/02/2015 10:39 AM, Caolán McNamara wrote:
> On Fri, 2015-01-30 at 10:47 -0800, scan-admin@coverity.com wrote:
> > ________________________________________________________________________________________________________
> > *** CID 1267636:  Logically dead code  (DEADCODE)
> > /sw/source/core/doc/textboxhelper.cxx: 242 in SwTextBoxHelper::getByIndex(SdrPage *, int, std::set<const SwFrmFmt *, 
> > std::less<const SwFrmFmt *>, std::allocator<const SwFrmFmt *>> &)()
> > 236                 pRet = pPage->GetObj(i);
> > 237                 break;
> > 238             }
> > 239             ++nCount;
> > 240         }
> > 241         assert(pRet);
> > > > >      CID 1267636:  Logically dead code  (DEADCODE)
> > > > >      Execution cannot reach this statement: "<temporary>.Any();".
> > 242         return pRet ? uno::makeAny(uno::Reference<drawing::XShape>(pRet->getUnoShape(), 
> > uno::UNO_QUERY)) : uno::Any();
> > 243     }
> > 244
> > 245     sal_Int32 SwTextBoxHelper::getOrdNum(const SdrObject* pObject, std::set<const SwFrmFmt*>& 
> > rTextBoxes)
> > 246     {
> > 247         if (const SdrPage* pPage = pObject->GetPage())
>
> 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).

(And this particular check-after-assert was apparently introduced to 
work around a false positive from an NDEBUG Coverity build, 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=4da7559ab335d342ebc5b5f06539b5fc4ea4a3cb> 
"coverity#1219787 Explicit null dereferenced.")