[Libreoffice-commits] core.git: compilerplugins/clang include/svx include/tools shell/source svx/source sw/source tools/source
Noel Grandin
noel.grandin at collabora.co.uk
Fri Jun 22 18:12:16 UTC 2018
compilerplugins/clang/test/useuniqueptr.cxx | 4
compilerplugins/clang/useuniqueptr.cxx | 62 ++++++++++++++
include/svx/gallery1.hxx | 9 --
include/tools/config.hxx | 3
shell/source/unix/sysshell/recently_used_file_handler.cxx | 42 ++-------
svx/source/gengal/gengal.cxx | 20 ----
sw/source/filter/xml/xmlimpit.cxx | 40 +++------
sw/source/filter/xml/xmlithlp.cxx | 15 +--
sw/source/filter/xml/xmlithlp.hxx | 4
tools/source/generic/config.cxx | 22 +---
10 files changed, 114 insertions(+), 107 deletions(-)
New commits:
commit 200ca388246e525f6e8f909766977f534c0c897e
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Thu Jun 21 15:32:52 2018 +0200
teach useuniqueptr loplugin about calling delete on a param
which is often a useful indicator that the callers can be made to use
std::unique_ptr
Change-Id: Idb1745d1f58dbcf389c9f60f1a5728d7d092ade5
Reviewed-on: https://gerrit.libreoffice.org/56238
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/test/useuniqueptr.cxx b/compilerplugins/clang/test/useuniqueptr.cxx
index 8ef71175bb99..c7c92313a5e6 100644
--- a/compilerplugins/clang/test/useuniqueptr.cxx
+++ b/compilerplugins/clang/test/useuniqueptr.cxx
@@ -172,4 +172,8 @@ class Foo14 {
}
}
};
+void Foo15(int * p)
+{
+ delete p; // expected-error {{calling delete on a pointer param, should be either whitelisted here or simplified [loplugin:useuniqueptr]}}
+};
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/useuniqueptr.cxx b/compilerplugins/clang/useuniqueptr.cxx
index 2424bcca8c09..131727f0f44f 100644
--- a/compilerplugins/clang/useuniqueptr.cxx
+++ b/compilerplugins/clang/useuniqueptr.cxx
@@ -68,6 +68,8 @@ public:
bool VisitCXXMethodDecl(const CXXMethodDecl* );
bool VisitCompoundStmt(const CompoundStmt* );
+ bool VisitCXXDeleteExpr(const CXXDeleteExpr* );
+ bool TraverseFunctionDecl(FunctionDecl* );
private:
void CheckCompoundStmt(const CXXMethodDecl*, const CompoundStmt* );
void CheckForUnconditionalDelete(const CXXMethodDecl*, const CompoundStmt* );
@@ -79,6 +81,7 @@ private:
void CheckParenExpr(const CXXMethodDecl*, const ParenExpr*);
void CheckDeleteExpr(const CXXMethodDecl*, const CXXDeleteExpr*,
const MemberExpr*, StringRef message);
+ FunctionDecl const * mpCurrentFunctionDecl = nullptr;
};
bool UseUniquePtr::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
@@ -413,6 +416,65 @@ bool UseUniquePtr::VisitCompoundStmt(const CompoundStmt* compoundStmt)
return true;
}
+bool UseUniquePtr::TraverseFunctionDecl(FunctionDecl* functionDecl)
+{
+ if (ignoreLocation(functionDecl))
+ return true;
+
+ auto oldCurrent = mpCurrentFunctionDecl;
+ mpCurrentFunctionDecl = functionDecl;
+ bool ret = RecursiveASTVisitor::TraverseFunctionDecl(functionDecl);
+ mpCurrentFunctionDecl = oldCurrent;
+
+ return ret;
+}
+
+bool UseUniquePtr::VisitCXXDeleteExpr(const CXXDeleteExpr* deleteExpr)
+{
+ if (!mpCurrentFunctionDecl)
+ return true;
+ if (ignoreLocation(mpCurrentFunctionDecl))
+ return true;
+ if (isInUnoIncludeFile(mpCurrentFunctionDecl->getCanonicalDecl()->getLocStart()))
+ return true;
+ if (mpCurrentFunctionDecl->getIdentifier())
+ {
+ auto name = mpCurrentFunctionDecl->getName();
+ if (name == "delete_IncludesCollection" || name == "convertName"
+ || name == "createNamedType"
+ || name == "typelib_typedescriptionreference_release" || name == "deleteExceptions"
+ || name == "uno_threadpool_destroy"
+ || name == "AddRanges_Impl"
+ || name == "DestroySalInstance"
+ || name == "ImplHandleUserEvent"
+ || name == "releaseDecimalPtr" // TODO, basic
+ || name == "replaceAndReset" // TODO, connectivity
+ || name == "intrusive_ptr_release"
+ || name == "FreeParaList"
+ || name == "DeleteSdrUndoAction" // TODO, sc
+ || name == "lcl_MergeGCBox" || name == "lcl_MergeGCLine" || name == "lcl_DelHFFormat")
+ return true;
+ }
+
+ auto declRefExpr = dyn_cast<DeclRefExpr>(deleteExpr->getArgument()->IgnoreParenImpCasts());
+ if (!declRefExpr)
+ return true;
+ auto varDecl = dyn_cast<ParmVarDecl>(declRefExpr->getDecl());
+ if (!varDecl)
+ return true;
+
+ /*
+ Sometimes we can pass the param as std::unique_ptr<T>& or std::unique_ptr, sometimes the method
+ just needs to be inlined, which normally exposes more simplification.
+ */
+ report(
+ DiagnosticsEngine::Warning,
+ "calling delete on a pointer param, should be either whitelisted here or simplified",
+ deleteExpr->getLocStart())
+ << deleteExpr->getSourceRange();
+ return true;
+}
+
loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr", false);
}
diff --git a/include/svx/gallery1.hxx b/include/svx/gallery1.hxx
index 66474d17180b..989603e46deb 100644
--- a/include/svx/gallery1.hxx
+++ b/include/svx/gallery1.hxx
@@ -86,10 +86,6 @@ class GalleryThemeCacheEntry;
class SVX_DLLPUBLIC Gallery : public SfxBroadcaster
{
- // only for gengal utility!
- friend Gallery* createGallery( const OUString& );
- friend void disposeGallery( Gallery* );
-
typedef std::vector<GalleryThemeCacheEntry*> GalleryCacheThemeList;
private:
@@ -108,12 +104,13 @@ private:
SAL_DLLPRIVATE GalleryTheme* ImplGetCachedTheme( const GalleryThemeEntry* pThemeEntry );
SAL_DLLPRIVATE void ImplDeleteCachedTheme( GalleryTheme const * pTheme );
- Gallery( const OUString& rMultiPath );
- virtual ~Gallery() override;
Gallery& operator=( Gallery const & ) = delete; // MSVC2015 workaround
Gallery( Gallery const & ) = delete; // MSVC2015 workaround
public:
+ // only for gengal utility!
+ Gallery( const OUString& rMultiPath );
+ virtual ~Gallery() override;
static Gallery* GetGalleryInstance();
diff --git a/include/tools/config.hxx b/include/tools/config.hxx
index d6a00aa61d64..76e4270b5e9d 100644
--- a/include/tools/config.hxx
+++ b/include/tools/config.hxx
@@ -21,6 +21,7 @@
#include <tools/toolsdllapi.h>
#include <rtl/ustring.hxx>
+#include <memory>
struct ImplConfigData;
struct ImplGroupData;
@@ -30,7 +31,7 @@ class SAL_WARN_UNUSED TOOLS_DLLPUBLIC Config
private:
OUString maFileName;
OString maGroupName;
- ImplConfigData* mpData;
+ std::unique_ptr<ImplConfigData> mpData;
ImplGroupData* mpActGroup;
sal_uInt32 mnDataUpdateId;
diff --git a/shell/source/unix/sysshell/recently_used_file_handler.cxx b/shell/source/unix/sysshell/recently_used_file_handler.cxx
index 6ca13932143c..ced51eda9787 100644
--- a/shell/source/unix/sysshell/recently_used_file_handler.cxx
+++ b/shell/source/unix/sysshell/recently_used_file_handler.cxx
@@ -32,6 +32,7 @@
#include <i_xml_parser_event_handler.hxx>
#include <map>
+#include <memory>
#include <vector>
#include <algorithm>
#include <string.h>
@@ -194,7 +195,7 @@ namespace /* private */ {
string_container_t groups_;
};
- typedef std::vector<recently_used_item*> recently_used_item_list_t;
+ typedef std::vector<std::unique_ptr<recently_used_item>> recently_used_item_list_t;
typedef void (recently_used_item::* SET_COMMAND)(const string_t&);
// thrown if we encounter xml tags that we do not know
@@ -205,7 +206,6 @@ namespace /* private */ {
{
public:
explicit recently_used_file_filter(recently_used_item_list_t& item_list) :
- item_(nullptr),
item_list_(item_list)
{
named_command_map_[TAG_RECENT_FILES] = &recently_used_item::set_nothing;
@@ -227,7 +227,7 @@ namespace /* private */ {
const xml_tag_attribute_container_t& /*attributes*/) override
{
if ((local_name == TAG_RECENT_ITEM) && (nullptr == item_))
- item_ = new recently_used_item;
+ item_.reset(new recently_used_item);
}
virtual void end_element(const string_t& /*raw_name*/, const string_t& local_name) override
@@ -237,17 +237,16 @@ namespace /* private */ {
return; // will result in an XML parser error anyway
if (named_command_map_.find(local_name) != named_command_map_.end())
- (item_->*named_command_map_[local_name])(current_element_);
+ (item_.get()->*named_command_map_[local_name])(current_element_);
else
{
- delete item_;
+ item_.reset();
throw unknown_xml_format_exception();
}
if (local_name == TAG_RECENT_ITEM)
{
- item_list_.push_back(item_);
- item_ = nullptr;
+ item_list_.push_back(std::move(item_));
}
current_element_.clear();
}
@@ -264,7 +263,7 @@ namespace /* private */ {
virtual void comment(const string_t& /*comment*/) override
{}
private:
- recently_used_item* item_;
+ std::unique_ptr<recently_used_item> item_;
std::map<string_t, SET_COMMAND> named_command_map_;
string_t current_element_;
recently_used_item_list_t& item_list_;
@@ -301,7 +300,7 @@ namespace /* private */ {
items_written_(0)
{}
- void operator() (const recently_used_item* item)
+ void operator() (const std::unique_ptr<recently_used_item> & item)
{
if (items_written_++ < MAX_RECENTLY_USED_ITEMS)
item->write_xml(file_);
@@ -338,23 +337,6 @@ namespace /* private */ {
}
- struct delete_recently_used_item
- {
- void operator() (const recently_used_item* item) const
- { delete item; }
- };
-
-
- void recently_used_item_list_clear(recently_used_item_list_t& item_list)
- {
- std::for_each(
- item_list.begin(),
- item_list.end(),
- delete_recently_used_item());
- item_list.clear();
- }
-
-
class find_item_predicate
{
public:
@@ -362,7 +344,7 @@ namespace /* private */ {
uri_(uri)
{}
- bool operator() (const recently_used_item* item) const
+ bool operator() (const std::unique_ptr<recently_used_item> & item) const
{ return (item->uri_ == uri_); }
private:
string_t uri_;
@@ -371,7 +353,7 @@ namespace /* private */ {
struct greater_recently_used_item
{
- bool operator ()(const recently_used_item* lhs, const recently_used_item* rhs) const
+ bool operator ()(const std::unique_ptr<recently_used_item> & lhs, const std::unique_ptr<recently_used_item> & rhs) const
{ return (lhs->timestamp_ > rhs->timestamp_); }
};
@@ -416,7 +398,7 @@ namespace /* private */ {
if (mimetype.length() == 0)
mimetype = "application/octet-stream";
- item_list.push_back(new recently_used_item(uri, mimetype, groups));
+ item_list.emplace_back(new recently_used_item(uri, mimetype, groups));
}
// sort decreasing after the timestamp
@@ -434,7 +416,7 @@ namespace /* private */ {
item_list_(item_list)
{}
~cleanup_guard()
- { recently_used_item_list_clear(item_list_); }
+ { item_list_.clear(); }
recently_used_item_list_t& item_list_;
};
diff --git a/svx/source/gengal/gengal.cxx b/svx/source/gengal/gengal.cxx
index c4654bd580da..1a218d1d0524 100644
--- a/svx/source/gengal/gengal.cxx
+++ b/svx/source/gengal/gengal.cxx
@@ -56,28 +56,12 @@ protected:
void DeInit() override;
};
-Gallery* createGallery( const OUString& rURL )
-{
- return new Gallery( rURL );
-}
-
-void disposeGallery( Gallery* pGallery )
-{
- delete pGallery;
-}
-
static void createTheme( const OUString& aThemeName, const OUString& aGalleryURL,
const OUString& aDestDir, std::vector<INetURLObject> &rFiles,
bool bRelativeURLs )
{
- Gallery* pGallery;
+ std::unique_ptr<Gallery> pGallery(new Gallery( aGalleryURL ));
- pGallery = createGallery( aGalleryURL );
- if (!pGallery ) {
- fprintf( stderr, "Couldn't create '%s'\n",
- OUStringToOString( aGalleryURL, RTL_TEXTENCODING_UTF8 ).getStr() );
- exit( 1 );
- }
fprintf( stderr, "Work on gallery '%s'\n",
OUStringToOString( aGalleryURL, RTL_TEXTENCODING_UTF8 ).getStr() );
@@ -127,8 +111,6 @@ static void createTheme( const OUString& aThemeName, const OUString& aGalleryURL
}
pGallery->ReleaseTheme( pGalTheme, aListener );
-
- disposeGallery( pGallery );
}
static int PrintHelp()
diff --git a/sw/source/filter/xml/xmlimpit.cxx b/sw/source/filter/xml/xmlimpit.cxx
index 0c9566012c26..d6b1ea06ad44 100644
--- a/sw/source/filter/xml/xmlimpit.cxx
+++ b/sw/source/filter/xml/xmlimpit.cxx
@@ -224,32 +224,24 @@ SvXMLImportItemMapper::finished(SfxItemSet &, SvXMLUnitConverter const&) const
struct BoxHolder
{
- SvxBorderLine* pTop;
- SvxBorderLine* pBottom;
- SvxBorderLine* pLeft;
- SvxBorderLine* pRight;
+ std::unique_ptr<SvxBorderLine> pTop;
+ std::unique_ptr<SvxBorderLine> pBottom;
+ std::unique_ptr<SvxBorderLine> pLeft;
+ std::unique_ptr<SvxBorderLine> pRight;
BoxHolder(BoxHolder const&) = delete;
BoxHolder& operator=(BoxHolder const&) = delete;
explicit BoxHolder(SvxBoxItem const & rBox)
{
- pTop = rBox.GetTop() == nullptr ?
- nullptr : new SvxBorderLine( *rBox.GetTop() );
- pBottom = rBox.GetBottom() == nullptr ?
- nullptr : new SvxBorderLine( *rBox.GetBottom() );
- pLeft = rBox.GetLeft() == nullptr ?
- nullptr : new SvxBorderLine( *rBox.GetLeft() );
- pRight = rBox.GetRight() == nullptr ?
- nullptr : new SvxBorderLine( *rBox.GetRight() );
- }
-
- ~BoxHolder()
- {
- delete pTop;
- delete pBottom;
- delete pLeft;
- delete pRight;
+ if (rBox.GetTop())
+ pTop.reset(new SvxBorderLine( *rBox.GetTop() ));
+ if (rBox.GetBottom())
+ pBottom.reset(new SvxBorderLine( *rBox.GetBottom() ));
+ if (rBox.GetLeft())
+ pLeft.reset(new SvxBorderLine( *rBox.GetLeft() ));
+ if (rBox.GetRight())
+ pRight.reset(new SvxBorderLine( *rBox.GetRight() ));
}
};
@@ -576,10 +568,10 @@ bool SvXMLImportItemMapper::PutXMLValue(
break;
}
- rBox.SetLine( aBoxes.pTop, SvxBoxItemLine::TOP );
- rBox.SetLine( aBoxes.pBottom, SvxBoxItemLine::BOTTOM );
- rBox.SetLine( aBoxes.pLeft, SvxBoxItemLine::LEFT );
- rBox.SetLine( aBoxes.pRight, SvxBoxItemLine::RIGHT );
+ rBox.SetLine( aBoxes.pTop.get(), SvxBoxItemLine::TOP );
+ rBox.SetLine( aBoxes.pBottom.get(), SvxBoxItemLine::BOTTOM );
+ rBox.SetLine( aBoxes.pLeft.get(), SvxBoxItemLine::LEFT );
+ rBox.SetLine( aBoxes.pRight.get(), SvxBoxItemLine::RIGHT );
bOk = true;
}
diff --git a/sw/source/filter/xml/xmlithlp.cxx b/sw/source/filter/xml/xmlithlp.cxx
index 4d7a5f3e09c6..d879292d5aba 100644
--- a/sw/source/filter/xml/xmlithlp.cxx
+++ b/sw/source/filter/xml/xmlithlp.cxx
@@ -140,7 +140,7 @@ void sw_frmitems_setXMLBorderStyle( SvxBorderLine& rLine, sal_uInt16 nStyle )
rLine.SetBorderLineStyle(eStyle);
}
-bool sw_frmitems_setXMLBorder( SvxBorderLine*& rpLine,
+bool sw_frmitems_setXMLBorder( std::unique_ptr<SvxBorderLine>& rpLine,
bool bHasStyle, sal_uInt16 nStyle,
bool bHasWidth, sal_uInt16 nWidth,
sal_uInt16 nNamedWidth,
@@ -151,12 +151,7 @@ bool sw_frmitems_setXMLBorder( SvxBorderLine*& rpLine,
(bHasWidth && USHRT_MAX == nNamedWidth && 0 == nWidth) )
{
bool bRet = nullptr != rpLine;
- if( rpLine )
- {
- delete rpLine;
- rpLine = nullptr;
- }
-
+ rpLine.reset();
return bRet;
}
@@ -166,7 +161,7 @@ bool sw_frmitems_setXMLBorder( SvxBorderLine*& rpLine,
// We now do know that there will be a line
if( !rpLine )
- rpLine = new SvxBorderLine;
+ rpLine.reset(new SvxBorderLine);
if( ( bHasWidth &&
(USHRT_MAX != nNamedWidth || (nWidth != rpLine->GetWidth() ) ) ) ||
@@ -208,12 +203,12 @@ bool sw_frmitems_setXMLBorder( SvxBorderLine*& rpLine,
return true;
}
-void sw_frmitems_setXMLBorder( SvxBorderLine*& rpLine,
+void sw_frmitems_setXMLBorder( std::unique_ptr<SvxBorderLine>& rpLine,
sal_uInt16 nWidth, sal_uInt16 nOutWidth,
sal_uInt16 nInWidth, sal_uInt16 nDistance )
{
if( !rpLine )
- rpLine = new SvxBorderLine;
+ rpLine.reset(new SvxBorderLine);
if( nWidth > 0 )
rpLine->SetWidth( nWidth );
diff --git a/sw/source/filter/xml/xmlithlp.hxx b/sw/source/filter/xml/xmlithlp.hxx
index c3c0512f16b8..e5498009bd3e 100644
--- a/sw/source/filter/xml/xmlithlp.hxx
+++ b/sw/source/filter/xml/xmlithlp.hxx
@@ -40,13 +40,13 @@ bool sw_frmitems_parseXMLBorder( const OUString& rValue,
sal_uInt16& rNamedWidth,
bool& rHasColor, Color& rColor );
-bool sw_frmitems_setXMLBorder( editeng::SvxBorderLine*& rpLine,
+bool sw_frmitems_setXMLBorder( std::unique_ptr<editeng::SvxBorderLine>& rpLine,
bool bHasStyle, sal_uInt16 nStyle,
bool bHasWidth, sal_uInt16 nWidth,
sal_uInt16 nNamedWidth,
bool bHasColor, const Color& rColor );
-void sw_frmitems_setXMLBorder( editeng::SvxBorderLine*& rpLine,
+void sw_frmitems_setXMLBorder( std::unique_ptr<editeng::SvxBorderLine>& rpLine,
sal_uInt16 nWidth, sal_uInt16 nOutWidth,
sal_uInt16 nInWidth, sal_uInt16 nDistance );
diff --git a/tools/source/generic/config.cxx b/tools/source/generic/config.cxx
index 55ce2bdc03b8..169f6e6455aa 100644
--- a/tools/source/generic/config.cxx
+++ b/tools/source/generic/config.cxx
@@ -578,35 +578,27 @@ static void ImplDeleteConfigData( ImplConfigData* pData )
pData->mpFirstGroup = nullptr;
}
-static ImplConfigData* ImplGetConfigData( const OUString& rFileName )
+static std::unique_ptr<ImplConfigData> ImplGetConfigData( const OUString& rFileName )
{
- ImplConfigData* pData;
-
- pData = new ImplConfigData;
+ std::unique_ptr<ImplConfigData> pData(new ImplConfigData);
pData->maFileName = rFileName;
pData->mpFirstGroup = nullptr;
pData->mnDataUpdateId = 0;
pData->meLineEnd = LINEEND_CRLF;
pData->mbRead = false;
pData->mbIsUTF8BOM = false;
- ImplReadConfig( pData );
+ ImplReadConfig( pData.get() );
return pData;
}
-static void ImplFreeConfigData( ImplConfigData* pDelData )
-{
- ImplDeleteConfigData( pDelData );
- delete pDelData;
-}
-
bool Config::ImplUpdateConfig() const
{
// Re-read file if timestamp differs
if ( mpData->mnTimeStamp != ImplSysGetConfigTimeStamp( maFileName ) )
{
- ImplDeleteConfigData( mpData );
- ImplReadConfig( mpData );
+ ImplDeleteConfigData( mpData.get() );
+ ImplReadConfig( mpData.get() );
mpData->mnDataUpdateId++;
return true;
}
@@ -667,7 +659,7 @@ Config::~Config()
SAL_INFO("tools.generic", "Config::~Config()" );
Flush();
- ImplFreeConfigData( mpData );
+ ImplDeleteConfigData( mpData.get() );
}
void Config::SetGroup(const OString& rGroup)
@@ -973,7 +965,7 @@ OString Config::ReadKey(sal_uInt16 nKey) const
void Config::Flush()
{
if ( mpData->mbModified )
- ImplWriteConfig( mpData );
+ ImplWriteConfig( mpData.get() );
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
More information about the Libreoffice-commits
mailing list