From a735b5e5e92c9d8596fb2bb5d068982d84ae0575 Mon Sep 17 00:00:00 2001
From: Emmanuel Lepage Vallee <elv1313@gmail.com>
Date: Mon, 22 Feb 2016 03:20:36 -0500
Subject: [PATCH] quality: Fix memory leaks
There is still some, but most are fixed.
The Person::Address::d_ptr leak a lot of memory. Fixing it
trigger something in ASAN and assert, this need investigation.
Change-Id: I98b4f18cac7f36a5878f5be6ca17c672d6a3fc72
Tuleap: #414
---
src/categorizedcontactmodel.cpp | 5 ++++-
src/categorizedhistorymodel.cpp | 5 ++++-
src/collectioninterface.cpp | 2 ++
src/collectioninterface.hpp | 9 ++++++++-
src/collectionmediator.h | 1 +
src/collectionmediator.hpp | 6 ++++++
src/collectionmodel.cpp | 11 +++++++++++
src/localbookmarkcollection.cpp | 2 +-
src/media/recordingmodel.cpp | 30 +++++++++++++++++++++++++-----
src/person.cpp | 9 ++++++++-
src/person.h | 3 ++-
src/private/collectionmodel_p.h | 4 +++-
src/profilemodel.cpp | 2 ++
13 files changed, 77 insertions(+), 12 deletions(-)
diff --git a/src/categorizedcontactmodel.cpp b/src/categorizedcontactmodel.cpp
index 87cdbcd6..f4fa3dcb 100644
--- a/src/categorizedcontactmodel.cpp
+++ b/src/categorizedcontactmodel.cpp
@@ -99,7 +99,7 @@ public:
bool m_SortAlphabetical ;
QString m_DefaultCategory ;
bool m_UnreachableHidden;
- SortingCategory::ModelTuple* m_pSortedProxy ;
+ SortingCategory::ModelTuple* m_pSortedProxy {nullptr};
CategorizedContactModel::SortedProxy m_pProxies ;
//Helper
@@ -260,6 +260,9 @@ CategorizedContactModel::~CategorizedContactModel()
foreach(ContactTreeNode* item,d_ptr->m_lCategoryCounter) {
delete item;
}
+
+ if (d_ptr->m_pSortedProxy)
+ delete d_ptr->m_pSortedProxy;
}
CategorizedContactModel& CategorizedContactModel::instance()
diff --git a/src/categorizedhistorymodel.cpp b/src/categorizedhistorymodel.cpp
index a8910074..d3c784d3 100644
--- a/src/categorizedhistorymodel.cpp
+++ b/src/categorizedhistorymodel.cpp
@@ -62,7 +62,7 @@ public:
QVector<HistoryNode*> m_lCategoryCounter ;
QHash<int,HistoryNode*> m_hCategories ;
QHash<QString,HistoryNode*> m_hCategoryByName ;
- SortingCategory::ModelTuple* m_pSortedProxy ;
+ SortingCategory::ModelTuple* m_pSortedProxy {nullptr};
int m_Role ;
QStringList m_lMimes ;
CategorizedHistoryModel::SortedProxy m_pProxies;
@@ -128,6 +128,9 @@ CategorizedHistoryModel::~CategorizedHistoryModel()
delete item;
}
+
+ if (d_ptr->m_pSortedProxy)
+ delete d_ptr->m_pSortedProxy;
}
QHash<int,QByteArray> CategorizedHistoryModel::roleNames() const
diff --git a/src/collectioninterface.cpp b/src/collectioninterface.cpp
index 0e28a025..8cc75c16 100644
--- a/src/collectioninterface.cpp
+++ b/src/collectioninterface.cpp
@@ -80,6 +80,8 @@ void CollectionInterface::init()
///Destructor
CollectionInterface::~CollectionInterface()
{
+ d_ptr->m_fDestruct();
+ delete d_ptr2;
delete d_ptr;
}
diff --git a/src/collectioninterface.hpp b/src/collectioninterface.hpp
index 9060e07a..75a7250c 100644
--- a/src/collectioninterface.hpp
+++ b/src/collectioninterface.hpp
@@ -41,11 +41,15 @@ public:
std::function<bool(ItemBase*)> m_fSave ;
std::function<bool(ItemBase*)> m_fEdit ;
std::function<bool(ItemBase*)> m_fRemove ;
- std::function<int() > m_fSize ;
+ std::function<int() > m_fSize ;
+ std::function<void() > m_fDestruct ;
std::function<CollectionConfigurationInterface*()> m_fConfigurator;
};
+/**
+ * The collection from now on take ownership of the editor
+ */
template<typename T>
CollectionInterface::CollectionInterface(CollectionEditor<T>* editor, CollectionInterface* parent) :
d_ptr(new CollectionInterfacePrivateT())
@@ -72,6 +76,9 @@ d_ptr(new CollectionInterfacePrivateT())
d_ptr->m_fSize = [editor]()->int {
return editor?editor->items().size() : 0;
};
+ d_ptr->m_fDestruct = [editor]() {
+ delete editor;
+ };
}
template<typename T>
diff --git a/src/collectionmediator.h b/src/collectionmediator.h
index 6b45e538..1bd22833 100644
--- a/src/collectionmediator.h
+++ b/src/collectionmediator.h
@@ -39,6 +39,7 @@ template<typename T>
class LIB_EXPORT CollectionMediator {
public:
CollectionMediator(CollectionManagerInterface<T>* parentManager, QAbstractItemModel* m);
+ virtual ~CollectionMediator();
bool addItem (const T* item);
bool removeItem(const T* item);
diff --git a/src/collectionmediator.hpp b/src/collectionmediator.hpp
index d3f90d85..13e89613 100644
--- a/src/collectionmediator.hpp
+++ b/src/collectionmediator.hpp
@@ -34,6 +34,12 @@ CollectionMediator<T>::CollectionMediator(CollectionManagerInterface<T>* parentM
d_ptr->m_pModel = m;
}
+template<typename T>
+CollectionMediator<T>::~CollectionMediator()
+{
+ delete d_ptr;
+}
+
template<typename T>
bool CollectionMediator<T>::addItem(const T* item)
{
diff --git a/src/collectionmodel.cpp b/src/collectionmodel.cpp
index 6c471d8c..8c7bbecc 100644
--- a/src/collectionmodel.cpp
+++ b/src/collectionmodel.cpp
@@ -42,6 +42,17 @@ CollectionModelPrivate::CollectionModelPrivate(CollectionModel* parent) : QObjec
m_pManageableProxy(nullptr),m_NewCollectionMutex(QMutex::Recursive)
{}
+
+CollectionModelPrivate::~CollectionModelPrivate()
+{
+}
+
+CollectionModelPrivate::ProxyItem::~ProxyItem()
+{
+ foreach(ProxyItem* i, m_Children)
+ delete i;
+}
+
CollectionModel::CollectionModel(QObject* parent) : QAbstractTableModel(parent), d_ptr(new CollectionModelPrivate(this))
{
load();
diff --git a/src/localbookmarkcollection.cpp b/src/localbookmarkcollection.cpp
index 1d443ca2..20b41038 100644
--- a/src/localbookmarkcollection.cpp
+++ b/src/localbookmarkcollection.cpp
@@ -80,7 +80,7 @@ public:
constexpr const char LocalBookmarkCollectionPrivate::FILENAME[];
LocalBookmarkCollection::LocalBookmarkCollection(CollectionMediator<ContactMethod>* mediator) :
- CollectionInterface(new LocalBookmarkEditor(mediator))
+ CollectionInterface(new LocalBookmarkEditor(mediator)), d_ptr(new LocalBookmarkCollectionPrivate())
{
load();
}
diff --git a/src/media/recordingmodel.cpp b/src/media/recordingmodel.cpp
index 84d3cebe..fc1eed58 100644
--- a/src/media/recordingmodel.cpp
+++ b/src/media/recordingmodel.cpp
@@ -31,8 +31,9 @@
//DRing
#include "dbus/configurationmanager.h"
-struct RecordingNode
+struct RecordingNode final
{
+ ~RecordingNode();
enum class Type {
TOP_LEVEL,
@@ -52,16 +53,17 @@ struct RecordingNode
};
-class RecordingModelPrivate : public QObject
+class RecordingModelPrivate final : public QObject
{
Q_OBJECT
public:
- RecordingModelPrivate(Media::RecordingModel* parent);
+ explicit RecordingModelPrivate(Media::RecordingModel* parent);
+ ~RecordingModelPrivate();
//Attributes
QVector<RecordingNode*> m_lCategories ;
- RecordingNode* m_pText ;
- RecordingNode* m_pAudioVideo ;
+ RecordingNode* m_pText {nullptr};
+ RecordingNode* m_pAudioVideo {nullptr};
LocalTextRecordingCollection* m_pTextRecordingCollection;
int m_UnreadCount ;
@@ -80,12 +82,30 @@ RecordingNode::RecordingNode(RecordingNode::Type type) :
}
+RecordingNode::~RecordingNode()
+{
+ foreach(RecordingNode* c, m_lChildren)
+ delete c;
+}
+
RecordingModelPrivate::RecordingModelPrivate(Media::RecordingModel* parent) : q_ptr(parent),m_pText(nullptr),
m_pAudioVideo(nullptr)/*,m_pFiles(nullptr)*/
{
}
+RecordingModelPrivate::~RecordingModelPrivate()
+{
+ if (m_pTextRecordingCollection)
+ delete m_pTextRecordingCollection;
+
+ if (m_pText)
+ delete m_pText;
+
+ if (m_pAudioVideo)
+ delete m_pAudioVideo;
+}
+
void RecordingModelPrivate::forwardInsertion(const QMap<QString,QString>& message, ContactMethod* cm, Media::Media::Direction direction)
{
Q_UNUSED(message)
diff --git a/src/person.cpp b/src/person.cpp
index 88c85bfc..37a7dcce 100644
--- a/src/person.cpp
+++ b/src/person.cpp
@@ -41,9 +41,11 @@
#include "media/textrecording.h"
#include "mime.h"
-class AddressPrivate
+class AddressPrivate final
{
public:
+ ~AddressPrivate() {}
+
QString addressLine;
QString city;
QString zipCode;
@@ -57,6 +59,11 @@ Person::Address::Address() : d_ptr(new AddressPrivate())
}
+Person::Address::~Address()
+{
+ //delete d_ptr; //FIXME ASAN doesn't like for some reasons, but also report a leak
+}
+
QString Person::Address::addressLine() const
{
return d_ptr->addressLine;
diff --git a/src/person.h b/src/person.h
index 71263fc1..ea236a75 100644
--- a/src/person.h
+++ b/src/person.h
@@ -69,7 +69,8 @@ public:
///Represent the physical address of a contact
class Address {
public:
- Address();
+ explicit Address();
+ virtual ~Address();
//Getters
QString addressLine() const;
diff --git a/src/private/collectionmodel_p.h b/src/private/collectionmodel_p.h
index e916ad69..261f2f3f 100644
--- a/src/private/collectionmodel_p.h
+++ b/src/private/collectionmodel_p.h
@@ -35,7 +35,8 @@ class CollectionModelPrivate final : public QObject
{
Q_OBJECT
public:
- CollectionModelPrivate(CollectionModel* parent);
+ explicit CollectionModelPrivate(CollectionModel* parent);
+ virtual ~CollectionModelPrivate();
/*
* This is not very efficient, it doesn't really have to be given the low
@@ -44,6 +45,7 @@ public:
*/
struct ProxyItem {
ProxyItem() : parent(nullptr),col(0),row(0),collection(nullptr),manageableCount(0){}
+ virtual ~ProxyItem();
int row;
int col;
CollectionInterface* collection;
diff --git a/src/profilemodel.cpp b/src/profilemodel.cpp
index c7409b1d..69ff8cc5 100644
--- a/src/profilemodel.cpp
+++ b/src/profilemodel.cpp
@@ -124,6 +124,8 @@ struct Node final
virtual ~Node() {
QObject::disconnect(m_ChangedConn);
+ foreach( Node* n, children)
+ delete n;
}
enum class Type : bool {
--
GitLab