[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: [kdelibs/frameworks] tier1/kwidgetsaddons/src: Introduce KMessageBoxNotifyInterface
From:       Aurélien_Gâteau <agateau () kde ! org>
Date:       2013-10-22 11:14:06
Message-ID: 2352848.pedz9smdPf () trinity
[Download RAW message or body]

Le lundi 21 octobre 2013, 18:35:39 David Faure a écrit :
> On Monday 21 October 2013 15:56:35 Aurélien Gâteau wrote:
> >      // TODO use Qt-5.1's Q_GLOBAL_STATIC
> >      if (!s_dontAskAgainInterface) {
> >      
> >          s_dontAskAgainInterface = new
> >          KMessageBoxDontAskAgainMemoryStorage;
> > 
> > }
> > +    if (!s_notifyInterface) {
> > +        s_notifyInterface = new KMessageBoxNotifyDummy;
> > +    }
> 
> Could you, while you're there, implement the TODO for both instances?
> (to avoid memory leak reports)

Looking at it, see attached patch.

Using QSharedPointer is not a problem, but I am not sure we want to use 
Q_GLOBAL_STATIC for the default implementations: isn't this going to create a 
situation where the default implementations are deleted at exit but not the 
implementations provided by the plugin?

Aurélien
["kmessagebox-qsharedpointer.diff" (kmessagebox-qsharedpointer.diff)]

diff --git a/tier1/kwidgetsaddons/src/kmessagebox_p.cpp \
b/tier1/kwidgetsaddons/src/kmessagebox_p.cpp index b4ecb3e..1186340 100644
--- a/tier1/kwidgetsaddons/src/kmessagebox_p.cpp
+++ b/tier1/kwidgetsaddons/src/kmessagebox_p.cpp
@@ -21,6 +21,7 @@
 #include "kmessagebox_p.h"
 
 #include <QPluginLoader>
+#include <QSharedPointer>
 #include <QVariant>
 
 namespace KMessageBox {
@@ -67,9 +68,8 @@ public:
     void sendNotification(QMessageBox::Icon /*notificationType*/, const QString \
&/*message*/, QWidget */*parent*/) {}  };
 
-// TODO should we use QSharedPointer here?
-static KMessageBoxDontAskAgainInterface* s_dontAskAgainInterface = 0;
-static KMessageBoxNotifyInterface* s_notifyInterface = 0;
+static QSharedPointer<KMessageBoxDontAskAgainInterface> s_dontAskAgainInterface;
+static QSharedPointer<KMessageBoxNotifyInterface> s_notifyInterface;
 
 static void loadKMessageBoxPlugin()
 {
@@ -80,16 +80,20 @@ static void loadKMessageBoxPlugin()
         QPluginLoader lib(QStringLiteral("kf5/FrameworkIntegrationPlugin"));
         QObject *rootObj = lib.instance();
         if (rootObj) {
-            s_dontAskAgainInterface = \
rootObj->property(KMESSAGEBOXDONTASKAGAIN_PROPERTY).value<KMessageBoxDontAskAgainInterface \
                *>();
-            s_notifyInterface = \
rootObj->property(KMESSAGEBOXNOTIFY_PROPERTY).value<KMessageBoxNotifyInterface *>(); \
+            s_dontAskAgainInterface.reset( +                \
rootObj->property(KMESSAGEBOXDONTASKAGAIN_PROPERTY).value<KMessageBoxDontAskAgainInterface \
*>() +                );
+            s_notifyInterface.reset(
+                rootObj->property(KMESSAGEBOXNOTIFY_PROPERTY).value<KMessageBoxNotifyInterface \
*>() +                );
         }
     }
     // TODO use Qt-5.1's Q_GLOBAL_STATIC
     if (!s_dontAskAgainInterface) {
-        s_dontAskAgainInterface = new KMessageBoxDontAskAgainMemoryStorage;
+        s_dontAskAgainInterface.reset(new KMessageBoxDontAskAgainMemoryStorage);
     }
     if (!s_notifyInterface) {
-        s_notifyInterface = new KMessageBoxNotifyDummy;
+        s_notifyInterface.reset(new KMessageBoxNotifyDummy);
     }
 }
 
@@ -98,7 +102,7 @@ KMessageBoxDontAskAgainInterface *dontAskAgainInterface()
     if (!s_dontAskAgainInterface) {
         loadKMessageBoxPlugin();
     }
-    return s_dontAskAgainInterface;
+    return s_dontAskAgainInterface.data();
 }
 
 KMessageBoxNotifyInterface *notifyInterface()
@@ -106,20 +110,19 @@ KMessageBoxNotifyInterface *notifyInterface()
     if (!s_notifyInterface) {
         loadKMessageBoxPlugin();
     }
-    return s_notifyInterface;
+    return s_notifyInterface.data();
 }
 
 void setDontShowAgainInterface(KMessageBoxDontAskAgainInterface* \
dontAskAgainInterface)  {
     Q_ASSERT(dontAskAgainInterface != 0);
-    // FIXME should we delete s_dontAskAgainInterface before? Or perhaps use smart \
                pointers to avoid problems?
-    s_dontAskAgainInterface = dontAskAgainInterface;
+    s_dontAskAgainInterface.reset(dontAskAgainInterface);
 }
 
 void setNotifyInterface(KMessageBoxNotifyInterface *notifyInterface)
 {
     Q_ASSERT(notifyInterface != 0);
-    s_notifyInterface = notifyInterface;
+    s_notifyInterface.reset(notifyInterface);
 }
 
 } // KMessageBox namespace



[prev in list] [next in list] [prev in thread] [next in thread]