tdf#116977 secured ::Clone methods

Renamed SdrPage::Clone -> SdrPage::CloneSdrPage
Renamed SdrObject::Clone -> SdrObject::CloneSdrObject

Giving SdrModel is no longer an option, but a must (as
reference). This makes future changes more safe by force
usage to think about it. Also equals the constructors
which already require a target SdrModel.
Done the same for ::CloneSdrPage.

Change-Id: I06f0129e15140bd8693db27a445037d7e2f7f652
Reviewed-on: https://gerrit.libreoffice.org/53933
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Armin Le Grand <Armin.Le.Grand@cib.de>

7 files changed, 32 insertions, 18 deletions

diff --git a/reportdesign/inc/RptObject.hxx b/reportdesign/inc/RptObject.hxx
index 17f433629c56..b078e1f7d143 100644
--- a/reportdesign/inc/RptObject.hxx
+++ b/reportdesign/inc/RptObject.hxx
@@ -124,6 +124,11 @@ class REPORTDESIGN_DLLPUBLIC OCustomShape final : public SdrObjCustomShape , pub
 {
     friend class OReportPage;
     friend class DlgEdFactory;
+
+private:
+    // protected destructor - due to final, make private
+    virtual ~OCustomShape() override;
+
 public:
     static OCustomShape* Create(
         SdrModel& rSdrModel,
@@ -132,8 +137,6 @@ public:
         return new OCustomShape(rSdrModel, _xComponent );
     }
 
-    virtual ~OCustomShape() override;
-
     virtual css::uno::Reference< css::beans::XPropertySet> getAwtComponent() override;
 
     virtual css::uno::Reference< css::uno::XInterface > getUnoShape() override;
@@ -165,6 +168,11 @@ class REPORTDESIGN_DLLPUBLIC OOle2Obj final : public SdrOle2Obj , public OObject
 {
     friend class OReportPage;
     friend class DlgEdFactory;
+
+private:
+    // protected destructor - due to final, make private
+    virtual ~OOle2Obj() override;
+
 public:
     static OOle2Obj* Create(
         SdrModel& rSdrModel,
@@ -174,15 +182,13 @@ public:
         return new OOle2Obj(rSdrModel, _xComponent, _nType);
     }
 
-    virtual ~OOle2Obj() override;
-
     virtual css::uno::Reference< css::beans::XPropertySet> getAwtComponent() override;
 
     virtual css::uno::Reference< css::uno::XInterface > getUnoShape() override;
     virtual sal_uInt16 GetObjIdentifier() const override;
     virtual SdrInventor GetObjInventor() const override;
     // Clone() should make a complete copy of the object.
-    virtual OOle2Obj* Clone(SdrModel* pTargetModel = nullptr) const override;
+    virtual OOle2Obj* CloneSdrObject(SdrModel& rTargetModel) const override;
     virtual void initializeOle() override;
 
     OOle2Obj& operator=(const OOle2Obj& rObj);
@@ -234,6 +240,7 @@ protected:
         const OUString& rModelName,
         sal_uInt16 _nObjectType);
 
+    // protected destructor
     virtual ~OUnoObject() override;
 
     virtual void NbcMove( const Size& rSize ) override;
@@ -259,7 +266,7 @@ public:
     virtual css::uno::Reference< css::uno::XInterface > getUnoShape() override;
     virtual sal_uInt16 GetObjIdentifier() const override;
     virtual SdrInventor GetObjInventor() const override;
-    virtual OUnoObject* Clone(SdrModel* pTargetModel = nullptr) const override;
+    virtual OUnoObject* CloneSdrObject(SdrModel& rTargetModel) const override;
 
     OUnoObject& operator=(const OUnoObject& rObj);
 
diff --git a/reportdesign/inc/RptPage.hxx b/reportdesign/inc/RptPage.hxx
index 96fb26a84a67..80d65c6777f6 100644
--- a/reportdesign/inc/RptPage.hxx
+++ b/reportdesign/inc/RptPage.hxx
@@ -55,7 +55,7 @@ public:
     OReportPage( OReportModel& rModel
                 ,const css::uno::Reference< css::report::XSection >& _xSection );
 
-    virtual SdrPage* Clone(SdrModel* pNewModel = nullptr) const override;
+    virtual SdrPage* CloneSdrPage(SdrModel& rTargetModel) const override;
 
     virtual void NbcInsertObject(SdrObject* pObj, size_t nPos=SAL_MAX_SIZE) override;
     virtual SdrObject* RemoveObject(size_t nObjNum) override;
diff --git a/reportdesign/source/core/api/Shape.cxx b/reportdesign/source/core/api/Shape.cxx
index 671bccaa2f32..c3c766df4892 100644
--- a/reportdesign/source/core/api/Shape.cxx
+++ b/reportdesign/source/core/api/Shape.cxx
@@ -313,7 +313,7 @@ uno::Reference< util::XCloneable > SAL_CALL OShape::createClone(  )
             SdrObject* pObject = pShape->GetSdrObject();
             if ( pObject )
             {
-                SdrObject* pClone = pObject->Clone();
+                SdrObject* pClone(pObject->CloneSdrObject(pObject->getSdrModelFromSdrObject()));
                 if ( pClone )
                 {
                     xSet.set(pClone->getUnoShape(),uno::UNO_QUERY_THROW );
diff --git a/reportdesign/source/core/sdr/RptObject.cxx b/reportdesign/source/core/sdr/RptObject.cxx
index 8d38b5ddf478..85a50a242c58 100644
--- a/reportdesign/source/core/sdr/RptObject.cxx
+++ b/reportdesign/source/core/sdr/RptObject.cxx
@@ -883,9 +883,9 @@ void OUnoObject::impl_setUnoShape( const uno::Reference< uno::XInterface >& rxUn
     releaseUnoShape();
 }
 
-OUnoObject* OUnoObject::Clone(SdrModel* pTargetModel) const
+OUnoObject* OUnoObject::CloneSdrObject(SdrModel& rTargetModel) const
 {
-    return CloneHelper< OUnoObject >(pTargetModel);
+    return CloneHelper< OUnoObject >(rTargetModel);
 }
 
 OUnoObject& OUnoObject::operator=(const OUnoObject& rObj)
@@ -1089,9 +1089,9 @@ uno::Reference< chart2::data::XDatabaseDataProvider > lcl_getDataProvider(const
 }
 
 // Clone() should make a complete copy of the object.
-OOle2Obj* OOle2Obj::Clone(SdrModel* pTargetModel) const
+OOle2Obj* OOle2Obj::CloneSdrObject(SdrModel& rTargetModel) const
 {
-    return CloneHelper< OOle2Obj >(pTargetModel);
+    return CloneHelper< OOle2Obj >(rTargetModel);
 }
 
 OOle2Obj& OOle2Obj::operator=(const OOle2Obj& rObj)
diff --git a/reportdesign/source/core/sdr/RptPage.cxx b/reportdesign/source/core/sdr/RptPage.cxx
index b4e09805f04c..e19c1f725a31 100644
--- a/reportdesign/source/core/sdr/RptPage.cxx
+++ b/reportdesign/source/core/sdr/RptPage.cxx
@@ -42,9 +42,9 @@ OReportPage::~OReportPage()
 {
 }
 
-SdrPage* OReportPage::Clone(SdrModel* pNewModel) const
+SdrPage* OReportPage::CloneSdrPage(SdrModel& rTargetModel) const
 {
-    OReportModel& rOReportModel(static_cast< OReportModel& >(nullptr == pNewModel ? getSdrModelFromSdrPage() : *pNewModel));
+    OReportModel& rOReportModel(static_cast< OReportModel& >(rTargetModel));
     OReportPage* pClonedOReportPage(
         new OReportPage(
             rOReportModel,
diff --git a/reportdesign/source/ui/report/ReportController.cxx b/reportdesign/source/ui/report/ReportController.cxx
index 84a8569b430b..b981e1bba458 100644
--- a/reportdesign/source/ui/report/ReportController.cxx
+++ b/reportdesign/source/ui/report/ReportController.cxx
@@ -3144,7 +3144,10 @@ void OReportController::createControl(const Sequence< PropertyValue >& _aArgs,co
                             ,nullptr,nullptr,_nObjectId,SdrInventor::ReportDesign,OBJ_DLG_FIXEDTEXT,
                          nullptr,pSectionWindow->getReportSection().getPage(),m_aReportModel.get(),
                          pLabel,pControl);
-        delete pLabel;
+
+        // always use SdrObject::Free(...) for SdrObjects (!)
+        SdrObject* pTemp(pLabel);
+        SdrObject::Free(pTemp);
 
         pNewControl = pControl;
         OUnoObject* pObj = dynamic_cast<OUnoObject*>(pControl);
@@ -3576,7 +3579,11 @@ void OReportController::addPairControls(const Sequence< PropertyValue >& aArgs)
             else
             {
                 for(SdrUnoObj* i : pControl)
-                    delete i;
+                {
+                    // always use SdrObject::Free(...) for SdrObjects (!)
+                    SdrObject* pTemp(i);
+                    SdrObject::Free(pTemp);
+                }
             }
         }
     }
diff --git a/reportdesign/source/ui/report/ReportSection.cxx b/reportdesign/source/ui/report/ReportSection.cxx
index 3acc54f2bb4a..62c9275ef15d 100644
--- a/reportdesign/source/ui/report/ReportSection.cxx
+++ b/reportdesign/source/ui/report/ReportSection.cxx
@@ -265,7 +265,7 @@ void OReportSection::Paste(const uno::Sequence< beans::NamedValue >& _aAllreadyC
                         if ( pObject )
                         {
                             // Clone to target SdrModel
-                            SdrObject* pNewObj(pObject->Clone(m_pModel.get()));
+                            SdrObject* pNewObj(pObject->CloneSdrObject(*m_pModel.get()));
 
                             pNewObj->SetPage( m_pPage );
                             m_pPage->InsertObject(pNewObj, SAL_MAX_SIZE);
@@ -357,7 +357,7 @@ void OReportSection::Copy(uno::Sequence< beans::NamedValue >& _rAllreadyCopiedOb
         {
             try
             {
-                SdrObject* pNewObj = pSdrObject->Clone();
+                SdrObject* pNewObj(pSdrObject->CloneSdrObject(pSdrObject->getSdrModelFromSdrObject()));
                 aCopies.emplace_back(pNewObj->getUnoShape(),uno::UNO_QUERY);
                 if ( _bEraseAnddNoClone )
                 {