compilerplugins/clang/useuniqueptr.cxx | 6 ------ sc/inc/token.hxx | 9 ++++----- sc/source/core/tool/interpr1.cxx | 10 +++++----- sc/source/core/tool/interpr4.cxx | 4 ++-- sc/source/core/tool/token.cxx | 18 +++++++++++++++--- 5 files changed, 26 insertions(+), 21 deletions(-)
New commits: commit 80dd56035e91666efa551c1e4dd4341d30c06510 Author: Noel Grandin <noel.gran...@collabora.co.uk> Date: Wed Aug 30 12:46:38 2017 +0200 fix ScJumpMatrixToken memory handling ScJumpMatrixToken unconditionally deletes the ScJumpMatrix pointer it receives. But it's copy constructor also just copies that pointer, meaning that we could end up freeing that pointer twice. ScJumpMatrix has no copy constructor, so I just managed it via shared_ptr. Change-Id: I9cf13312afb4f2869fdc878e5f34060614e31842 Reviewed-on: https://gerrit.libreoffice.org/41728 Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> Tested-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/compilerplugins/clang/useuniqueptr.cxx b/compilerplugins/clang/useuniqueptr.cxx index e47ebd23450a..0f6937f182b2 100644 --- a/compilerplugins/clang/useuniqueptr.cxx +++ b/compilerplugins/clang/useuniqueptr.cxx @@ -127,15 +127,9 @@ void UseUniquePtr::CheckForSingleUnconditionalDelete(const CXXDestructorDecl* de // something platform-specific if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/hwpfilter/source/htags.h")) return; - // @TODO there is clearly a bug in the ownership here, the operator= method cannot be right - if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/formula/formdata.hxx")) - return; // passes pointers to member fields if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sd/inc/sdpptwrp.hxx")) return; - // @TODO there is clearly a bug in the ownership here, the ScJumpMatrixToken copy constructor cannot be right - if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/inc/token.hxx")) - return; // @TODO intrusive linked-lists here, with some trickiness if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/html/parcss1.hxx")) return; diff --git a/sc/inc/token.hxx b/sc/inc/token.hxx index 7e20b691d96b..9019ebf0ed89 100644 --- a/sc/inc/token.hxx +++ b/sc/inc/token.hxx @@ -20,6 +20,7 @@ #ifndef INCLUDED_SC_INC_TOKEN_HXX #define INCLUDED_SC_INC_TOKEN_HXX +#include <memory> #include <vector> #include <boost/intrusive_ptr.hpp> @@ -247,12 +248,10 @@ private: class ScJumpMatrixToken : public formula::FormulaToken { private: - ScJumpMatrix* pJumpMatrix; + std::shared_ptr<ScJumpMatrix> mpJumpMatrix; public: - ScJumpMatrixToken( ScJumpMatrix* p ) : - FormulaToken( formula::svJumpMatrix ), pJumpMatrix( p ) {} - ScJumpMatrixToken( const ScJumpMatrixToken& r ) : - FormulaToken( r ), pJumpMatrix( r.pJumpMatrix ) {} + ScJumpMatrixToken( std::shared_ptr<ScJumpMatrix> p ); + ScJumpMatrixToken( const ScJumpMatrixToken & p ); virtual ~ScJumpMatrixToken() override; virtual ScJumpMatrix* GetJumpMatrix() const override; virtual bool operator==( const formula::FormulaToken& rToken ) const override; diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx index 7471dd9161d2..1b1ba4f703a6 100644 --- a/sc/source/core/tool/interpr1.cxx +++ b/sc/source/core/tool/interpr1.cxx @@ -112,7 +112,7 @@ void ScInterpreter::ScIfJump() xNew = (*aMapIter).second; else { - ScJumpMatrix* pJumpMat = new ScJumpMatrix( nCols, nRows ); + std::shared_ptr<ScJumpMatrix> pJumpMat( std::make_shared<ScJumpMatrix>( nCols, nRows ) ); for ( SCSIZE nC=0; nC < nCols; ++nC ) { for ( SCSIZE nR=0; nR < nRows; ++nR ) @@ -341,7 +341,7 @@ void ScInterpreter::ScIfError( bool bNAonly ) else { const ScMatrix* pMatPtr = pMat.get(); - ScJumpMatrix* pJumpMat = new ScJumpMatrix( nCols, nRows ); + std::shared_ptr<ScJumpMatrix> pJumpMat( std::make_shared<ScJumpMatrix>( nCols, nRows ) ); // Init all jumps to no error to save single calls. Error // is the exceptional condition. const double fFlagResult = CreateDoubleError( FormulaError::JumpMatHasResult); @@ -353,7 +353,7 @@ void ScInterpreter::ScIfError( bool bNAonly ) { for (nR = 0 ; nR < nRows && (nC != nErrorCol || nR != nErrorRow); ++nR) { - lcl_storeJumpMatResult(pMatPtr, pJumpMat, nC, nR); + lcl_storeJumpMatResult(pMatPtr, pJumpMat.get(), nC, nR); } if (nC != nErrorCol && nR != nErrorRow) ++nC; @@ -370,7 +370,7 @@ void ScInterpreter::ScIfError( bool bNAonly ) } else { // FALSE, EMPTY path, store result instead - lcl_storeJumpMatResult(pMatPtr, pJumpMat, nC, nR); + lcl_storeJumpMatResult(pMatPtr, pJumpMat.get(), nC, nR); } } nR = 0; @@ -432,7 +432,7 @@ void ScInterpreter::ScChooseJump() xNew = (*aMapIter).second; else { - ScJumpMatrix* pJumpMat = new ScJumpMatrix( nCols, nRows ); + std::shared_ptr<ScJumpMatrix> pJumpMat( std::make_shared<ScJumpMatrix>( nCols, nRows ) ); for ( SCSIZE nC=0; nC < nCols; ++nC ) { for ( SCSIZE nR=0; nR < nRows; ++nR ) diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx index 282e9319a88c..6fa8f6294f4f 100644 --- a/sc/source/core/tool/interpr4.cxx +++ b/sc/source/core/tool/interpr4.cxx @@ -1579,7 +1579,7 @@ bool ScInterpreter::ConvertMatrixParameters() xNew = (*aMapIter).second; else { - ScJumpMatrix* pJumpMat = new ScJumpMatrix( nJumpCols, nJumpRows); + std::unique_ptr<ScJumpMatrix> pJumpMat( new ScJumpMatrix( nJumpCols, nJumpRows) ); pJumpMat->SetAllJumps( 1.0, nStart, nNext, nStop); // pop parameters and store in ScJumpMatrix, push in JumpMatrix() ScTokenVec* pParams = new ScTokenVec( nParams); @@ -1591,7 +1591,7 @@ bool ScInterpreter::ConvertMatrixParameters() (*pParams)[ nParams - i ] = p; } pJumpMat->SetJumpParameters( pParams); - xNew = new ScJumpMatrixToken( pJumpMat ); + xNew = new ScJumpMatrixToken( std::move(pJumpMat) ); GetTokenMatrixMap().emplace(pCur, xNew); } PushTempTokenWithoutError( xNew.get()); diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx index 2a540a38aa6a..4d77ad89e1a6 100644 --- a/sc/source/core/tool/token.cxx +++ b/sc/source/core/tool/token.cxx @@ -997,14 +997,26 @@ bool ScTableRefToken::operator==( const FormulaToken& r ) const return true; } -ScJumpMatrix* ScJumpMatrixToken::GetJumpMatrix() const { return pJumpMatrix; } +ScJumpMatrixToken::ScJumpMatrixToken( std::shared_ptr<ScJumpMatrix> p ) + : FormulaToken( formula::svJumpMatrix ), mpJumpMatrix( p ) +{} + +ScJumpMatrixToken::ScJumpMatrixToken( const ScJumpMatrixToken & p ) + : FormulaToken( p ), mpJumpMatrix( p.mpJumpMatrix ) +{} + +ScJumpMatrix* ScJumpMatrixToken::GetJumpMatrix() const +{ + return mpJumpMatrix.get(); +} + bool ScJumpMatrixToken::operator==( const FormulaToken& r ) const { - return FormulaToken::operator==( r ) && pJumpMatrix == r.GetJumpMatrix(); + return FormulaToken::operator==( r ) && mpJumpMatrix.get() == r.GetJumpMatrix(); } + ScJumpMatrixToken::~ScJumpMatrixToken() { - delete pJumpMatrix; } double ScEmptyCellToken::GetDouble() const { return 0.0; } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits