From e27eacb054af4e208f85d76298a5f6bb2677f707 Mon Sep 17 00:00:00 2001 From: Nick Schermer Date: Mon, 8 Feb 2010 18:44:58 +0100 Subject: Fix handling of dirty channels and use hash table. Use a hash table for looking up the channels by name. Also store the dirty bit inside the channel structure to simplefy the code. The GSList used in the old code was not properly maintained resulting in memory corruption when resetting a channel and a lot of channel flushing. --- xfconfd/xfconf-backend-perchannel-xml.c | 96 ++++++++++++++++----------------- 1 file changed, 45 insertions(+), 51 deletions(-) (limited to 'xfconfd') diff --git a/xfconfd/xfconf-backend-perchannel-xml.c b/xfconfd/xfconf-backend-perchannel-xml.c index 5531878..8e348aa 100644 --- a/xfconfd/xfconf-backend-perchannel-xml.c +++ b/xfconfd/xfconf-backend-perchannel-xml.c @@ -78,10 +78,9 @@ struct _XfconfBackendPerchannelXml gchar *config_save_path; - GTree *channels; + GHashTable *channels; guint save_id; - GSList *dirty_channels; XfconfPropertyChangedFunc prop_changed_func; gpointer prop_changed_data; @@ -96,6 +95,7 @@ typedef struct { GNode *properties; gboolean locked; + gboolean dirty; } XfconfChannel; typedef struct @@ -176,7 +176,7 @@ static void xfconf_backend_perchannel_xml_register_property_changed_func(XfconfB gpointer user_data); static void xfconf_backend_perchannel_xml_schedule_save(XfconfBackendPerchannelXml *xbpx, - const gchar *channel_name); + XfconfChannel *channel); static XfconfChannel *xfconf_backend_perchannel_xml_create_channel(XfconfBackendPerchannelXml *xbpx, const gchar *channel_name); @@ -223,10 +223,9 @@ xfconf_backend_perchannel_xml_class_init(XfconfBackendPerchannelXmlClass *klass) static void xfconf_backend_perchannel_xml_init(XfconfBackendPerchannelXml *instance) { - instance->channels = g_tree_new_full((GCompareDataFunc)g_ascii_strcasecmp, - NULL, - (GDestroyNotify)g_free, - (GDestroyNotify)xfconf_channel_destroy); + instance->channels = g_hash_table_new_full(g_str_hash, g_str_equal, + (GDestroyNotify)g_free, + (GDestroyNotify)xfconf_channel_destroy); } static void @@ -237,12 +236,10 @@ xfconf_backend_perchannel_xml_finalize(GObject *obj) if(xbpx->save_id) { g_source_remove(xbpx->save_id); xbpx->save_id = 0; - } - - if(xbpx->dirty_channels) xfconf_backend_perchannel_xml_flush(XFCONF_BACKEND(xbpx), NULL); + } - g_tree_destroy(xbpx->channels); + g_hash_table_destroy(xbpx->channels); g_free(xbpx->config_save_path); @@ -297,7 +294,7 @@ xfconf_backend_perchannel_xml_set(XfconfBackend *backend, GError **error) { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); - XfconfChannel *channel = g_tree_lookup(xbpx->channels, channel_name); + XfconfChannel *channel = g_hash_table_lookup(xbpx->channels, channel_name); XfconfProperty *cur_prop; if(!channel) { @@ -349,7 +346,7 @@ xfconf_backend_perchannel_xml_set(XfconfBackend *backend, xbpx->prop_changed_func(backend, channel_name, property, xbpx->prop_changed_data); } - xfconf_backend_perchannel_xml_schedule_save(xbpx, channel_name); + xfconf_backend_perchannel_xml_schedule_save(xbpx, channel); return TRUE; } @@ -362,7 +359,7 @@ xfconf_backend_perchannel_xml_get(XfconfBackend *backend, GError **error) { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); - XfconfChannel *channel = g_tree_lookup(xbpx->channels, channel_name); + XfconfChannel *channel = g_hash_table_lookup(xbpx->channels, channel_name); XfconfProperty *cur_prop; GValue *value_to_get = NULL; @@ -451,7 +448,7 @@ xfconf_backend_perchannel_xml_get_all(XfconfBackend *backend, GError **error) { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); - XfconfChannel *channel = g_tree_lookup(xbpx->channels, channel_name); + XfconfChannel *channel = g_hash_table_lookup(xbpx->channels, channel_name); GNode *props_tree; gchar cur_path[MAX_PROP_PATH], *p; @@ -496,7 +493,7 @@ xfconf_backend_perchannel_xml_exists(XfconfBackend *backend, GError **error) { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); - XfconfChannel *channel = g_tree_lookup(xbpx->channels, channel_name); + XfconfChannel *channel = g_hash_table_lookup(xbpx->channels, channel_name); XfconfProperty *prop; if(!channel) { @@ -582,19 +579,8 @@ do_reset_channel(XfconfBackend *backend, { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); gchar *filename; - GSList *dirty; PropChangeData pdata; - if((dirty = g_slist_find_custom(xbpx->dirty_channels, channel_name, - (GCompareFunc)g_ascii_strcasecmp))) - { - xbpx->dirty_channels = g_slist_remove(xbpx->dirty_channels, dirty); - if(!xbpx->dirty_channels && xbpx->save_id) { - g_source_remove(xbpx->save_id); - xbpx->save_id = 0; - } - } - pdata.xbpx = xbpx; pdata.channel_name = channel_name; g_node_traverse(properties, G_POST_ORDER, G_TRAVERSE_ALL, -1, @@ -603,7 +589,7 @@ do_reset_channel(XfconfBackend *backend, /* we could probably prune the existing proptree, or even just leave * it as-is, but it's easier to just kill it. it'll get reloaded later * from the system file (if any) if needed. */ - g_tree_remove(xbpx->channels, channel_name); + g_hash_table_remove(xbpx->channels, channel_name); /* regardless of whether or not we have a system file, we don't need * the user file anymore */ @@ -631,7 +617,7 @@ xfconf_backend_perchannel_xml_reset(XfconfBackend *backend, GError **error) { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); - XfconfChannel *channel = g_tree_lookup(xbpx->channels, channel_name); + XfconfChannel *channel = g_hash_table_lookup(xbpx->channels, channel_name); if(!channel) { channel = xfconf_backend_perchannel_xml_load_channel(xbpx, channel_name, @@ -686,7 +672,7 @@ xfconf_backend_perchannel_xml_reset(XfconfBackend *backend, } } - xfconf_backend_perchannel_xml_schedule_save(xbpx, channel_name); + xfconf_backend_perchannel_xml_schedule_save(xbpx, channel); return TRUE; } @@ -736,7 +722,7 @@ xfconf_backend_perchannel_xml_is_property_locked(XfconfBackend *backend, GError **error) { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); - XfconfChannel *channel = g_tree_lookup(xbpx->channels, channel_name); + XfconfChannel *channel = g_hash_table_lookup(xbpx->channels, channel_name); XfconfProperty *prop = NULL; if(!channel) { @@ -752,15 +738,29 @@ xfconf_backend_perchannel_xml_is_property_locked(XfconfBackend *backend, return TRUE; } +static void +xfconf_backend_perchannel_xml_flush_get_dirty(gpointer key, + gpointer value, + gpointer user_data) +{ + XfconfChannel *channel = value; + GSList **dirty = user_data; + if(channel->dirty) + *dirty = g_slist_prepend(*dirty, key); +} + static gboolean xfconf_backend_perchannel_xml_flush(XfconfBackend *backend, GError **error) { XfconfBackendPerchannelXml *xbpx = XFCONF_BACKEND_PERCHANNEL_XML(backend); - GSList *l; + GSList *dirty = NULL, *l; - for(l = xbpx->dirty_channels; l; l = l->next) + g_hash_table_foreach(xbpx->channels, xfconf_backend_perchannel_xml_flush_get_dirty, &dirty); + + for(l = dirty; l; l = l->next) xfconf_backend_perchannel_xml_flush_channel(xbpx, l->data, error); + g_slist_free(dirty); TRACE("exiting, flushed all channels"); @@ -991,24 +991,13 @@ xfconf_backend_perchannel_xml_save_timeout(gpointer data) static void xfconf_backend_perchannel_xml_schedule_save(XfconfBackendPerchannelXml *xbpx, - const gchar *channel_name) + XfconfChannel *channel) { - if(!g_slist_find_custom(xbpx->dirty_channels, channel_name, - (GCompareFunc)g_ascii_strcasecmp)) - { - gpointer orig_key = NULL, val = NULL; - - if(!g_tree_lookup_extended(xbpx->channels, channel_name, &orig_key, &val)) { - g_warning("Attempt to schedule save for a nonexistent channel."); - return; - } - - xbpx->dirty_channels = g_slist_prepend(xbpx->dirty_channels, orig_key); - } - if(xbpx->save_id) g_source_remove(xbpx->save_id); + channel->dirty = TRUE; + xbpx->save_id = g_timeout_add_seconds(WRITE_TIMEOUT, xfconf_backend_perchannel_xml_save_timeout, xbpx); @@ -1021,7 +1010,8 @@ xfconf_backend_perchannel_xml_create_channel(XfconfBackendPerchannelXml *xbpx, XfconfChannel *channel; XfconfProperty *prop; - if((channel = g_tree_lookup(xbpx->channels, channel_name))) { + channel = g_hash_table_lookup(xbpx->channels, channel_name); + if(channel) { g_warning("Attempt to create channel when one already exists."); return channel; } @@ -1030,7 +1020,7 @@ xfconf_backend_perchannel_xml_create_channel(XfconfBackendPerchannelXml *xbpx, prop = g_slice_new0(XfconfProperty); prop->name = g_strdup("/"); channel->properties = g_node_new(prop); - g_tree_insert(xbpx->channels, g_ascii_strdown(channel_name, -1), channel); + g_hash_table_insert(xbpx->channels, g_ascii_strdown(channel_name, -1), channel); return channel; } @@ -1688,7 +1678,7 @@ xfconf_backend_perchannel_xml_load_channel(XfconfBackendPerchannelXml *xbpx, channel, NULL); } - g_tree_insert(xbpx->channels, g_ascii_strdown(channel_name, -1), channel); + g_hash_table_insert(xbpx->channels, g_ascii_strdown(channel_name, -1), channel); out: g_strfreev(filenames); @@ -1900,11 +1890,13 @@ xfconf_backend_perchannel_xml_flush_channel(XfconfBackendPerchannelXml *xbpx, GError **error) { gboolean ret = FALSE; - XfconfChannel *channel = g_tree_lookup(xbpx->channels, channel_name); + XfconfChannel *channel = g_hash_table_lookup(xbpx->channels, channel_name); GNode *child; gchar *filename = NULL, *filename_tmp = NULL; FILE *fp = NULL; + DBG("Flushed dirty channel \"%s\"", channel_name); + if(!channel) { if(error) { g_set_error(error, XFCONF_ERROR, @@ -1977,5 +1969,7 @@ out: g_free(filename); g_free(filename_tmp); + channel->dirty = FALSE; + return ret; } -- cgit v1.2.1