[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 13:13:23
Message-ID: 6157905.glXrWvC1ZZ () trinity
[Download RAW message or body]

Le mardi 22 octobre 2013, 14:27:02 David Faure a écrit :
> On Tuesday 22 October 2013 13:14:06 Aurélien Gâteau wrote:
> > Using QSharedPointer is not a problem
> 
> I don't understand the use of QSharedPointer here. That's a reference count.
> But what are we counting? This class is the only one which can see the
> QSharedPointer, so the refcount is always 1, isn't it?

I blindly followed the TODO comments and turned the pointers into 
QSharedPointer, but that leads to a double delete since Qt plugin system 
deletes its root object itself.

> > 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?
> 
> Yep. How is that a problem? The plugin deletes its own instances.
> (see
> staging/frameworkintegration/src/integrationplugin/frameworkintegrationplugi
> n.h they are member vars of the plugin)

I assumed the Qt plugin system did not delete its root object itself, but that 
was wrong. See updated diff. Maybe we should also document the fact that he 
set*Interface() methods do not take ownership of the implementation objects?

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

diff --git a/tier1/kwidgetsaddons/src/kmessagebox_p.cpp \
b/tier1/kwidgetsaddons/src/kmessagebox_p.cpp index b4ecb3e..c0bcf80 100644
--- a/tier1/kwidgetsaddons/src/kmessagebox_p.cpp
+++ b/tier1/kwidgetsaddons/src/kmessagebox_p.cpp
@@ -67,7 +67,9 @@ public:
     void sendNotification(QMessageBox::Icon /*notificationType*/, const QString \
&/*message*/, QWidget */*parent*/) {}  };
 
-// TODO should we use QSharedPointer here?
+Q_GLOBAL_STATIC(KMessageBoxDontAskAgainMemoryStorage, \
s_defaultDontAskAgainInterface); +Q_GLOBAL_STATIC(KMessageBoxNotifyDummy, \
s_defaultNotifyInterface); +
 static KMessageBoxDontAskAgainInterface* s_dontAskAgainInterface = 0;
 static KMessageBoxNotifyInterface* s_notifyInterface = 0;
 
@@ -84,12 +86,11 @@ static void loadKMessageBoxPlugin()
             s_notifyInterface = \
rootObj->property(KMESSAGEBOXNOTIFY_PROPERTY).value<KMessageBoxNotifyInterface *>();  \
}  }
-    // TODO use Qt-5.1's Q_GLOBAL_STATIC
     if (!s_dontAskAgainInterface) {
-        s_dontAskAgainInterface = new KMessageBoxDontAskAgainMemoryStorage;
+        s_dontAskAgainInterface = s_defaultDontAskAgainInterface;
     }
     if (!s_notifyInterface) {
-        s_notifyInterface = new KMessageBoxNotifyDummy;
+        s_notifyInterface = s_defaultNotifyInterface;
     }
 }
 
@@ -112,7 +113,6 @@ KMessageBoxNotifyInterface *notifyInterface()
 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;
 }
 



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