From c0ee80f9787bc5e86fb7a342b567c0649253e1e3 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Fri, 18 May 2018 15:58:55 +0200 Subject: loplugin:useuniqueptr in SdrDragView and fix potential leak on early return in SdrDragView::BegDragObj Change-Id: I707be6e2c7dc2c251f37447fe3cd98c4b50b59d1 Reviewed-on: https://gerrit.libreoffice.org/53751 Tested-by: Jenkins Reviewed-by: Noel Grandin --- include/svx/svddrgv.hxx | 5 +-- svx/source/svdraw/svddrgv.cxx | 84 +++++++++++++++++++++---------------------- svx/source/svdraw/svdview.cxx | 2 +- 3 files changed, 45 insertions(+), 46 deletions(-) diff --git a/include/svx/svddrgv.hxx b/include/svx/svddrgv.hxx index 180d8cc09269..5fbb65c9dee7 100644 --- a/include/svx/svddrgv.hxx +++ b/include/svx/svddrgv.hxx @@ -22,6 +22,7 @@ #include #include +#include class SdrUndoGeoObj; @@ -38,7 +39,7 @@ class SVX_DLLPUBLIC SdrDragView : public SdrExchangeView protected: SdrHdl* mpDragHdl; - SdrDragMethod* mpCurrentSdrDragMethod; + std::unique_ptr mpCurrentSdrDragMethod; SdrUndoGeoObj* mpInsPointUndo; tools::Rectangle maDragLimit; OUString maInsPointUndoStr; @@ -104,7 +105,7 @@ public: void BrkDragObj(); bool IsDragObj() const { return mpCurrentSdrDragMethod && !mbInsPolyPoint && !mbInsGluePoint; } SdrHdl* GetDragHdl() const { return mpDragHdl; } - SdrDragMethod* GetDragMethod() const { return mpCurrentSdrDragMethod; } + SdrDragMethod* GetDragMethod() const { return mpCurrentSdrDragMethod.get(); } bool IsDraggingPoints() const { return meDragHdl==SdrHdlKind::Poly; } bool IsDraggingGluePoints() const { return meDragHdl==SdrHdlKind::Glue; } diff --git a/svx/source/svdraw/svddrgv.cxx b/svx/source/svdraw/svddrgv.cxx index 73ff381b877c..55a5b8f36478 100644 --- a/svx/source/svdraw/svddrgv.cxx +++ b/svx/source/svdraw/svddrgv.cxx @@ -161,14 +161,14 @@ bool SdrDragView::TakeDragObjAnchorPos(Point& rPos, bool bTR ) const rPos = bTR ? aR.TopRight() : aR.TopLeft(); if (GetMarkedObjectCount()==1 && IsDragObj() && // only on single selection !IsDraggingPoints() && !IsDraggingGluePoints() && // not when moving points - dynamic_cast( mpCurrentSdrDragMethod) == nullptr) // not when moving handles + dynamic_cast( mpCurrentSdrDragMethod.get() ) == nullptr) // not when moving handles { SdrObject* pObj=GetMarkedObjectByIndex(0); if (dynamic_cast( pObj) != nullptr) { Point aPt(static_cast(pObj)->GetTailPos()); bool bTail=meDragHdl==SdrHdlKind::Poly; // drag tail - bool bOwn=dynamic_cast( mpCurrentSdrDragMethod) != nullptr; // specific to object + bool bOwn=dynamic_cast( mpCurrentSdrDragMethod.get() ) != nullptr; // specific to object if (!bTail) { // for bTail, TakeActionRect already does the right thing if (bOwn) @@ -195,10 +195,13 @@ bool SdrDragView::TakeDragLimit(SdrDragMode /*eMode*/, tools::Rectangle& /*rRect return false; } -bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl, short nMinMov, SdrDragMethod* pForcedMeth) +bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl, short nMinMov, SdrDragMethod* _pForcedMeth) { BrkAction(); + // so we don't leak the object on early return + std::unique_ptr pForcedMeth(_pForcedMeth); + bool bRet=false; { SetDragWithCopy(false); @@ -250,7 +253,7 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl } else if(mbDragHdl) { - mpCurrentSdrDragMethod = new SdrDragMovHdl(*this); + mpCurrentSdrDragMethod.reset(new SdrDragMovHdl(*this)); } else if(!bNotDraggable) { @@ -275,7 +278,7 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl // because 3D objects are limited rotations if (!b3DObjSelected && !IsShearAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragShear(*this,meDragMode==SdrDragMode::Rotate); + mpCurrentSdrDragMethod.reset(new SdrDragShear(*this,meDragMode==SdrDragMode::Rotate)); } break; case SdrHdlKind::UpperLeft: case SdrHdlKind::UpperRight: case SdrHdlKind::LowerLeft: case SdrHdlKind::LowerRight: @@ -283,12 +286,12 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl if (meDragMode==SdrDragMode::Shear) { if (!IsDistortAllowed(true) && !IsDistortAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragDistort(*this); + mpCurrentSdrDragMethod.reset(new SdrDragDistort(*this)); } else { if (!IsRotateAllowed(true)) return false; - mpCurrentSdrDragMethod = new SdrDragRotate(*this); + mpCurrentSdrDragMethod.reset(new SdrDragRotate(*this)); } } break; default: @@ -296,12 +299,12 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl if (IsMarkedHitMovesAlways() && meDragHdl==SdrHdlKind::Move) { // SdrHdlKind::Move is true, even if Obj is hit directly if (!IsMoveAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset(new SdrDragMove(*this)); } else { if (!IsRotateAllowed(true)) return false; - mpCurrentSdrDragMethod = new SdrDragRotate(*this); + mpCurrentSdrDragMethod.reset(new SdrDragRotate(*this)); } } } @@ -311,12 +314,12 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl if (meDragHdl==SdrHdlKind::Move && IsMarkedHitMovesAlways()) { if (!IsMoveAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset(new SdrDragMove(*this)); } else { if (!IsMirrorAllowed(true,true)) return false; - mpCurrentSdrDragMethod = new SdrDragMirror(*this); + mpCurrentSdrDragMethod.reset(new SdrDragMirror(*this)); } } break; @@ -326,13 +329,13 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl { if (!IsMoveAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset(new SdrDragMove(*this)); } else { if (!IsCropAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragCrop(*this); + mpCurrentSdrDragMethod.reset(new SdrDragCrop(*this)); } } break; @@ -343,14 +346,14 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl { if(!IsMoveAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset(new SdrDragMove(*this)); } else { if(!IsTransparenceAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragGradient(*this, false); + mpCurrentSdrDragMethod.reset(new SdrDragGradient(*this, false)); } break; } @@ -360,14 +363,14 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl { if(!IsMoveAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset(new SdrDragMove(*this)); } else { if(!IsGradientAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragGradient(*this); + mpCurrentSdrDragMethod.reset(new SdrDragGradient(*this)); } break; } @@ -377,12 +380,12 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl if (meDragHdl==SdrHdlKind::Move && IsMarkedHitMovesAlways()) { if (!IsMoveAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset( new SdrDragMove(*this) ); } else { if (!IsCrookAllowed(true) && !IsCrookAllowed()) return false; - mpCurrentSdrDragMethod = new SdrDragCrook(*this); + mpCurrentSdrDragMethod.reset( new SdrDragCrook(*this) ); } } break; @@ -395,7 +398,7 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl } else if(meDragHdl == SdrHdlKind::Glue) { - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset( new SdrDragMove(*this) ); } else { @@ -403,7 +406,7 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl { if(meDragHdl == SdrHdlKind::Move) { - mpCurrentSdrDragMethod = new SdrDragMove(*this); + mpCurrentSdrDragMethod.reset( new SdrDragMove(*this) ); } else { @@ -422,9 +425,9 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl bSingleTextObjMark = true; } if ( bSingleTextObjMark ) - mpCurrentSdrDragMethod = new SdrDragObjOwn(*this); + mpCurrentSdrDragMethod.reset( new SdrDragObjOwn(*this) ); else - mpCurrentSdrDragMethod = new SdrDragResize(*this); + mpCurrentSdrDragMethod.reset( new SdrDragResize(*this) ); } } else @@ -435,7 +438,7 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl if(bCustomShapeSelected) { - mpCurrentSdrDragMethod = new SdrDragMove( *this ); + mpCurrentSdrDragMethod.reset( new SdrDragMove( *this ) ); } } else if(SdrHdlKind::Poly == meDragHdl) @@ -459,44 +462,41 @@ bool SdrDragView::BegDragObj(const Point& rPnt, OutputDevice* pOut, SdrHdl* pHdl if(!mpCurrentSdrDragMethod) { // fallback to DragSpecial if no interaction defined - mpCurrentSdrDragMethod = new SdrDragObjOwn(*this); + mpCurrentSdrDragMethod.reset( new SdrDragObjOwn(*this) ); } } } } } } - if (pForcedMeth!=nullptr) + if (pForcedMeth) { - delete mpCurrentSdrDragMethod; - mpCurrentSdrDragMethod = pForcedMeth; + mpCurrentSdrDragMethod = std::move(pForcedMeth); } - maDragStat.SetDragMethod(mpCurrentSdrDragMethod); + maDragStat.SetDragMethod(mpCurrentSdrDragMethod.get()); if (mpCurrentSdrDragMethod) { bRet = mpCurrentSdrDragMethod->BeginSdrDrag(); if (!bRet) { - if (pHdl==nullptr && dynamic_cast< const SdrDragObjOwn* >(mpCurrentSdrDragMethod) != nullptr) + if (pHdl==nullptr && dynamic_cast< const SdrDragObjOwn* >(mpCurrentSdrDragMethod.get()) != nullptr) { // Obj may not Move SpecialDrag, so try with MoveFrameDrag - delete mpCurrentSdrDragMethod; - mpCurrentSdrDragMethod = nullptr; + mpCurrentSdrDragMethod.reset(); if (!IsMoveAllowed()) return false; mbFramDrag=true; - mpCurrentSdrDragMethod = new SdrDragMove(*this); - maDragStat.SetDragMethod(mpCurrentSdrDragMethod); + mpCurrentSdrDragMethod.reset( new SdrDragMove(*this) ); + maDragStat.SetDragMethod(mpCurrentSdrDragMethod.get()); bRet = mpCurrentSdrDragMethod->BeginSdrDrag(); } } if (!bRet) { - delete mpCurrentSdrDragMethod; - mpCurrentSdrDragMethod = nullptr; - maDragStat.SetDragMethod(mpCurrentSdrDragMethod); + mpCurrentSdrDragMethod.reset(); + maDragStat.SetDragMethod(mpCurrentSdrDragMethod.get()); } } } @@ -540,8 +540,7 @@ bool SdrDragView::EndDragObj(bool bCopy) if( IsInsertGluePoint() && bUndo) EndUndo(); - delete mpCurrentSdrDragMethod; - mpCurrentSdrDragMethod = nullptr; + mpCurrentSdrDragMethod.reset(); if (bEliminatePolyPoints) { @@ -592,8 +591,7 @@ void SdrDragView::BrkDragObj() { mpCurrentSdrDragMethod->CancelSdrDrag(); - delete mpCurrentSdrDragMethod; - mpCurrentSdrDragMethod = nullptr; + mpCurrentSdrDragMethod.reset(); if (mbInsPolyPoint) { @@ -855,8 +853,8 @@ void SdrDragView::SetDragStripes(bool bOn) bool SdrDragView::IsOrthoDesired() const { - if(mpCurrentSdrDragMethod && (dynamic_cast< const SdrDragObjOwn* >( mpCurrentSdrDragMethod) != nullptr - || dynamic_cast< const SdrDragResize* >(mpCurrentSdrDragMethod) != nullptr)) + if(mpCurrentSdrDragMethod && (dynamic_cast< const SdrDragObjOwn* >( mpCurrentSdrDragMethod.get() ) != nullptr + || dynamic_cast< const SdrDragResize* >(mpCurrentSdrDragMethod.get()) != nullptr)) { return bOrthoDesiredOnMarked; } diff --git a/svx/source/svdraw/svdview.cxx b/svx/source/svdraw/svdview.cxx index c831ce9b26e9..d41766e62b9c 100644 --- a/svx/source/svdraw/svdview.cxx +++ b/svx/source/svdraw/svdview.cxx @@ -1192,7 +1192,7 @@ OUString SdrView::GetStatusText() { SAL_INFO( "svx.svdraw", - "(" << this << ") " << mpCurrentSdrDragMethod); + "(" << this << ") " << mpCurrentSdrDragMethod.get()); mpCurrentSdrDragMethod->TakeSdrDragComment(aStr); } } -- cgit