coverity#1158232 Fix ownership of NamedDBs::insert argument
f70d03436b
"coverity#1158232 have a stab at silencing warning with function markup" claimed that NamedDBs::insert always takes ownerhip of its argument, but boost::ptr_set::insert(std::auto_ptr<U> x) simply calls insert(x.release()), so only takes ownership when it returns true. ScDBDocFunc::AddDBRange (sc/source/ui/docshell/dbdocfun.cxx) relies on this behavior, deleting the argument when insert failed. ScDBDocFunc::RenameDBRange (sc/source/ui/docshell/dbdocfun.cxx) relied on this behavior, deleting the argument when insert failed, untilf55cc330de
"Fixed the fallout of the changes in ScDBCollection" removed the delete (presumably in error?). I put it back in now. All other uses of NamedDBs::insert ignored the return value (witnessed with SAL_WARN_UNUSED_RESULT). Some are insert-if-not-found cases, where I added asserts now (Sc10Import::LoadDataBaseCollection, sc/source/filter/starcalc/scflt.cxx, is not entirely clear to me, so I added a TODO), while others would have potentially leaked the argument, in which cases I fixed the code. Change-Id: Iad40fbeb625c8ce6b0a61cbf16298d71cdc7de80
This commit is contained in:
@@ -177,7 +177,8 @@ public:
|
||||
const_iterator end() const;
|
||||
ScDBData* findByIndex(sal_uInt16 nIndex);
|
||||
ScDBData* findByUpperName(const OUString& rName);
|
||||
bool insert(ScDBData* p);
|
||||
// Takes ownership of p iff it returns true:
|
||||
SAL_WARN_UNUSED_RESULT bool insert(ScDBData* p);
|
||||
void erase(iterator itr);
|
||||
void erase(const ScDBData& r);
|
||||
bool empty() const;
|
||||
|
@@ -145,7 +145,11 @@ void Test::testFormulaCreateStringFromTokens()
|
||||
ScDBData* pData = new ScDBData(
|
||||
OUString::createFromAscii(
|
||||
aDBs[i].pName), aDBs[i].nTab, aDBs[i].nCol1, aDBs[i].nRow1, aDBs[i].nCol2,aDBs[i].nRow2);
|
||||
pDBs->getNamedDBs().insert(pData);
|
||||
bool bInserted = pDBs->getNamedDBs().insert(pData);
|
||||
CPPUNIT_ASSERT_MESSAGE(
|
||||
OString(
|
||||
"Failed to insert \"" + OString(aDBs[i].pName) + "\"").getStr(),
|
||||
bInserted);
|
||||
}
|
||||
|
||||
const char* aTests[] = {
|
||||
|
@@ -17,6 +17,10 @@
|
||||
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
|
||||
*/
|
||||
|
||||
#include <sal/config.h>
|
||||
|
||||
#include <cassert>
|
||||
|
||||
#include "formulacell.hxx"
|
||||
#include "grouptokenconverter.hxx"
|
||||
|
||||
@@ -379,7 +383,8 @@ void adjustDBRange(ScToken* pToken, ScDocument& rNewDoc, const ScDocument* pOldD
|
||||
if (!pNewDBData)
|
||||
{
|
||||
pNewDBData = new ScDBData(*pDBData);
|
||||
aNewNamedDBs.insert(pNewDBData);
|
||||
bool ins = aNewNamedDBs.insert(pNewDBData);
|
||||
assert(ins); (void)ins;
|
||||
}
|
||||
pToken->SetIndex(pNewDBData->GetIndex());
|
||||
}
|
||||
|
@@ -683,7 +683,6 @@ ScDBData* ScDBCollection::NamedDBs::findByUpperName(const OUString& rName)
|
||||
return itr == maDBs.end() ? NULL : &(*itr);
|
||||
}
|
||||
|
||||
// coverity[+free : arg-0]
|
||||
bool ScDBCollection::NamedDBs::insert(ScDBData* p)
|
||||
{
|
||||
SAL_WNODEPRECATED_DECLARATIONS_PUSH
|
||||
|
@@ -40,6 +40,7 @@
|
||||
#include <editeng/justifyitem.hxx>
|
||||
#include <svl/zforlist.hxx>
|
||||
#include <svl/PasswordHelper.hxx>
|
||||
#include <cassert>
|
||||
#include <stdio.h>
|
||||
#include <math.h>
|
||||
#include <string.h>
|
||||
@@ -1394,7 +1395,9 @@ void Sc10Import::LoadDataBaseCollection()
|
||||
( SCROW ) pOldData->DataBaseRec.Block.y2,
|
||||
true,
|
||||
( sal_Bool) pOldData->DataBaseRec.RowHeader );
|
||||
pDoc->GetDBCollection()->getNamedDBs().insert(pNewData);
|
||||
bool ins = pDoc->GetDBCollection()->getNamedDBs().insert(pNewData);
|
||||
assert(ins); (void)ins;
|
||||
//TODO: or can this fail (and need delete pNewData)?
|
||||
}
|
||||
}
|
||||
|
||||
|
@@ -491,7 +491,10 @@ void ScXMLDatabaseRangeContext::EndElement()
|
||||
if (pData.get())
|
||||
{
|
||||
setAutoFilterFlags(*pDoc, *pData);
|
||||
pDoc->GetDBCollection()->getNamedDBs().insert(pData.release());
|
||||
if (pDoc->GetDBCollection()->getNamedDBs().insert(pData.get()))
|
||||
{
|
||||
pData.release();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@@ -17,6 +17,10 @@
|
||||
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
|
||||
*/
|
||||
|
||||
#include <sal/config.h>
|
||||
|
||||
#include <cassert>
|
||||
|
||||
#include <comphelper/string.hxx>
|
||||
#include <vcl/msgbox.hxx>
|
||||
|
||||
@@ -469,7 +473,8 @@ IMPL_LINK_NOARG(ScDbNameDlg, AddBtnHdl)
|
||||
pNewEntry->SetKeepFmt( m_pBtnKeepFmt->IsChecked() );
|
||||
pNewEntry->SetStripData( m_pBtnStripData->IsChecked() );
|
||||
|
||||
aLocalDbCol.getNamedDBs().insert(pNewEntry);
|
||||
bool ins = aLocalDbCol.getNamedDBs().insert(pNewEntry);
|
||||
assert(ins); (void)ins;
|
||||
}
|
||||
|
||||
UpdateNames();
|
||||
|
@@ -167,7 +167,10 @@ bool ScDBDocFunc::RenameDBRange( const OUString& rOld, const OUString& rNew )
|
||||
rDBs.erase(*pOld);
|
||||
bool bInserted = rDBs.insert(pNewData);
|
||||
if (!bInserted) // Fehler -> alten Zustand wiederherstellen
|
||||
{
|
||||
delete pNewData;
|
||||
pDoc->SetDBCollection(pUndoColl); // gehoert dann dem Dokument
|
||||
}
|
||||
|
||||
pDoc->CompileDBFormula( false ); // CompileFormulaString
|
||||
|
||||
|
@@ -17,6 +17,10 @@
|
||||
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
|
||||
*/
|
||||
|
||||
#include <sal/config.h>
|
||||
|
||||
#include <cassert>
|
||||
|
||||
#include "scitems.hxx"
|
||||
#include <vcl/msgbox.hxx>
|
||||
#include <vcl/waitobj.hxx>
|
||||
@@ -274,7 +278,8 @@ ScDBData* ScDocShell::GetDBData( const ScRange& rMarked, ScGetDBMode eMode, ScGe
|
||||
pNoNameData = new ScDBData( aNewName, nTab,
|
||||
nStartCol,nStartRow, nEndCol,nEndRow,
|
||||
true, bHasHeader );
|
||||
rDBs.insert(pNoNameData);
|
||||
bool ins = rDBs.insert(pNoNameData);
|
||||
assert(ins); (void)ins;
|
||||
}
|
||||
else
|
||||
{
|
||||
|
Reference in New Issue
Block a user