Merge lp:~michihenning/online-accounts-api/leak-fix into lp:online-accounts-api

[Michi Henning]

Fixed memory leak in PendingCallWatcher.
Got rid of compiler warning.
Added missing \endlist to documentation.

Fixed memory leak in PendingCallWatcher.
Got rid of compiler warning.
Added missing \endlist to documentation.

This would not have happened with std::unique_ptr or QScopedPointer. I strongly recommend to never use raw pointers. As this particular example demonstrates, it is easy to get even the most simple case wrong. It took me considerable time to track this down; much better to not allow this type of error to ever arise in the first place.

[Alberto Mardegan (mardy)]

Hi Michi! I still do prefer an explicit delete, when the lifetime of the object is as simple as in this case (and yet I forgot it! :-) ).
If you still decide to go for the QScopedPointer, please add its include file to pending_call.h (now it works even without it, but compilation might break on a future Qt release).

review: Needs Fixing

[lp:~michihenning/online-accounts-api/leak-fix]

44. By Michi Henning on 2016-12-21

Added include for QScopedPointer.

[Michi Henning (michihenning)]

Well, if this doesn't convince you, I don't know what will. This has cost us quite a few hours. And, by using QScopedPointer or unique_ptr, this bug could never have happened in the first place.

I cannot for the life of me see what writing the code with a raw pointer and an explicit delete is preferable when, with a smart pointer, no code is needed at all.

[Alberto Mardegan (mardy)]

It's just that old habits are hard to die, but hey, I'm trying to change :-)

review: Approve

[Michi Henning (michihenning)]

The bug was introduced here: https://code.launchpad.net/~mardy/online-accounts-api/style/+merge/249168

[Michi Henning (michihenning)]

Mardy, I don't know the correct process for this project. I take it that you will merge and release this?

=== modified file 'src/lib/OnlineAccounts/pending_call.h'
--- src/lib/OnlineAccounts/pending_call.h	2015-02-27 10:28:50 +0000
+++ src/lib/OnlineAccounts/pending_call.h	2016-12-21 07:21:59 +0000
@@ -23,6 +23,7 @@
 
 #include <QExplicitlySharedDataPointer>
 #include <QObject>
+#include <QScopedPointer>
 
 #include "global.h"
 
@@ -69,7 +70,7 @@
 
 private:
     Q_DECLARE_PRIVATE(PendingCallWatcher)
-    PendingCallWatcherPrivate *d_ptr;
+    QScopedPointer<PendingCallWatcherPrivate> d_ptr;
 };
 
 } // namespace
 
=== modified file 'src/lib/OnlineAccountsDaemon/i18n.cpp'
--- src/lib/OnlineAccountsDaemon/i18n.cpp	2016-11-07 08:01:24 +0000
+++ src/lib/OnlineAccountsDaemon/i18n.cpp	2016-12-21 07:21:59 +0000
@@ -31,4 +31,4 @@
                                       text.toUtf8().constData()));
 }
 
-}; // namespace
+}  // namespace
 
=== modified file 'src/lib/Ubuntu/OnlineAccounts.2/account.cpp'
--- src/lib/Ubuntu/OnlineAccounts.2/account.cpp	2016-11-07 13:19:06 +0000
+++ src/lib/Ubuntu/OnlineAccounts.2/account.cpp	2016-12-21 07:21:59 +0000
@@ -205,6 +205,7 @@
  *   \li \c displayName - the localized display name for the service
  *   \li \c iconSource - URL for the icon; can be a "file://" URL to a local
  *   file, or an icon from the theme if the URL starts with "image://theme/"
+ * \endlist
  */
 QJSValue Account::service() const
 {