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.

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>

1 files 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))