diff options
| -rw-r--r-- | ChangeLog | 11 | ||||
| -rw-r--r-- | glib/dbus-gvalue-utils.c | 24 |
2 files changed, 25 insertions, 10 deletions
@@ -1,5 +1,16 @@ 2005-05-06 Robert McQueen <robot101@debian.org> + * glib/dbus-gvalue-utils.c: Fix the failing test where static string + pointers were put into a GPtrArray-based specialised collection, and + then freed along with the array. GValues which you add into + collections or maps which have the NOCOPY flag set are assumed to not + belong to the caller, so rather than the existing pointer-stealing + semantics, they are copied instead. Given that the main consumers of + this abstraction are the bindings themselves, I don't think this is + too bad, but others should watch their choice of take vs set_static. + +2005-05-06 Robert McQueen <robot101@debian.org> + * glib/dbus-gvalue-utils.c: Spotted a warning about the return value of g_slist_prepend not being used. Fixed copying of slist-based specialised collections, then wrote a test case and found that it was diff --git a/glib/dbus-gvalue-utils.c b/glib/dbus-gvalue-utils.c index 83e95572..89ff16d9 100644 --- a/glib/dbus-gvalue-utils.c +++ b/glib/dbus-gvalue-utils.c @@ -744,6 +744,20 @@ gvalue_take_ptrarray_value (GValue *value, gpointer instance) static gpointer ptrarray_value_from_gvalue (const GValue *value) { + GValue tmp = {0, }; + + /* if the NOCOPY flag is set, then value was created via set_static and hence + * is not owned by us. in order to preserve the "take" semantics that the API + * has in general (which avoids copying in the common case), we must copy any + * static values so that we can indiscriminately free the entire collection + * later. */ + if (value->data[1].v_uint & G_VALUE_NOCOPY_CONTENTS) + { + g_value_init (&tmp, G_VALUE_TYPE (value)); + g_value_copy (value, &tmp); + value = &tmp; + } + switch (g_type_fundamental (G_VALUE_TYPE (value))) { case G_TYPE_STRING: @@ -1315,17 +1329,7 @@ _dbus_gvalue_utils_test (const char *datadir) g_assert (!strcmp ("bar", g_ptr_array_index (instance, 1))); g_assert (!strcmp ("baz", g_ptr_array_index (instance, 2))); - /* FIXME this crashes, I believe because ptrarray_append - * doesn't copy the incoming static string, then ptrarray_free - * tries to free it; looks to me like always copying appended - * values would be the only working approach. - */ g_value_unset (&val); - /* FIXME make sure this test fails for everyone, since - * apparently people didn't see it, the bad free - * maybe didn't crash everywhere - */ - g_assert_not_reached(); } type = dbus_g_type_get_struct ("GValueArray", G_TYPE_STRING, G_TYPE_UINT, DBUS_TYPE_G_OBJECT_PATH, G_TYPE_INVALID); |