From 5d8d758627d61cd2d9b6eafa2543d6c78bd9f8dd Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 17 Feb 2017 12:21:46 +0000 Subject: [PATCH 02/12] activation: Put activation directories in an ordered list There are two circumstances in which we load .service files. The first is bus_activation_reload(), which is given an ordered list of directory paths, and reads each one in its correct order, highest-precedence first (normally ~/.local/share > /usr/local/share > /usr/share). This seems correct. However, if we are asked to activate a service for which we do not know of a .service file, we opportunistically reload the search path and try again, in the hope that it was recently-installed and not yet discovered by inotify. Prior to this commit, this would iterate through the hash table in arbitrary hash order, so we might load a service from /usr/share even though it was meant to be masked by a higher-priority service file in ~/.local/share or /usr/local/share. Before I add more elements to the search path, we should make sure it is always searched in the expected order. We do not actually make use of the hash table's faster-than-O(n) lookup by directory path anywhere, so there is no point in using a hash table, and we can safely replace it with an ordered data structure. Signed-off-by: Simon McVittie --- bus/activation.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index a104276f..43b92284 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -53,7 +53,7 @@ struct BusActivation * i.e. number of pending activation requests, not pending * activations per se */ - DBusHashTable *directories; + DBusList *directories; DBusHashTable *environment; }; @@ -814,16 +814,9 @@ bus_activation_reload (BusActivation *activation, goto failed; } - if (activation->directories != NULL) - _dbus_hash_table_unref (activation->directories); - activation->directories = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, - (DBusFreeFunction)bus_service_directory_unref); - - if (activation->directories == NULL) - { - BUS_SET_OOM (error); - goto failed; - } + _dbus_list_foreach (&activation->directories, + (DBusForeachFunction) bus_service_directory_unref, NULL); + _dbus_list_clear (&activation->directories); link = _dbus_list_get_first_link (directories); while (link != NULL) @@ -858,7 +851,7 @@ bus_activation_reload (BusActivation *activation, goto failed; } - if (!_dbus_hash_table_insert_string (activation->directories, s_dir->dir_c, s_dir)) + if (!_dbus_list_append (&activation->directories, s_dir)) { bus_service_directory_unref (s_dir); BUS_SET_OOM (error); @@ -965,8 +958,11 @@ bus_activation_unref (BusActivation *activation) _dbus_hash_table_unref (activation->entries); if (activation->pending_activations) _dbus_hash_table_unref (activation->pending_activations); - if (activation->directories) - _dbus_hash_table_unref (activation->directories); + + _dbus_list_foreach (&activation->directories, + (DBusForeachFunction) bus_service_directory_unref, NULL); + _dbus_list_clear (&activation->directories); + if (activation->environment) _dbus_hash_table_unref (activation->environment); @@ -1537,15 +1533,14 @@ add_cancel_pending_to_transaction (BusTransaction *transaction, static dbus_bool_t update_service_cache (BusActivation *activation, DBusError *error) { - DBusHashIter iter; + DBusList *iter; - _dbus_hash_iter_init (activation->directories, &iter); - while (_dbus_hash_iter_next (&iter)) + for (iter = _dbus_list_get_first_link (&activation->directories); + iter != NULL; + iter = _dbus_list_get_next_link (&activation->directories, iter)) { DBusError tmp_error; - BusServiceDirectory *s_dir; - - s_dir = _dbus_hash_iter_get_value (&iter); + BusServiceDirectory *s_dir = iter->data; dbus_error_init (&tmp_error); if (!update_directory (activation, s_dir, &tmp_error)) -- 2.11.0