On 02/02/2015 10:39 AM, Caolán McNamara wrote:
On Fri, 2015-01-30 at 10:47 -0800, scan-ad...@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.")
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
