[Libreoffice-commits] core.git: compilerplugins/clang sc/inc sc/source
Noel Grandin
noel.grandin at collabora.co.uk
Wed Aug 30 11:27:23 UTC 2017
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.grandin at 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.grandin at collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin at 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; }
More information about the Libreoffice-commits
mailing list