Identity: Avoid leak in toString()
AcceptedPublic

Authored by palokisa on Aug 10 2018, 7:48 AM.

Details

Summary

Based on polkit documentation the pointer returned by polkit_identity_to_string() must be g_free-ed.

Diff Detail

Repository
R563 Polkit-1 Qt Library
Lint
Lint Skipped
Unit
Unit Tests Skipped
palokisa requested review of this revision.Aug 10 2018, 7:48 AM
palokisa created this revision.
davidedmundson accepted this revision.Aug 10 2018, 7:52 AM
This revision is now accepted and ready to land.Aug 10 2018, 7:52 AM
palokisa updated this revision to Diff 39386.Aug 10 2018, 7:53 AM

char -> gchar

@davidedmundson
While looking at the implementation here... would you accept using RAII here? Something like:

diff
diff --git a/core/polkitqt1-identity.cpp b/core/polkitqt1-identity.cpp
index 9af489e..90e4a3d 100644
--- a/core/polkitqt1-identity.cpp
+++ b/core/polkitqt1-identity.cpp
@@ -21,6 +21,7 @@
 #include "polkitqt1-identity.h"
 
 #include <polkit/polkit.h>
+#include <memory>
 
 #include <QtCore/QDebug>
 
@@ -30,23 +31,20 @@ namespace PolkitQt1
 class Identity::Data : public QSharedData
 {
 public:
-    Data() : identity(0) {}
+    Data() : identity(nullptr, &g_object_unref) {}
     Data(const Data& other)
         : QSharedData(other)
-        , identity(other.identity)
+        , identity(nullptr, &g_object_unref)
     {
-        if (identity) {
-            g_object_ref(identity);
+        if (other.identity) {
+            identity.reset(static_cast<PolkitIdentity *>(g_object_ref(other.identity.get())));
         }
     }
     ~Data()
     {
-        if (identity) {
-            g_object_unref(identity);
-        }
     }
 
-    PolkitIdentity *identity;
+    std::unique_ptr<PolkitIdentity, void (*)(gpointer)> identity;
 };
 
 Identity::Identity()
@@ -63,10 +61,9 @@ Identity::Identity(PolkitIdentity *polkitIdentity)
 #if !GLIB_CHECK_VERSION(2,36,0)
     g_type_init();
 #endif
-    d->identity = polkitIdentity;
 
-    if (d->identity) {
-        g_object_ref(d->identity);
+    if (polkitIdentity) {
+        d->identity.reset(static_cast<PolkitIdentity *>(g_object_ref(polkitIdentity)));
     }
 }
 
@@ -88,38 +85,28 @@ Identity& Identity::operator=(const PolkitQt1::Identity& other)
 
 bool Identity::isValid() const
 {
-    return d->identity != NULL;
+    return static_cast<bool>(d->identity);
 }
 
 PolkitIdentity *Identity::identity() const
 {
-    return d->identity;
+    return d->identity.get();
 }
 
 void Identity::setIdentity(PolkitIdentity *identity)
 {
-    if (d->identity == identity) {
+    if (d->identity.get() == identity) {
         return;
     }
-
-    if (d->identity) {
-        g_object_unref(d->identity);
-    }
-
-    d->identity = identity;
-
-    if (d->identity) {
-        g_object_ref(d->identity);
-    }
+    d->identity.reset(identity ? static_cast<PolkitIdentity *>(g_object_ref(identity)) : nullptr);
 }
 
 QString Identity::toString() const
 {
     Q_ASSERT(d->identity);
-    gchar * id_str = polkit_identity_to_string(d->identity);
-    const QString result = QString::fromUtf8(id_str);
-    g_free(id_str);
-    return result;
+    // from c++14: const auto id_str = std::make_unique(polkit_identity_to_string(d->identity.get()), &g_free);
+    const std::unique_ptr<gchar, void (*)(void *)> id_str{polkit_identity_to_string(d->identity.get()), &g_free};
+    return QString::fromUtf8(id_str.get());
 }
 
 Identity Identity::fromString(const QString &string)