]> git.ipfire.org Git - thirdparty/dbus.git/commitdiff
activation: Put activation directories in an ordered list
authorSimon McVittie <simon.mcvittie@collabora.co.uk>
Fri, 17 Feb 2017 12:21:46 +0000 (12:21 +0000)
committerSimon McVittie <simon.mcvittie@collabora.co.uk>
Mon, 20 Feb 2017 12:52:31 +0000 (12:52 +0000)
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.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
bus/activation.c

index a104276fb6fe80dbe7c873a5d1bfa4a86dfed346..43b922845b9202fb62285f77173c7de2613a71b3 100644 (file)
@@ -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))