This is a multi-part message in MIME format.

--nextPart8547321.KRI46fXxXN
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"

Le mardi 22 octobre 2013, 14:27:02 David Faure a =E9crit :
> On Tuesday 22 October 2013 13:14:06 Aur=E9lien G=E2teau wrote:
> > Using QSharedPointer is not a problem
>=20
> 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 th=
e
> QSharedPointer, so the refcount is always 1, isn't it?

I blindly followed the TODO comments and turned the pointers into=20
QSharedPointer, but that leads to a double delete since Qt plugin syste=
m=20
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 t=
o
> > create
> > a  situation where the default implementations are deleted at exit =
but not
> > the implementations provided by the plugin?
>=20
> Yep. How is that a problem? The plugin deletes its own instances.
> (see
> staging/frameworkintegration/src/integrationplugin/frameworkintegrati=
onplugi
> n.h they are member vars of the plugin)

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

Aur=E9lien
--nextPart8547321.KRI46fXxXN
Content-Disposition: attachment; filename="kmessagebox-qglobalstatic.diff"
Content-Transfer-Encoding: 7Bit
Content-Type: text/x-patch; charset="UTF-8"; name="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;
 }
 

--nextPart8547321.KRI46fXxXN--