From: Manuel Nickschas <sputnick@quassel-irc.org>
Date: Wed, 5 Sep 2018 18:39:12 +0000 (+0200)
Subject: clang-tidy: Avoid potential memory leak in QtUi
X-Git-Tag: 0.13-rc2~40
X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=f18d6c40cd704e26734a77b371ae2c4a284bee38;ds=sidebyside

clang-tidy: Avoid potential memory leak in QtUi

Clean up the instance handling in QtUi (and GraphicalUi) to ensure
that a newly created QtUi instance is always assigned to the local
_instance variable. Otherwise, clang-tidy warns about a potential
memory leak (although this would not happen in practice).
---

diff --git a/src/qtui/qtui.cpp b/src/qtui/qtui.cpp
index d899ba8e..d335d078 100644
--- a/src/qtui/qtui.cpp
+++ b/src/qtui/qtui.cpp
@@ -40,21 +40,30 @@
 #include "types.h"
 #include "util.h"
 
-QtUi *QtUi::_instance = nullptr;
 MainWin *QtUi::_mainWin = nullptr;
 QList<AbstractNotificationBackend *> QtUi::_notificationBackends;
 QList<AbstractNotificationBackend::Notification> QtUi::_notifications;
 
+namespace {
+
+QtUi *_instance{nullptr};
+
+}
+
+
+QtUi *QtUi::instance()
+{
+    if (!_instance) {
+        _instance = new QtUi();
+    }
+    return _instance;
+}
+
+
 QtUi::QtUi()
     : GraphicalUi()
     , _systemIconTheme{QIcon::themeName()}
 {
-    if (_instance != nullptr) {
-        qWarning() << "QtUi has been instantiated again!";
-        return;
-    }
-    _instance = this;
-
     if (Quassel::isOptionSet("icontheme")) {
         _systemIconTheme = Quassel::optionValue("icontheme");
         QIcon::setThemeName(_systemIconTheme);
diff --git a/src/qtui/qtui.h b/src/qtui/qtui.h
index 907fb31b..f85d2252 100644
--- a/src/qtui/qtui.h
+++ b/src/qtui/qtui.h
@@ -49,13 +49,12 @@ class QtUi : public GraphicalUi
     Q_OBJECT
 
 public:
-    QtUi();
     ~QtUi();
 
     MessageModel *createMessageModel(QObject *parent) override;
     AbstractMessageProcessor *createMessageProcessor(QObject *parent) override;
 
-    inline static QtUi *instance();
+    static QtUi *instance();
     inline static QtUiStyle *style();
     inline static MainWin *mainWindow();
 
@@ -123,13 +122,14 @@ private slots:
     void useSystemTrayChanged(const QVariant &);
 
 private:
+    QtUi();
+
     /**
      * Sets up icon theme handling.
      */
     void setupIconTheme();
 
 private:
-    static QtUi *_instance;
     static MainWin *_mainWin;
     static QList<AbstractNotificationBackend *> _notificationBackends;
     static QList<AbstractNotificationBackend::Notification> _notifications;
@@ -143,7 +143,5 @@ private:
     bool _useSystemTray;
 };
 
-
-QtUi *QtUi::instance() { return _instance ? _instance : new QtUi(); }
 QtUiStyle *QtUi::style() { return qobject_cast<QtUiStyle *>(uiStyle()); }
 MainWin *QtUi::mainWindow() { return _mainWin; }
diff --git a/src/qtui/qtuiapplication.cpp b/src/qtui/qtuiapplication.cpp
index 20c1662e..b59899e3 100644
--- a/src/qtui/qtuiapplication.cpp
+++ b/src/qtui/qtuiapplication.cpp
@@ -110,7 +110,7 @@ void QtUiApplication::init()
         throw ExitException{EXIT_FAILURE, tr("Could not load or upgrade client settings!")};
     }
 
-    Client::init(new QtUi());
+    Client::init(QtUi::instance());
 
     // Init UI only after the event loop has started
     // TODO Qt5: Make this a lambda
diff --git a/src/uisupport/graphicalui.cpp b/src/uisupport/graphicalui.cpp
index c05ed403..fb4a2c7f 100644
--- a/src/uisupport/graphicalui.cpp
+++ b/src/uisupport/graphicalui.cpp
@@ -36,7 +36,6 @@
 #  include <KWindowSystem>
 #endif
 
-GraphicalUi *GraphicalUi::_instance = 0;
 QWidget *GraphicalUi::_mainWidget = 0;
 QHash<QString, ActionCollection *> GraphicalUi::_actionCollections;
 ContextMenuActionProvider *GraphicalUi::_contextMenuActionProvider = 0;
@@ -44,6 +43,18 @@ ToolBarActionProvider *GraphicalUi::_toolBarActionProvider = 0;
 UiStyle *GraphicalUi::_uiStyle = 0;
 bool GraphicalUi::_onAllDesktops = false;
 
+namespace {
+
+GraphicalUi *_instance{nullptr};
+
+}
+
+
+GraphicalUi *GraphicalUi::instance() {
+    return _instance;
+}
+
+
 GraphicalUi::GraphicalUi(QObject *parent) : AbstractUi(parent)
 {
     Q_ASSERT(!_instance);
diff --git a/src/uisupport/graphicalui.h b/src/uisupport/graphicalui.h
index f250e1ab..b7e96bf3 100644
--- a/src/uisupport/graphicalui.h
+++ b/src/uisupport/graphicalui.h
@@ -40,9 +40,6 @@ class GraphicalUi : public AbstractUi
     Q_OBJECT
 
 public:
-    GraphicalUi(QObject *parent = 0);
-    virtual void init();
-
     //! Access global ActionCollections.
     /** These ActionCollections are associated with the main window, i.e. they contain global
     *  actions (and thus, shortcuts). Widgets providing application-wide shortcuts should
@@ -78,6 +75,9 @@ public:
     static bool isMainWidgetVisible();
 
 protected:
+    GraphicalUi(QObject *parent = 0);
+    virtual void init();
+
     //! This is the widget we associate global actions with, typically the main window
     void setMainWidget(QWidget *);
 
@@ -109,9 +109,8 @@ protected slots:
     virtual void disconnectedFromCore();
 
 private:
-    static inline GraphicalUi *instance();
+    static GraphicalUi *instance();
 
-    static GraphicalUi *_instance;
     static QWidget *_mainWidget;
     static QHash<QString, ActionCollection *> _actionCollections;
     static ContextMenuActionProvider *_contextMenuActionProvider;
@@ -130,7 +129,6 @@ private:
 
 // inlines
 
-GraphicalUi *GraphicalUi::instance() { return _instance; }
 ContextMenuActionProvider *GraphicalUi::contextMenuActionProvider() { return _contextMenuActionProvider; }
 ToolBarActionProvider *GraphicalUi::toolBarActionProvider() { return _toolBarActionProvider; }
 UiStyle *GraphicalUi::uiStyle() { return _uiStyle; }