compilerplugins/clang/useuniqueptr.cxx | 137 +++++++++++++++++++++++++++++++ hwpfilter/source/hwpreader.cxx | 3 hwpfilter/source/hwpreader.hxx | 4 include/basegfx/raster/bpixelraster.hxx | 5 - include/basegfx/raster/bzpixelraster.hxx | 5 - sw/source/filter/ww8/wrtw8sty.cxx | 3 sw/source/filter/ww8/wrtww8.cxx | 12 +- sw/source/filter/ww8/wrtww8.hxx | 6 - sw/source/filter/ww8/ww8scan.cxx | 14 +-- sw/source/filter/ww8/ww8scan.hxx | 9 +- sw/source/filter/ww8/ww8toolbar.cxx | 6 - sw/source/filter/ww8/ww8toolbar.hxx | 5 - sw/source/ui/index/swuiidxmrk.cxx | 4 sw/source/uibase/inc/swuiidxmrk.hxx | 4 14 files changed, 173 insertions(+), 44 deletions(-)
New commits: commit 98e4013c22c4ce63090a575e698cc2af82925e6b Author: Noel Grandin <noel.gran...@collabora.co.uk> Date: Wed Jan 11 14:19:47 2017 +0200 new loplugin useuniqueptr Change-Id: Ic7a8b32887c968d86568e4cfad7ddd1f4da7c73f Reviewed-on: https://gerrit.libreoffice.org/33339 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 new file mode 100644 index 0000000..82d95e5 --- /dev/null +++ b/compilerplugins/clang/useuniqueptr.cxx @@ -0,0 +1,137 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <cassert> +#include <string> +#include <iostream> +#include <fstream> +#include <set> +#include "plugin.hxx" + +/** + Find destructors that only contain a single call to delete of a field. In which + case that field should really be managed by unique_ptr. +*/ + +namespace { + +class UseUniquePtr: + public RecursiveASTVisitor<UseUniquePtr>, public loplugin::Plugin +{ +public: + explicit UseUniquePtr(InstantiationData const & data): Plugin(data) {} + + virtual void run() override + { + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool VisitCXXDestructorDecl(const CXXDestructorDecl* ); +}; + +bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDecl) +{ + if (ignoreLocation(destructorDecl)) + return true; + if (isInUnoIncludeFile(destructorDecl)) + return true; + +/* + StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(destructorDecl->getLocStart())); + // weird stuff, passing pointers to internal members of struct + if (aFileName.startswith(SRCDIR "/include/jvmfwk/framework.hxx")) + return true; +*/ + if (destructorDecl->getBody() == nullptr) + return true; + const CompoundStmt* compoundStmt = dyn_cast< CompoundStmt >( destructorDecl->getBody() ); + if (compoundStmt == nullptr) { + return true; + } + if (compoundStmt->size() != 1) { + return true; + } + const CXXDeleteExpr* deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_front()); + if (deleteExpr == nullptr) { + return true; + } + + const ImplicitCastExpr* pCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument()); + if (!pCastExpr) + return true; + const MemberExpr* pMemberExpr = dyn_cast<MemberExpr>(pCastExpr->getSubExpr()); + if (!pMemberExpr) + return true; + + // ignore union games + const FieldDecl* pFieldDecl = dyn_cast<FieldDecl>(pMemberExpr->getMemberDecl()); + if (!pFieldDecl) + return true; + TagDecl const * td = dyn_cast<TagDecl>(pFieldDecl->getDeclContext()); + if (td->isUnion()) + return true; + + // ignore calling delete on someone else's field + if (pFieldDecl->getParent() != destructorDecl->getParent() ) + return true; + + if (ignoreLocation(pFieldDecl)) + return true; + // to ignore things like the CPPUNIT macros + StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(pFieldDecl->getLocStart())); + if (aFileName.startswith(WORKDIR)) + return true; + // weird stuff, passing pointers to internal members of struct + if (aFileName.startswith(SRCDIR "/include/jvmfwk/framework.hxx")) + return true; + if (aFileName.startswith(SRCDIR "/jvmfwk/")) + return true; + // passes and stores pointers to member fields + if (aFileName.startswith(SRCDIR "/sot/source/sdstor/stgdir.hxx")) + return true; + // passes and stores pointers to member fields + if (aFileName.startswith(SRCDIR "/desktop/source/migration/services/jvmfwk.cxx")) + return true; + // something platform-specific + if (aFileName.startswith(SRCDIR "/hwpfilter/source/htags.h")) + return true; + // @TODO there is clearly a bug in the ownership here, the operator= method cannot be right + if (aFileName.startswith(SRCDIR "/include/formula/formdata.hxx")) + return true; + // passes pointers to member fields + if (aFileName.startswith(SRCDIR "/sd/inc/sdpptwrp.hxx")) + return true; + // @TODO there is clearly a bug in the ownership here, the ScJumpMatrixToken copy constructor cannot be right + if (aFileName.startswith(SRCDIR "/sc/inc/token.hxx")) + return true; + // @TODO intrusive linked-lists here, with some trickiness + if (aFileName.startswith(SRCDIR "/sw/source/filter/html/parcss1.hxx")) + return true; + // @TODO SwDoc has some weird ref-counting going on + if (aFileName.startswith(SRCDIR "/sw/inc/shellio.hxx")) + return true; + + report( + DiagnosticsEngine::Warning, + "a destructor with only a single unconditional call to delete on a member, is a sure sign it should be using std::unique_ptr for that field", + deleteExpr->getLocStart()) + << deleteExpr->getSourceRange(); + report( + DiagnosticsEngine::Note, + "member is here", + pFieldDecl->getLocStart()) + << pFieldDecl->getSourceRange(); + return true; +} + +loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr"); + +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/hwpfilter/source/hwpreader.cxx b/hwpfilter/source/hwpreader.cxx index f353bd7..68388d1 100644 --- a/hwpfilter/source/hwpreader.cxx +++ b/hwpfilter/source/hwpreader.cxx @@ -108,13 +108,12 @@ struct HwpReaderPrivate HwpReader::HwpReader() { mxList = new AttributeListImpl; - d = new HwpReaderPrivate; + d.reset( new HwpReaderPrivate ); } HwpReader::~HwpReader() { - delete d; } extern "C" SAL_DLLPUBLIC_EXPORT bool SAL_CALL TestImportHWP(const OUString &rURL) diff --git a/hwpfilter/source/hwpreader.hxx b/hwpfilter/source/hwpreader.hxx index d1dbd16..c44a0c3 100644 --- a/hwpfilter/source/hwpreader.hxx +++ b/hwpfilter/source/hwpreader.hxx @@ -19,6 +19,7 @@ #ifndef INCLUDED_HWPFILTER_SOURCE_HWPREADER_HXX #define INCLUDED_HWPFILTER_SOURCE_HWPREADER_HXX + #include <errno.h> #include <stdio.h> #include <string.h> @@ -40,6 +41,7 @@ #include <cppuhelper/implbase.hxx> #include <cppuhelper/supportsservice.hxx> #include <cppuhelper/weak.hxx> +#include <memory> using namespace ::cppu; using namespace ::com::sun::star::lang; @@ -92,7 +94,7 @@ private: Reference< XDocumentHandler > m_rxDocumentHandler; rtl::Reference<AttributeListImpl> mxList; HWPFile hwpfile; - HwpReaderPrivate *d; + std::unique_ptr<HwpReaderPrivate> d; private: /* -------- Document Parsing --------- */ void makeMeta(); diff --git a/include/basegfx/raster/bpixelraster.hxx b/include/basegfx/raster/bpixelraster.hxx index f85c0af..46ef649 100644 --- a/include/basegfx/raster/bpixelraster.hxx +++ b/include/basegfx/raster/bpixelraster.hxx @@ -38,13 +38,13 @@ namespace basegfx sal_uInt32 mnWidth; sal_uInt32 mnHeight; sal_uInt32 mnCount; - BPixel* mpContent; + std::unique_ptr<BPixel[]> mpContent; public: // reset void reset() { - memset(mpContent, 0, sizeof(BPixel) * mnCount); + memset(mpContent.get(), 0, sizeof(BPixel) * mnCount); } // constructor/destructor @@ -59,7 +59,6 @@ namespace basegfx ~BPixelRaster() { - delete [] mpContent; } // coordinate calcs between X/Y and span diff --git a/include/basegfx/raster/bzpixelraster.hxx b/include/basegfx/raster/bzpixelraster.hxx index 722abde..3e0d51b 100644 --- a/include/basegfx/raster/bzpixelraster.hxx +++ b/include/basegfx/raster/bzpixelraster.hxx @@ -30,7 +30,7 @@ namespace basegfx { protected: // additionally, host a ZBuffer - sal_uInt16* mpZBuffer; + std::unique_ptr<sal_uInt16[]> mpZBuffer; public: // constructor/destructor @@ -38,12 +38,11 @@ namespace basegfx : BPixelRaster(nWidth, nHeight), mpZBuffer(new sal_uInt16[mnCount]) { - memset(mpZBuffer, 0, sizeof(sal_uInt16) * mnCount); + memset(mpZBuffer.get(), 0, sizeof(sal_uInt16) * mnCount); } ~BZPixelRaster() { - delete [] mpZBuffer; } // data access read only diff --git a/sw/source/filter/ww8/wrtw8sty.cxx b/sw/source/filter/ww8/wrtw8sty.cxx index 23a7122..420ac67 100644 --- a/sw/source/filter/ww8/wrtw8sty.cxx +++ b/sw/source/filter/ww8/wrtw8sty.cxx @@ -1919,7 +1919,6 @@ WW8_WrPlcSubDoc::WW8_WrPlcSubDoc() WW8_WrPlcSubDoc::~WW8_WrPlcSubDoc() { - delete pTextPos; } void WW8_WrPlcFootnoteEdn::Append( WW8_CP nCp, const SwFormatFootnote& rFootnote ) @@ -2002,7 +2001,7 @@ bool WW8_WrPlcSubDoc::WriteGenericText( WW8Export& rWrt, sal_uInt8 nTTyp, return false; sal_uLong nCpStart = rWrt.Fc2Cp( rWrt.Strm().Tell() ); - pTextPos = new WW8_WrPlc0( nCpStart ); + pTextPos.reset( new WW8_WrPlc0( nCpStart ) ); sal_uInt16 i; switch ( nTTyp ) diff --git a/sw/source/filter/ww8/wrtww8.cxx b/sw/source/filter/ww8/wrtww8.cxx index 10ebf2d..447e3c0 100644 --- a/sw/source/filter/ww8/wrtww8.cxx +++ b/sw/source/filter/ww8/wrtww8.cxx @@ -814,12 +814,11 @@ WW8_WrPlc1::WW8_WrPlc1( sal_uInt16 nStructSz ) : nStructSiz( nStructSz ) { nDataLen = 16 * nStructSz; - pData = new sal_uInt8[ nDataLen ]; + pData.reset( new sal_uInt8[ nDataLen ] ); } WW8_WrPlc1::~WW8_WrPlc1() { - delete[] pData; } WW8_CP WW8_WrPlc1::Prev() const @@ -836,12 +835,11 @@ void WW8_WrPlc1::Append( WW8_CP nCp, const void* pNewData ) if( nDataLen < nInsPos + nStructSiz ) { sal_uInt8* pNew = new sal_uInt8[ 2 * nDataLen ]; - memcpy( pNew, pData, nDataLen ); - delete[] pData; - pData = pNew; + memcpy( pNew, pData.get(), nDataLen ); + pData.reset(pNew); nDataLen *= 2; } - memcpy( pData + nInsPos, pNewData, nStructSiz ); + memcpy( pData.get() + nInsPos, pNewData, nStructSiz ); } void WW8_WrPlc1::Finish( sal_uLong nLastCp, sal_uLong nSttCp ) @@ -861,7 +859,7 @@ void WW8_WrPlc1::Write( SvStream& rStrm ) for( i = 0; i < aPos.size(); ++i ) SwWW8Writer::WriteLong( rStrm, aPos[i] ); if( i ) - rStrm.WriteBytes(pData, (i-1) * nStructSiz); + rStrm.WriteBytes(pData.get(), (i-1) * nStructSiz); } // Class WW8_WrPlcField for fields diff --git a/sw/source/filter/ww8/wrtww8.hxx b/sw/source/filter/ww8/wrtww8.hxx index 6cbb890..10ccb85 100644 --- a/sw/source/filter/ww8/wrtww8.hxx +++ b/sw/source/filter/ww8/wrtww8.hxx @@ -1169,8 +1169,8 @@ private: protected: std::vector<WW8_CP> aCps; std::vector<const void*> aContent; // PTRARR of SwFormatFootnote/PostIts/.. - std::vector<const SwFrameFormat*> aSpareFormats; //a backup for aContent: if there's no SdrObject, stores the fmt directly here - WW8_WrPlc0* pTextPos; // positions of the individual texts + std::vector<const SwFrameFormat*> aSpareFormats; // a backup for aContent: if there's no SdrObject, stores the fmt directly here + std::unique_ptr<WW8_WrPlc0> pTextPos; // positions of the individual texts WW8_WrPlcSubDoc(); virtual ~WW8_WrPlcSubDoc(); @@ -1281,7 +1281,7 @@ class WW8_WrPlc1 { private: std::vector<WW8_CP> aPos; - sal_uInt8* pData; // content ( structures ) + std::unique_ptr<sal_uInt8[]> pData; // content ( structures ) sal_uLong nDataLen; sal_uInt16 nStructSiz; diff --git a/sw/source/filter/ww8/ww8scan.cxx b/sw/source/filter/ww8/ww8scan.cxx index 039b24c..3d221b2 100644 --- a/sw/source/filter/ww8/ww8scan.cxx +++ b/sw/source/filter/ww8/ww8scan.cxx @@ -1123,14 +1123,12 @@ WW8PLCFx_PCD::WW8PLCFx_PCD(const WW8Fib& rFib, WW8PLCFpcd* pPLCFpcd, : WW8PLCFx(rFib, false), nClipStart(-1) { // construct own iterator - pPcdI = new WW8PLCFpcd_Iter(*pPLCFpcd, nStartCp); + pPcdI.reset( new WW8PLCFpcd_Iter(*pPLCFpcd, nStartCp) ); bVer67= bVer67P; } WW8PLCFx_PCD::~WW8PLCFx_PCD() { - // pPcd-Dtor which in called from WW8ScannerBase - delete pPcdI; } sal_uInt32 WW8PLCFx_PCD::GetIMax() const @@ -3098,8 +3096,8 @@ WW8PLCFx_Cp_FKP::WW8PLCFx_Cp_FKP( SvStream* pSt, SvStream* pTableSt, { ResetAttrStartEnd(); - pPcd = rSBase.m_pPiecePLCF ? new WW8PLCFx_PCD(GetFIB(), - rBase.m_pPiecePLCF, 0, IsSevenMinus(GetFIBVersion())) : nullptr; + if (rSBase.m_pPiecePLCF) + pPcd.reset( new WW8PLCFx_PCD(GetFIB(), rBase.m_pPiecePLCF, 0, IsSevenMinus(GetFIBVersion())) ); /* Make a copy of the piece attributes for so that the calls to HasSprm on a @@ -3110,7 +3108,7 @@ WW8PLCFx_Cp_FKP::WW8PLCFx_Cp_FKP( SvStream* pSt, SvStream* pTableSt, if (pPcd) { pPCDAttrs = rSBase.m_pPLCFx_PCDAttrs ? new WW8PLCFx_PCDAttrs( - *rSBase.m_pWw8Fib, pPcd, &rSBase) : nullptr; + *rSBase.m_pWw8Fib, pPcd.get(), &rSBase) : nullptr; } pPieceIter = rSBase.m_pPieceIter; @@ -3118,7 +3116,6 @@ WW8PLCFx_Cp_FKP::WW8PLCFx_Cp_FKP( SvStream* pSt, SvStream* pTableSt, WW8PLCFx_Cp_FKP::~WW8PLCFx_Cp_FKP() { - delete pPcd; } void WW8PLCFx_Cp_FKP::ResetAttrStartEnd() @@ -3680,12 +3677,11 @@ WW8PLCFx_FLD::WW8PLCFx_FLD( SvStream* pSt, const WW8Fib& rMyFib, short nType) } if( nLen ) - pPLCF = new WW8PLCFspecial( pSt, nFc, nLen, 2 ); + pPLCF.reset( new WW8PLCFspecial( pSt, nFc, nLen, 2 ) ); } WW8PLCFx_FLD::~WW8PLCFx_FLD() { - delete pPLCF; } sal_uInt32 WW8PLCFx_FLD::GetIdx() const diff --git a/sw/source/filter/ww8/ww8scan.hxx b/sw/source/filter/ww8/ww8scan.hxx index fab1906..d5a0ff7 100644 --- a/sw/source/filter/ww8/ww8scan.hxx +++ b/sw/source/filter/ww8/ww8scan.hxx @@ -27,6 +27,7 @@ #include <cassert> #include <cstddef> #include <list> +#include <memory> #include <stack> #include <unordered_map> #include <vector> @@ -446,7 +447,7 @@ public: class WW8PLCFx_PCD : public WW8PLCFx // iterator for Piece table { private: - WW8PLCFpcd_Iter* pPcdI; + std::unique_ptr<WW8PLCFpcd_Iter> pPcdI; bool bVer67; WW8_CP nClipStart; @@ -468,7 +469,7 @@ public: WW8_FC AktPieceStartCp2Fc( WW8_CP nCp ); static void AktPieceFc2Cp(WW8_CP& rStartPos, WW8_CP& rEndPos, const WW8ScannerBase *pSBase); - WW8PLCFpcd_Iter* GetPLCFIter() { return pPcdI; } + WW8PLCFpcd_Iter* GetPLCFIter() { return pPcdI.get(); } void SetClipStart(WW8_CP nIn) { nClipStart = nIn; } WW8_CP GetClipStart() { return nClipStart; } @@ -612,7 +613,7 @@ class WW8PLCFx_Cp_FKP : public WW8PLCFx_Fc_FKP { private: const WW8ScannerBase& rSBase; - WW8PLCFx_PCD* pPcd; + std::unique_ptr<WW8PLCFx_PCD> pPcd; WW8PLCFpcd_Iter *pPieceIter; WW8_CP nAttrStart, nAttrEnd; bool bLineEnd : 1; @@ -703,7 +704,7 @@ public: class WW8PLCFx_FLD : public WW8PLCFx { private: - WW8PLCFspecial* pPLCF; + std::unique_ptr<WW8PLCFspecial> pPLCF; const WW8Fib& rFib; WW8PLCFx_FLD(const WW8PLCFx_FLD&) = delete; WW8PLCFx_FLD& operator=(const WW8PLCFx_FLD &) = delete; diff --git a/sw/source/filter/ww8/ww8toolbar.cxx b/sw/source/filter/ww8/ww8toolbar.cxx index ae31723..0d66d77 100644 --- a/sw/source/filter/ww8/ww8toolbar.cxx +++ b/sw/source/filter/ww8/ww8toolbar.cxx @@ -1097,7 +1097,6 @@ TcgSttbfCore::TcgSttbfCore() : fExtend( 0 ) TcgSttbfCore::~TcgSttbfCore() { - delete[] dataItems; } bool TcgSttbfCore::Read( SvStream& rS ) @@ -1109,7 +1108,7 @@ bool TcgSttbfCore::Read( SvStream& rS ) { if (cData > rS.remainingSize() / 4) //definitely an invalid record return false; - dataItems = new SBBItem[ cData ]; + dataItems.reset( new SBBItem[ cData ] ); for ( sal_Int32 index = 0; index < cData; ++index ) { rS.ReadUInt16( dataItems[ index ].cchData ); @@ -1145,7 +1144,6 @@ MacroNames::MacroNames() : MacroNames::~MacroNames() { - delete[] rgNames; } bool MacroNames::Read( SvStream &rS) @@ -1160,7 +1158,7 @@ bool MacroNames::Read( SvStream &rS) size_t nMaxAvailableRecords = rS.remainingSize()/sizeof(sal_uInt16); if (iMac > nMaxAvailableRecords) return false; - rgNames = new MacroName[ iMac ]; + rgNames.reset( new MacroName[ iMac ] ); for ( sal_Int32 index = 0; index < iMac; ++index ) { if ( !rgNames[ index ].Read( rS ) ) diff --git a/sw/source/filter/ww8/ww8toolbar.hxx b/sw/source/filter/ww8/ww8toolbar.hxx index c37176f..e98f664 100644 --- a/sw/source/filter/ww8/ww8toolbar.hxx +++ b/sw/source/filter/ww8/ww8toolbar.hxx @@ -11,6 +11,7 @@ #include <com/sun/star/container/XIndexContainer.hpp> #include <filter/msfilter/mstoolbar.hxx> +#include <memory> class Xst : public TBBase { @@ -294,7 +295,7 @@ class TcgSttbfCore : public TBBase sal_uInt16 fExtend; sal_uInt16 cData; sal_uInt16 cbExtra; - SBBItem* dataItems; + std::unique_ptr<SBBItem[]> dataItems; TcgSttbfCore(const TcgSttbfCore&) = delete; TcgSttbfCore& operator = ( const TcgSttbfCore&) = delete; @@ -358,7 +359,7 @@ public: class MacroNames : public Tcg255SubStruct { sal_uInt16 iMac; //An unsigned integer that specifies the number of MacroName structures in rgNames. - MacroName* rgNames; + std::unique_ptr<MacroName[]> rgNames; MacroNames(const MacroNames&) = delete; MacroNames& operator = ( const MacroNames&) = delete; diff --git a/sw/source/ui/index/swuiidxmrk.cxx b/sw/source/ui/index/swuiidxmrk.cxx index bcf1905..199d3ec 100644 --- a/sw/source/ui/index/swuiidxmrk.cxx +++ b/sw/source/ui/index/swuiidxmrk.cxx @@ -949,14 +949,12 @@ IMPL_LINK( SwIndexMarkPane, KeyDCBModifyHdl, Edit&, rEdit, void ) SwIndexMarkPane::~SwIndexMarkPane() { - delete pTOXMgr; } void SwIndexMarkPane::ReInitDlg(SwWrtShell& rWrtShell, SwTOXMark* pCurTOXMark) { pSh = &rWrtShell; - delete pTOXMgr; - pTOXMgr = new SwTOXMgr(pSh); + pTOXMgr.reset( new SwTOXMgr(pSh) ); if(pCurTOXMark) { for(sal_uInt16 i = 0; i < pTOXMgr->GetTOXMarkCount(); i++) diff --git a/sw/source/uibase/inc/swuiidxmrk.hxx b/sw/source/uibase/inc/swuiidxmrk.hxx index 609e2d1..2ae2ab4 100644 --- a/sw/source/uibase/inc/swuiidxmrk.hxx +++ b/sw/source/uibase/inc/swuiidxmrk.hxx @@ -35,6 +35,7 @@ #include <sfx2/childwin.hxx> #include "toxe.hxx" #include <com/sun/star/i18n/XExtendedIndexEntrySupplier.hpp> +#include <memory> class SwWrtShell; class SwTOXMgr; @@ -99,7 +100,8 @@ class SwIndexMarkPane css::uno::Reference< css::i18n::XExtendedIndexEntrySupplier > xExtendedIndexEntrySupplier; - SwTOXMgr* pTOXMgr; + std::unique_ptr<SwTOXMgr> + pTOXMgr; SwWrtShell* pSh; void Apply(); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits