[pygobject] fix marshaling of arrays of GVariants

[Martin Pitt <martinpitt src gnome org>]

commit c7aa0e79dfb4c1092c51ae1464b8414083b4f3fc
Author: Mikkel Kamstrup Erlandsen <mikkel kamstrup canonical com>
Date:   Tue Oct 4 12:28:26 2011 +0200

    fix marshaling of arrays of GVariants
    
    Add unit tests for marshaling of arrays of variants with all
    transfer modes. Requires latest gobject-introspection.
    
    Plug potential leaks of GArray data members
    
    Fix calling of wrong cleanup_from_py for arrays
    
    Simplify and fix logic for cleaning up arrays both in from_py()
    and to_py() code paths.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=638915
    
    Signed-off-by: Martin Pitt <martin pitt ubuntu com>

 gi/pygi-cache.c           |    2 +-
 gi/pygi-marshal-cleanup.c |   81 +++++++++++++++++++++++++++++----------------
 gi/pygi-marshal-from-py.c |   10 ++++-
 gi/pygi-marshal-to-py.c   |   13 ++++++-
 tests/test_gi.py          |   14 +++++++-
 5 files changed, 85 insertions(+), 35 deletions(-)
---
diff --git a/gi/pygi-cache.c b/gi/pygi-cache.c
index 41ca32c..5dc811a 100644
--- a/gi/pygi-cache.c
+++ b/gi/pygi-cache.c
@@ -498,7 +498,7 @@ _arg_cache_from_py_array_setup (PyGIArgCache *arg_cache,
         callable_cache->args_cache[seq_cache->len_arg_index] = child_cache;
     }
 
-    arg_cache->from_py_cleanup = _pygi_marshal_cleanup_to_py_array;
+    arg_cache->from_py_cleanup = _pygi_marshal_cleanup_from_py_array;
 
     return TRUE;
 }
diff --git a/gi/pygi-marshal-cleanup.c b/gi/pygi-marshal-cleanup.c
index 8ed9bdb..f80ebfa 100644
--- a/gi/pygi-marshal-cleanup.c
+++ b/gi/pygi-marshal-cleanup.c
@@ -273,6 +273,37 @@ _pygi_marshal_cleanup_to_py_interface_struct_foreign (PyGIInvokeState *state,
             data);
 }
 
+static GArray*
+_wrap_c_array (PyGIInvokeState   *state,
+               PyGISequenceCache *sequence_cache,
+               gpointer           data)
+{
+    GArray *array_;
+    gsize   len;
+  
+    if (sequence_cache->fixed_size >= 0) {
+        len = sequence_cache->fixed_size;
+    } else if (sequence_cache->is_zero_terminated) {
+        len = g_strv_length ((gchar **)data);
+    } else {
+        GIArgument *len_arg = state->args[sequence_cache->len_arg_index];
+        len = len_arg->v_long;
+    }
+
+    array_ = g_array_new (FALSE,
+                          FALSE,
+                          sequence_cache->item_size);
+
+    if (array_ == NULL)
+        return NULL;
+
+    g_free (array_->data);
+    array_->data = data;
+    array_->len = len;
+
+    return array_;
+}
+
 void
 _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
                                      PyGIArgCache    *arg_cache,
@@ -286,26 +317,11 @@ _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
         /* If this isn't a garray create one to help process variable sized
            array elements */
         if (sequence_cache->array_type == GI_ARRAY_TYPE_C) {
-            gsize len;
-            if (sequence_cache->fixed_size >= 0) {
-                len = sequence_cache->fixed_size;
-            } else if (sequence_cache->is_zero_terminated) {
-                len = g_strv_length ((gchar **)data);
-            } else {
-                GIArgument *len_arg = state->args[sequence_cache->len_arg_index];
-                len = len_arg->v_long;
-            }
-
-            array_ = g_array_new (FALSE,
-                                  FALSE,
-                                  sequence_cache->item_size);
-
+            array_ = _wrap_c_array (state, sequence_cache, data);
+            
             if (array_ == NULL)
                 return;
-
-            array_->data = data;
-            array_->len = len;
-
+            
         } else {
             array_ = (GArray *) data;
         }
@@ -324,12 +340,12 @@ _pygi_marshal_cleanup_from_py_array (PyGIInvokeState *state,
             }
         }
 
-        if (state->failed ||
-            arg_cache->transfer == GI_TRANSFER_NOTHING ||
-            arg_cache->transfer == GI_TRANSFER_CONTAINER) {
+        /* Only free the array when we didn't transfer ownership */
+        if (sequence_cache->array_type == GI_ARRAY_TYPE_C) {
+            g_array_free (array_, arg_cache->transfer == GI_TRANSFER_NOTHING);
+        } else if (state->failed ||
+                   arg_cache->transfer == GI_TRANSFER_NOTHING) {
             g_array_free (array_, TRUE);
-        } else if (sequence_cache->array_type == GI_ARRAY_TYPE_C) {
-            g_array_free (array_, FALSE);
         }
     }
 }
@@ -343,12 +359,20 @@ _pygi_marshal_cleanup_to_py_array (PyGIInvokeState *state,
     PyGISequenceCache *sequence_cache = (PyGISequenceCache *)arg_cache;
 
     if (arg_cache->transfer == GI_TRANSFER_EVERYTHING ||
-            arg_cache->transfer == GI_TRANSFER_CONTAINER) {
-        GArray *array_ = (GArray *) data;
+        arg_cache->transfer == GI_TRANSFER_CONTAINER) {
+        GArray *array_;
+        PyGISequenceCache *sequence_cache = (PyGISequenceCache *)arg_cache;
 
+        /* If this isn't a garray create one to help process variable sized
+           array elements */
         if (sequence_cache->array_type == GI_ARRAY_TYPE_C) {
-            g_free (data);
-            return;
+            array_ = _wrap_c_array (state, sequence_cache, data);
+            
+            if (array_ == NULL)
+                return;
+            
+        } else {
+            array_ = (GArray *) data;
         }
 
         if (sequence_cache->item_cache->to_py_cleanup != NULL) {
@@ -363,8 +387,7 @@ _pygi_marshal_cleanup_to_py_array (PyGIInvokeState *state,
             }
         }
 
-        if (arg_cache->transfer == GI_TRANSFER_EVERYTHING)
-            g_array_free (array_, TRUE);
+        g_array_free (array_, TRUE);
     }
 }
 
diff --git a/gi/pygi-marshal-from-py.c b/gi/pygi-marshal-from-py.c
index 1f807b4..0a94ffe 100644
--- a/gi/pygi-marshal-from-py.c
+++ b/gi/pygi-marshal-from-py.c
@@ -831,8 +831,14 @@ _pygi_marshal_from_py_array (PyGIInvokeState   *state,
                     PyGIMarshalCleanupFunc from_py_cleanup = item_arg_cache->from_py_cleanup;
                     gboolean is_boxed = g_type_is_a (item_iface_cache->g_type, G_TYPE_BOXED);
                     gboolean is_gvalue = item_iface_cache->g_type == G_TYPE_VALUE;
-
-                    if (!is_boxed || is_gvalue) {
+                    gboolean is_gvariant = item_iface_cache->g_type == G_TYPE_VARIANT;
+                    
+                    if (is_gvariant) {
+                        /* Item size will always be that of a pointer,
+                         * since GVariants are opaque hence always passed by ref */
+                        g_assert (item_size == sizeof (item.v_pointer));
+                        g_array_insert_val (array_, i, item.v_pointer);
+                    } else if (!is_boxed || is_gvalue) {
                         memcpy (array_->data + (i * item_size), item.v_pointer, item_size);
                         if (from_py_cleanup)
                             from_py_cleanup (state, item_arg_cache, item.v_pointer, TRUE);
diff --git a/gi/pygi-marshal-to-py.c b/gi/pygi-marshal-to-py.c
index 48dfa08..984e7c1 100644
--- a/gi/pygi-marshal-to-py.c
+++ b/gi/pygi-marshal-to-py.c
@@ -292,7 +292,8 @@ _pygi_marshal_to_py_array (PyGIInvokeState   *state,
 
             return NULL;
         }
-
+        
+        g_free (array_->data);
         array_->data = arg->v_pointer;
         array_->len = len;
     }
@@ -331,10 +332,18 @@ _pygi_marshal_to_py_array (PyGIInvokeState   *state,
                     item_arg.v_pointer = g_ptr_array_index ( ( GPtrArray *)array_, i);
                 } else if (item_arg_cache->type_tag == GI_TYPE_TAG_INTERFACE) {
                     PyGIInterfaceCache *iface_cache = (PyGIInterfaceCache *) item_arg_cache;
+                    gboolean is_gvariant = iface_cache->g_type == G_TYPE_VARIANT;
 
+                    // FIXME: This probably doesn't work with boxed types or gvalues. See fx. _pygi_marshal_from_py_array()
                     switch (g_base_info_get_type (iface_cache->interface_info)) {
                         case GI_INFO_TYPE_STRUCT:
-                            if (arg_cache->transfer == GI_TRANSFER_EVERYTHING) {
+                            if (is_gvariant) {
+                              g_assert (item_size == sizeof (gpointer));
+                              if (arg_cache->transfer == GI_TRANSFER_EVERYTHING)
+                                item_arg.v_pointer = g_variant_ref_sink (g_array_index (array_, gpointer, i));
+                              else
+                                item_arg.v_pointer = g_array_index (array_, gpointer, i);
+                            } else if (arg_cache->transfer == GI_TRANSFER_EVERYTHING) {
                                 gpointer *_struct = g_malloc (item_size);
                                 memcpy (_struct, array_->data + i * item_size,
                                         item_size);
diff --git a/tests/test_gi.py b/tests/test_gi.py
index dd91cb8..11ee2a4 100644
--- a/tests/test_gi.py
+++ b/tests/test_gi.py
@@ -10,7 +10,7 @@ import shutil
 import os
 import locale
 import subprocess
-from gi.repository import GObject
+from gi.repository import GObject, GLib
 
 from gi.repository import GIMarshallingTests
 
@@ -769,6 +769,18 @@ class TestArray(unittest.TestCase):
 
     def test_gstrv_inout(self):
         self.assertEquals(['-1', '0', '1', '2'], GIMarshallingTests.gstrv_inout(['0', '1', '2']))
+    
+    def test_array_gvariant_none_in(self):
+        v = [GLib.Variant("i", 27), GLib.Variant("s", "Hello")]
+        self.assertEquals([27, "Hello"], map(GLib.Variant.unpack, GIMarshallingTests.array_gvariant_none_in(v)))
+    
+    def test_array_gvariant_container_in(self):
+        v = [GLib.Variant("i", 27), GLib.Variant("s", "Hello")]
+        self.assertEquals([27, "Hello"], map(GLib.Variant.unpack, GIMarshallingTests.array_gvariant_none_in(v)))
+    
+    def test_array_gvariant_full_in(self):
+        v = [GLib.Variant("i", 27), GLib.Variant("s", "Hello")]
+        self.assertEquals([27, "Hello"], map(GLib.Variant.unpack, GIMarshallingTests.array_gvariant_none_in(v)))
 
 class TestGArray(unittest.TestCase):