Commit c7aa0e79 authored by Mikkel Kamstrup Erlandsen's avatar Mikkel Kamstrup Erlandsen Committed by Martin Pitt
Browse files

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's avatarMartin Pitt <martin.pitt@ubuntu.com>
parent c2ec4d8e
......@@ -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;
}
......
......@@ -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);
}
}
......
......@@ -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);
......
......@@ -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);
......
......@@ -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):
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment