Fix memleaks in GVariant visitor 04/36204/2
authorPiotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
Tue, 3 Mar 2015 11:24:50 +0000 (12:24 +0100)
committerPiotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
Tue, 3 Mar 2015 14:08:31 +0000 (15:08 +0100)
[Bug/Feature]   There were mem leaks in two cases:
                - during processing union fields
                - when invalid field type exception was thrown
[Cause]         N/A
[Solution]      N/A
[Verification]  Run tests under valgrind

Change-Id: I9ddce15a89b151301549dde0056cf1357c50b47c

src/config/from-gvariant-visitor.hpp

index ba7dc7e..4a0330a 100644 (file)
@@ -31,6 +31,7 @@
 
 #include <string>
 #include <vector>
+#include <memory>
 #include <cassert>
 #include <glib.h>
 
@@ -60,26 +61,20 @@ public:
     template<typename T>
     void visit(const std::string& name, T& value)
     {
-        GVariant* child = g_variant_iter_next_value(mIter);
-        if (child == NULL) {
+        auto child = makeUnique(g_variant_iter_next_value(mIter));
+        if (!child) {
             throw config::ConfigException(
                 "GVariant doesn't match with config. Can't set  '" + name + "'");
         }
-        fromGVariant(child, value);
-        g_variant_unref(child);
+        fromGVariant(child.get(), value);
     }
 
 private:
     GVariantIter* mIter;
 
-    explicit FromGVariantVisitor(GVariant* variant, bool isUnion)
+    static std::unique_ptr<GVariant, decltype(&g_variant_unref)> makeUnique(GVariant* variant)
     {
-        if (isUnion) {
-            checkType(variant, G_VARIANT_TYPE_VARIANT);
-            variant = g_variant_get_variant(variant);
-        }
-        checkType(variant, G_VARIANT_TYPE_TUPLE);
-        mIter = g_variant_iter_new(variant);
+        return std::unique_ptr<GVariant, decltype(&g_variant_unref)>(variant, g_variant_unref);
     }
 
     static void checkType(GVariant* object, const GVariantType* type)
@@ -128,18 +123,28 @@ private:
         int length = g_variant_iter_n_children(&iter);
         value.resize(static_cast<size_t>(length));
         for (int i = 0; i < length; ++i) {
-            GVariant* child = g_variant_iter_next_value(&iter);
-            assert(child != NULL);
-            fromGVariant(child, value[static_cast<size_t>(i)]);
-            g_variant_unref(child);
+            auto child = makeUnique(g_variant_iter_next_value(&iter));
+            assert(child);
+            fromGVariant(child.get(), value[static_cast<size_t>(i)]);
         }
     }
 
     template<typename T>
-    static typename std::enable_if<isVisitable<T>::value>::type
+    static typename std::enable_if<isUnion<T>::value>::type
+    fromGVariant(GVariant* object, T& value)
+    {
+        checkType(object, G_VARIANT_TYPE_VARIANT);
+        auto inner = makeUnique(g_variant_get_variant(object));
+
+        FromGVariantVisitor visitor(inner.get());
+        value.accept(visitor);
+    }
+
+    template<typename T>
+    static typename std::enable_if<isVisitable<T>::value && !isUnion<T>::value>::type
     fromGVariant(GVariant* object, T& value)
     {
-        FromGVariantVisitor visitor(object, isUnion<T>::value);
+        FromGVariantVisitor visitor(object);
         value.accept(visitor);
     }