This is an automated email from the git hooks/post-receive script. a l i p u s h e d a c o m m i t t o b r a n c h m a s t e r in repository xfce/xfconf.
commit 9db6659f309d68e615f1141fd334bfc03733adfe Author: Ali Abdallah <ali...@gmail.com> Date: Fri May 27 16:27:05 2016 +0200 Fix many issues with libxfconf. GPtrArray is now duplicated and saved in the cache, so the user can free the return array value. --- common/xfconf-gvaluefuncs.c | 35 +++++++-- common/xfconf-gvaluefuncs.h | 2 + xfconf/xfconf-cache.c | 168 +++++++++++++++++++++++++++++++------------- xfconf/xfconf-channel.c | 117 +++++------------------------- 4 files changed, 166 insertions(+), 156 deletions(-) diff --git a/common/xfconf-gvaluefuncs.c b/common/xfconf-gvaluefuncs.c index b20424c..dcd3163 100644 --- a/common/xfconf-gvaluefuncs.c +++ b/common/xfconf-gvaluefuncs.c @@ -561,23 +561,48 @@ GVariant *xfconf_hash_to_gvariant (GHashTable *hash) } } } - + variant = g_variant_builder_end (&builder); return variant; } -static void _destroy_array_elem (gpointer data) { +static void xfonf_free_array_elem_val (gpointer data) { GValue *val = (GValue*)data; g_value_unset (val); g_free (val); } + +GPtrArray * +xfconf_dup_value_array (GPtrArray *arr, gboolean auto_destroy_value) { + + GPtrArray *retArr; + uint i; + + if (auto_destroy_value) + retArr = g_ptr_array_new_full(arr->len, (GDestroyNotify)xfonf_free_array_elem_val); + else + retArr = g_ptr_array_sized_new(arr->len); + + for (i = 0; i< arr->len; i++) { + GValue *v, *vi; + v = g_new0(GValue, 1); + vi = g_ptr_array_index(arr, i); + g_value_init (v, G_VALUE_TYPE(vi)); + g_value_copy (vi, v); + g_ptr_array_add(retArr, v); + } + + return retArr; +} + + GValue * xfconf_gvariant_to_gvalue (GVariant *in_variant) { GValue *value; GVariant *variant; value = g_new0(GValue, 1); - + if (g_variant_is_of_type(in_variant, G_VARIANT_TYPE ("v"))) variant = g_variant_get_variant (in_variant); else @@ -592,7 +617,7 @@ GValue * xfconf_gvariant_to_gvalue (GVariant *in_variant) nchild = g_variant_n_children (variant); if (nchild > 0) { - arr = g_ptr_array_new_full(nchild, (GDestroyNotify)_destroy_array_elem); + arr = g_ptr_array_new_full(nchild, (GDestroyNotify)xfonf_free_array_elem_val); while (idx < nchild ) { GVariant *v; @@ -639,7 +664,7 @@ GHashTable *xfconf_gvariant_to_hash (GVariant *variant) g_variant_iter_init (&iter, variant); - while (g_variant_iter_next (&iter, "{&sv}", &key, &v)) { + while (g_variant_iter_next (&iter, "{sv}", &key, &v)) { GValue *value; value = xfconf_gvariant_to_gvalue (v); diff --git a/common/xfconf-gvaluefuncs.h b/common/xfconf-gvaluefuncs.h index 3293c58..3a100b6 100644 --- a/common/xfconf-gvaluefuncs.h +++ b/common/xfconf-gvaluefuncs.h @@ -38,6 +38,8 @@ G_GNUC_INTERNAL gboolean _xfconf_gvalue_is_equal(const GValue *value1, G_GNUC_INTERNAL void _xfconf_gvalue_free(GValue *value); +G_GNUC_INTERNAL GPtrArray *xfconf_dup_value_array (GPtrArray *arr, gboolean auto_destroy_value); + G_GNUC_INTERNAL GVariant *xfconf_basic_gvalue_to_gvariant (const GValue *value); G_GNUC_INTERNAL GVariant *xfconf_gvalue_to_gvariant (const GValue *value); diff --git a/xfconf/xfconf-cache.c b/xfconf/xfconf-cache.c index 491b99a..1923263 100644 --- a/xfconf/xfconf-cache.c +++ b/xfconf/xfconf-cache.c @@ -88,9 +88,16 @@ xfconf_cache_item_new(const GValue *value, if(G_LIKELY(steal)) { item->value = (GValue *) value; } else { + item->value = g_new0(GValue, 1); g_value_init(item->value, G_VALUE_TYPE(value)); - g_value_copy(value, item->value); + /* We need to dup the array */ + if (G_VALUE_TYPE(value) == G_TYPE_PTR_ARRAY) { + GPtrArray *arr = xfconf_dup_value_array(g_value_get_boxed(value), TRUE); + g_value_take_boxed(item->value, arr); + } else { + g_value_copy(value, item->value); + } } return item; @@ -110,8 +117,14 @@ xfconf_cache_item_update(XfconfCacheItem *item, if(value) { g_value_unset(item->value); g_value_init(item->value, G_VALUE_TYPE(value)); - g_value_copy(value, item->value); + /* We need to dup the array */ + if (G_VALUE_TYPE(value) == G_TYPE_PTR_ARRAY) { + GPtrArray *arr = xfconf_dup_value_array(g_value_get_boxed(value), TRUE); + g_value_take_boxed(item->value, arr); + } else { + g_value_copy(value, item->value); + } return TRUE; } @@ -135,10 +148,20 @@ xfconf_cache_item_free(XfconfCacheItem *item) typedef struct { gchar *property; - GCancellable *cancellable; XfconfCacheItem *item; + + GCancellable *cancellable; + + /** + * Variant to be send on the wire + * Used in xfconf_cache_old_item_end_call + * to end an already started call + **/ + GVariant *variant; + /* Pointer to the cache object */ XfconfCache *cache; + } XfconfCacheOldItem; @@ -153,6 +176,7 @@ xfconf_cache_old_item_new(XfconfCache *cache, const gchar *property) old_item->property = g_strdup(property); old_item->cancellable = g_cancellable_new (); old_item->cache = cache; + old_item->variant = NULL; return old_item; } @@ -170,6 +194,9 @@ xfconf_cache_old_item_free(XfconfCacheOldItem *old_item) g_object_unref (old_item->cancellable); g_free(old_item->property); + if (old_item->variant) + g_variant_unref (old_item->variant); + if(old_item->item) xfconf_cache_item_free(old_item->item); @@ -181,12 +208,31 @@ xfconf_cache_old_item_end_call(gpointer key, gpointer value, gpointer user_data) { + GError *error = NULL; XfconfCacheOldItem *old_item = value; + GDBusProxy *gproxy = _xfconf_get_gdbus_proxy(); + const gchar *channel_name = user_data; + GVariant *variant; g_return_val_if_fail(g_cancellable_is_cancelled(old_item->cancellable) == FALSE, TRUE); + variant = g_variant_new_variant (old_item->variant); + g_cancellable_cancel(old_item->cancellable); + xfconf_exported_call_set_property_sync ((XfconfExported *)gproxy, + channel_name, + old_item->property, + variant, + NULL, + &error); + + if (error) { + g_warning("Failed to set property \"%s::%s\": %s", + channel_name, old_item->property, error->message); + g_error_free(error); + } + return TRUE; } @@ -215,6 +261,8 @@ struct _XfconfCache GHashTable *pending_calls; GHashTable *old_properties; + gint g_signal_id; + #if GLIB_CHECK_VERSION (2, 32, 0) GMutex cache_lock; #else @@ -335,9 +383,9 @@ xfconf_cache_init(XfconfCache *cache) { GDBusProxy *gproxy = _xfconf_get_gdbus_proxy(); - g_signal_connect(gproxy, "g-signal", - G_CALLBACK(xfconf_cache_proxy_signal_received_cb), cache); - + cache->g_signal_id = g_signal_connect(gproxy, "g-signal", + G_CALLBACK(xfconf_cache_proxy_signal_received_cb), cache); + cache->properties = g_tree_new_full((GCompareDataFunc)strcmp, NULL, (GDestroyNotify)g_free, (GDestroyNotify)xfconf_cache_item_free); @@ -415,6 +463,11 @@ xfconf_cache_finalize(GObject *obj) { XfconfCache *cache = XFCONF_CACHE(obj); GHashTable *pending_calls; + GDBusProxy *proxy; + + proxy = _xfconf_get_gdbus_proxy(); + + g_signal_handler_disconnect(proxy,cache->g_signal_id); /* finish pending calls (without emitting signals, therefore we set * the hash table in the cache to %NULL) */ @@ -429,6 +482,7 @@ xfconf_cache_finalize(GObject *obj) g_tree_destroy(cache->properties); g_hash_table_destroy(cache->old_properties); + #if !GLIB_CHECK_VERSION (2, 32, 0) g_mutex_free (cache->cache_lock); #endif @@ -440,22 +494,20 @@ xfconf_cache_finalize(GObject *obj) static void xfconf_cache_handle_property_changed (XfconfCache *cache, GVariant *parameters) { + XfconfCacheItem *item; const gchar *channel_name, *property; GVariant *prop_variant; - GValue prop_value = G_VALUE_INIT; + GValue *prop_value; gboolean changed = TRUE; - if (g_variant_is_of_type(parameters, G_VARIANT_TYPE ("(ssv)"))) { - g_variant_get(parameters, "(ssv)", &channel_name, &property, &prop_variant); - + g_variant_get(parameters, "(&s&sv)", &channel_name, &property, &prop_variant); + if(strcmp(channel_name, cache->channel_name)) { - g_variant_unref(prop_variant); return; } - - g_dbus_gvariant_to_gvalue (prop_variant, &prop_value); - + prop_value = xfconf_gvariant_to_gvalue (prop_variant); + /* if a call was cancelled, we still receive a property-changed from * that value, in that case, abort the emission of the signal. we can * detect this because the new reply is not processed yet and thus @@ -464,63 +516,70 @@ xfconf_cache_handle_property_changed (XfconfCache *cache, GVariant *parameters) return; item = g_tree_lookup(cache->properties, property); - if(item) - changed = xfconf_cache_item_update(item, &prop_value); + if(item) { + changed = xfconf_cache_item_update(item, prop_value); + } else { - item = xfconf_cache_item_new(&prop_value, FALSE); + item = xfconf_cache_item_new(prop_value, TRUE); g_tree_insert(cache->properties, g_strdup(property), item); } - + if(changed) { g_signal_emit(G_OBJECT(cache), signals[SIG_PROPERTY_CHANGED], 0, - cache->channel_name, property, &prop_value); + cache->channel_name, property, prop_value); } - g_value_unset (&prop_value); + g_value_unset (prop_value); g_variant_unref(prop_variant); } else { g_warning("property changed handler expects (ssv) type, but %s received", g_variant_get_type_string(parameters)); } + } static void xfconf_cache_handle_property_removed (XfconfCache *cache, GVariant *parameters) { + const gchar *channel_name, *property; GValue value = G_VALUE_INIT; - if (g_variant_is_of_type(parameters, G_VARIANT_TYPE ("(ss)"))) { g_variant_get(parameters, "(&s&s)", &channel_name, &property); - + if(strcmp(channel_name, cache->channel_name)) return; - + g_tree_remove(cache->properties, property); - + g_signal_emit(G_OBJECT(cache), signals[SIG_PROPERTY_CHANGED], 0, cache->channel_name, property, &value); - + } else { g_warning("property removed handler expects (ss) type, but %s received", g_variant_get_type_string(parameters)); } + } -static void +static void xfconf_cache_proxy_signal_received_cb(GDBusProxy *proxy, gchar *sender_name, gchar *signal_name, GVariant *parameters, gpointer user_data) { + XfconfCache *cache=(XfconfCache*)user_data; + + g_return_if_fail(XFCONF_IS_CACHE(cache)); + if (g_strcmp0(signal_name, "PropertyChanged") == 0) - xfconf_cache_handle_property_changed (XFCONF_CACHE(user_data), parameters); + xfconf_cache_handle_property_changed (cache, parameters); else if (g_strcmp0(signal_name, "PropertyRemoved") == 0) - xfconf_cache_handle_property_removed(XFCONF_CACHE(user_data), parameters); + xfconf_cache_handle_property_removed(cache, parameters); else g_warning ("Unhandled signal name :%s\n", signal_name); } @@ -538,7 +597,6 @@ xfconf_cache_set_property_reply_handler(GDBusProxy *proxy, gboolean result; old_item = (XfconfCacheOldItem *) user_data; cache = old_item->cache; - if(!cache->pending_calls) return; @@ -659,15 +717,14 @@ xfconf_cache_prefetch(XfconfCache *cache, &props_variant, NULL, &tmp_error)) { g_variant_get (props_variant, "a{sv}", &iter); + while (g_variant_iter_next (iter, "{sv}", &key, &value)) { XfconfCacheItem *item; - GValue gvalue = G_VALUE_INIT; - g_dbus_gvariant_to_gvalue(value, &gvalue); - item = xfconf_cache_item_new(&gvalue, FALSE); + GValue *gvalue = xfconf_gvariant_to_gvalue (value); + item = xfconf_cache_item_new(gvalue, TRUE); g_tree_insert(cache->properties, key, item); - g_value_unset (&gvalue); } /* TODO: honor max entries */ ret = TRUE; @@ -688,21 +745,20 @@ xfconf_cache_lookup_locked(XfconfCache *cache, GError **error) { XfconfCacheItem *item = NULL; - item = g_tree_lookup(cache->properties, property); + if(!item) { GVariant *variant; GDBusProxy *proxy = _xfconf_get_gdbus_proxy(); - GValue tmpval = { 0, }; GError *tmp_error = NULL; /* blocking, ugh */ if(xfconf_exported_call_get_property_sync ((XfconfExported *)proxy, cache->channel_name, property, &variant, NULL, &tmp_error)) { - g_dbus_gvariant_to_gvalue (variant, &tmpval); - item = xfconf_cache_item_new(&tmpval, FALSE); + GValue *tmpval; + tmpval = xfconf_gvariant_to_gvalue(variant); + item = xfconf_cache_item_new(tmpval, TRUE); g_tree_insert(cache->properties, g_strdup(property), item); - g_value_unset(&tmpval); g_variant_unref (variant); /* TODO: check tree for evictions */ } else @@ -714,11 +770,24 @@ xfconf_cache_lookup_locked(XfconfCache *cache, if(!G_VALUE_TYPE(value)) g_value_init(value, G_VALUE_TYPE(item->value)); - if(G_VALUE_TYPE(value) == G_VALUE_TYPE(item->value)) - g_value_copy(item->value, value); - else { - if(!g_value_transform(item->value, value)) + if (G_VALUE_TYPE(item->value) == G_TYPE_PTR_ARRAY) { + if (G_VALUE_TYPE(value) != G_TYPE_PTR_ARRAY) { + g_warning("Given value is not of type G_TYPE_PTR_ARRAY"); item = NULL; + } + else { + GPtrArray *arr; + arr = xfconf_dup_value_array (g_value_get_boxed(item->value), FALSE); + g_value_take_boxed(value, arr); + } + } + else { + if(G_VALUE_TYPE(value) == G_VALUE_TYPE(item->value)) + g_value_copy(item->value, value); + else { + if(!g_value_transform(item->value, value)) + item = NULL; + } } } #if 0 @@ -776,7 +845,6 @@ xfconf_cache_set(XfconfCache *cache, && g_strcmp0(dbus_error_name, "org.xfce.Xfconf.Error.ChannelNotFound") != 0) { /* this is bad... */ - g_print ("this is bad\n"); g_propagate_error(error, tmp_error); xfconf_cache_mutex_unlock(cache); g_free (dbus_error_name); @@ -806,7 +874,6 @@ xfconf_cache_set(XfconfCache *cache, * the property. * we also steal the old_item from the pending_calls table * so there are no pending item left. */ - if(!g_cancellable_is_cancelled (old_item->cancellable)) { g_cancellable_cancel(old_item->cancellable); g_hash_table_steal(cache->pending_calls, old_item->cancellable); @@ -831,22 +898,23 @@ xfconf_cache_set(XfconfCache *cache, old_item->cancellable, (GAsyncReadyCallback) xfconf_cache_set_property_reply_handler, old_item); - + + /* Val will be freed asynchronously */ + old_item->variant = val; + g_hash_table_insert(cache->pending_calls, old_item->cancellable, old_item); - + if(item) xfconf_cache_item_update(item, value); else { item = xfconf_cache_item_new(value, FALSE); g_tree_insert(cache->properties, g_strdup(property), item); } - - xfconf_cache_mutex_unlock(cache); + xfconf_cache_mutex_unlock(cache); g_signal_emit(G_OBJECT(cache), signals[SIG_PROPERTY_CHANGED], 0, cache->channel_name, property, value); - - g_variant_unref (val); + return TRUE; } return FALSE; diff --git a/xfconf/xfconf-channel.c b/xfconf/xfconf-channel.c index 726a4f5..9532c44 100644 --- a/xfconf/xfconf-channel.c +++ b/xfconf/xfconf-channel.c @@ -458,47 +458,6 @@ xfconf_channel_get_internal(XfconfChannel *channel, return ret; } -static GPtrArray * -xfconf_fixup_16bit_ints(GPtrArray *arr) -{ - GPtrArray *arr_new = NULL; - guint i; - - for(i = 0; i < arr->len; ++i) { - GValue *v = g_ptr_array_index(arr, i); - - if(G_VALUE_TYPE(v) == XFCONF_TYPE_UINT16 - || G_VALUE_TYPE(v) == XFCONF_TYPE_INT16) - { - arr_new = g_ptr_array_sized_new(arr->len); - break; - } - } - - if(!arr_new) - return NULL; - - for(i = 0; i < arr->len; ++i) { - GValue *v_src, *v_dest; - - v_src = g_ptr_array_index(arr, i); - v_dest = g_new0(GValue, 1); - if(G_VALUE_TYPE(v_src) == XFCONF_TYPE_UINT16) { - g_value_init(v_dest, G_TYPE_UINT); - g_value_set_uint(v_dest, xfconf_g_value_get_uint16(v_src)); - } else if(G_VALUE_TYPE(v_src) == XFCONF_TYPE_INT16) { - g_value_init(v_dest, G_TYPE_INT); - g_value_set_int(v_dest, xfconf_g_value_get_int16(v_src)); - } else { - g_value_init(v_dest, G_VALUE_TYPE(v_src)); - g_value_copy(v_src, v_dest); - } - - g_ptr_array_add(arr_new, v_dest); - } - - return arr_new; -} static GPtrArray * xfconf_transform_array(GPtrArray *arr_src, @@ -1324,8 +1283,7 @@ xfconf_channel_set_property(XfconfChannel *channel, const gchar *property, const GValue *value) { - GValue *val, tmp_val = { 0, }; - GPtrArray *arr_new = NULL; + GValue val = { 0, }; gboolean ret; g_return_val_if_fail(XFCONF_IS_CHANNEL(channel) @@ -1335,34 +1293,11 @@ xfconf_channel_set_property(XfconfChannel *channel, || g_value_get_string(value) == NULL || g_utf8_validate(g_value_get_string(value), -1, NULL), FALSE); - - /* intercept uint16/int16 since dbus-glib doesn't know how to send - * them over the wire */ - if(G_VALUE_TYPE(value) == XFCONF_TYPE_UINT16) { - val = &tmp_val; - g_value_init(&tmp_val, G_TYPE_UINT); - g_value_set_uint(&tmp_val, xfconf_g_value_get_uint16(value)); - } else if(G_VALUE_TYPE(value) == XFCONF_TYPE_INT16) { - val = &tmp_val; - g_value_init(&tmp_val, G_TYPE_INT); - g_value_set_int(&tmp_val, xfconf_g_value_get_int16(value)); - } else if(G_VALUE_TYPE(value) == G_TYPE_PTR_ARRAY) { - arr_new = xfconf_fixup_16bit_ints(g_value_get_boxed(value)); - if(arr_new) { - val = &tmp_val; - g_value_init(&tmp_val, G_TYPE_PTR_ARRAY); - g_value_set_boxed(&tmp_val, arr_new); - } else - val = (GValue *)value; - } else - val = (GValue *)value; - - ret = xfconf_channel_set_internal(channel, property, val); - - if(val == &tmp_val) - g_value_unset(&tmp_val); - if(arr_new) - xfconf_array_free(arr_new); + + g_value_init(&val, G_VALUE_TYPE(value)); + g_value_copy(value, &val); + ret = xfconf_channel_set_internal(channel, property, &val); + g_value_unset(&val); return ret; } @@ -1543,46 +1478,32 @@ GPtrArray * xfconf_channel_get_arrayv(XfconfChannel *channel, const gchar *property) { - GPtrArray *arr = NULL; GValue val = { 0, }; - GVariant *variant; - GVariant *value_prop; - GVariantIter iter; - gsize count; + GPtrArray *arr = NULL; gboolean ret; g_return_val_if_fail(XFCONF_IS_CHANNEL(channel) && property, NULL); ret = xfconf_channel_get_internal(channel, property, &val); + if(!ret) return NULL; - if(G_TYPE_VARIANT != G_VALUE_TYPE(&val)) { + if(G_TYPE_PTR_ARRAY != G_VALUE_TYPE(&val)) { + g_warning ("Unexpected value type %s\n", G_VALUE_TYPE_NAME(&val)); g_value_unset(&val); return NULL; } - variant = g_value_get_variant (&val); - g_value_unset(&val); - - count = g_variant_iter_init (&iter, variant); - arr = g_ptr_array_sized_new (count + 1); - - while (g_variant_iter_next (&iter, "v", &value_prop)) { - GValue *arr_val; - arr_val = g_new0(GValue, 1); - g_dbus_gvariant_to_gvalue (value_prop, arr_val); - - g_ptr_array_add(arr, arr_val); - g_variant_unref (value_prop); - } - g_variant_unref (variant); - + /** + * Arr is owned by the Gvalue in the cache + * do not free it. + **/ + arr = g_value_get_boxed(&val); if(!arr->len) { g_ptr_array_free(arr, TRUE); return NULL; } - return arr; } @@ -1743,25 +1664,19 @@ xfconf_channel_set_arrayv(XfconfChannel *channel, const gchar *property, GPtrArray *values) { - GPtrArray *values_new = NULL; GValue val = { 0, }; gboolean ret; g_return_val_if_fail(XFCONF_IS_CHANNEL(channel) && property && values, FALSE); - values_new = xfconf_fixup_16bit_ints(values); - g_value_init(&val, G_TYPE_PTR_ARRAY); - g_value_set_static_boxed(&val, values_new ? values_new : values); + g_value_set_static_boxed(&val, values); ret = xfconf_channel_set_internal(channel, property, &val); g_value_unset(&val); - if(values_new) - xfconf_array_free(values_new); - return ret; } -- To stop receiving notification emails like this one, please contact the administrator of this repository. _______________________________________________ Xfce4-commits mailing list Xfce4-commits@xfce.org https://mail.xfce.org/mailman/listinfo/xfce4-commits