coverity#1158232 Fix ownership of NamedDBs::insert argument

f70d03436b7b501e0ed1d745935a204b9b97b855 "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, until
f55cc330dec0dec60c755e2ce28a840c7fca1956 "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

9 files changed, 36 insertions, 8 deletions

diff --git a/sc/inc/dbdata.hxx b/sc/inc/dbdata.hxx
index 78525b78d821..50ec53c486b1 100644
--- a/sc/inc/dbdata.hxx
+++ b/sc/inc/dbdata.hxx
@@ -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;
diff --git a/sc/qa/unit/ucalc_formula.cxx b/sc/qa/unit/ucalc_formula.cxx
index 69fe42051393..3fcc89686fc8 100644
--- a/sc/qa/unit/ucalc_formula.cxx
+++ b/sc/qa/unit/ucalc_formula.cxx
@@ -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[] = {
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 5b23770e4440..c500fa68d031 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -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());
 }
diff --git a/sc/source/core/tool/dbdata.cxx b/sc/source/core/tool/dbdata.cxx
index 62e3a36ccf1b..e8f171fa4ce0 100644
--- a/sc/source/core/tool/dbdata.cxx
+++ b/sc/source/core/tool/dbdata.cxx
@@ -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
diff --git a/sc/source/filter/starcalc/scflt.cxx b/sc/source/filter/starcalc/scflt.cxx
index 88616d3979df..91bcf02c4836 100644
--- a/sc/source/filter/starcalc/scflt.cxx
+++ b/sc/source/filter/starcalc/scflt.cxx
@@ -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)?
     }
 }
 
diff --git a/sc/source/filter/xml/xmldrani.cxx b/sc/source/filter/xml/xmldrani.cxx
index ab7e2a642d54..7cb8a551c671 100644
--- a/sc/source/filter/xml/xmldrani.cxx
+++ b/sc/source/filter/xml/xmldrani.cxx
@@ -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();
+            }
         }
     }
 }
diff --git a/sc/source/ui/dbgui/dbnamdlg.cxx b/sc/source/ui/dbgui/dbnamdlg.cxx
index b1d01b33f484..ba4e781af7e1 100644
--- a/sc/source/ui/dbgui/dbnamdlg.cxx
+++ b/sc/source/ui/dbgui/dbnamdlg.cxx
@@ -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();
diff --git a/sc/source/ui/docshell/dbdocfun.cxx b/sc/source/ui/docshell/dbdocfun.cxx
index 8a590db78f8d..1698b621a1c5 100644
--- a/sc/source/ui/docshell/dbdocfun.cxx
+++ b/sc/source/ui/docshell/dbdocfun.cxx
@@ -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
 
diff --git a/sc/source/ui/docshell/docsh5.cxx b/sc/source/ui/docshell/docsh5.cxx
index 0366dccae1cf..f3c9f39ebd26 100644
--- a/sc/source/ui/docshell/docsh5.cxx
+++ b/sc/source/ui/docshell/docsh5.cxx
@@ -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
             {