2003-04-01 Havoc Pennington <hp@redhat.com>

	* bus/config-parser.c, bus/bus.c: implement <servicedir> and
	<includedir> (at least mostly)

	* dbus/dbus-sysdeps.c (_dbus_change_identity): set the group ID
	first, then the user ID

9 files changed, 226 insertions, 54 deletions

diff --git a/ChangeLog b/ChangeLog
index 602341af..ba9cb712 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2003-04-01  Havoc Pennington  <hp@redhat.com>
+
+	* bus/config-parser.c, bus/bus.c: implement <servicedir> and
+	<includedir> (at least mostly)
+
+	* dbus/dbus-sysdeps.c (_dbus_change_identity): set the group ID
+	first, then the user ID
+
 2003-04-01  Havoc Pennington  <hp@pobox.com>
 
 	* dbus/dbus-server.c (dbus_server_set_auth_mechanisms): new
diff --git a/bus/activation.c b/bus/activation.c
index fe069c79..7f3693a0 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -316,12 +316,12 @@ load_directory (BusActivation *activation,
 BusActivation*
 bus_activation_new (BusContext        *context,
 		    const DBusString  *address,
-                    const char       **directories,
+                    DBusList         **directories,
                     DBusError         *error)
 {
-  int i;
   BusActivation *activation;
-
+  DBusList *link;
+  
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   
   activation = dbus_new0 (BusActivation, 1);
@@ -358,12 +358,12 @@ bus_activation_new (BusContext        *context,
     }
   
   /* Load service files */
-  i = 0;
-  while (directories[i] != NULL)
+  link = _dbus_list_get_first_link (directories);
+  while (link != NULL)
     {
-      if (!load_directory (activation, directories[i], error))
+      if (!load_directory (activation, link->data, error))
         goto failed;
-      ++i;
+      link = _dbus_list_get_next_link (directories, link);
     }
 
   return activation;
diff --git a/bus/activation.h b/bus/activation.h
index fd3a72c9..0e9e6ea3 100644
--- a/bus/activation.h
+++ b/bus/activation.h
@@ -25,11 +25,12 @@
 #define BUS_ACTIVATION_H
 
 #include <dbus/dbus.h>
+#include <dbus/dbus-list.h>
 #include "bus.h"
 
 BusActivation* bus_activation_new              (BusContext        *context,
                                                 const DBusString  *address,
-                                                const char       **paths,
+                                                DBusList         **directories,
                                                 DBusError         *error);
 void           bus_activation_ref              (BusActivation     *activation);
 void           bus_activation_unref            (BusActivation     *activation);
diff --git a/bus/bus.c b/bus/bus.c
index 1ca4feb8..82452d07 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -187,7 +187,6 @@ bus_context_new (const DBusString *config_file,
   DBusList **addresses;
   BusConfigParser *parser;
   DBusString full_address;
-  const char *service_dirs[] = { NULL, NULL };
   const char *user;
   char **auth_mechanisms;
   DBusList **auth_mechanisms_list;
@@ -336,7 +335,8 @@ bus_context_new (const DBusString *config_file,
   /* Create activation subsystem */
   
   context->activation = bus_activation_new (context, &full_address,
-                                            service_dirs, error);
+                                            bus_config_parser_get_service_dirs (parser),
+                                            error);
   if (context->activation == NULL)
     {
       _DBUS_ASSERT_ERROR_IS_SET (error);
diff --git a/bus/config-parser.c b/bus/config-parser.c
index dc3cb4d6..77b68cae 100644
--- a/bus/config-parser.c
+++ b/bus/config-parser.c
@@ -38,7 +38,9 @@ typedef enum
   ELEMENT_LIMIT,
   ELEMENT_ALLOW,
   ELEMENT_DENY,
-  ELEMENT_FORK
+  ELEMENT_FORK,
+  ELEMENT_SERVICEDIR,
+  ELEMENT_INCLUDEDIR
 } ElementType;
 
 typedef struct
@@ -87,6 +89,8 @@ struct BusConfigParser
   DBusList *listen_on; /**< List of addresses to listen to */
 
   DBusList *mechanisms; /**< Auth mechanisms */
+
+  DBusList *service_dirs; /**< Directories to look for services in */
   
   unsigned int fork : 1; /**< TRUE to fork into daemon mode */
 };
@@ -118,6 +122,10 @@ element_type_to_name (ElementType type)
       return "deny";
     case ELEMENT_FORK:
       return "fork";
+    case ELEMENT_SERVICEDIR:
+      return "servicedir";
+    case ELEMENT_INCLUDEDIR:
+      return "includedir";
     }
 
   _dbus_assert_not_reached ("bad element type");
@@ -210,6 +218,9 @@ merge_included (BusConfigParser *parser,
 
   while ((link = _dbus_list_pop_first_link (&included->mechanisms)))
     _dbus_list_append_link (&parser->mechanisms, link);
+
+  while ((link = _dbus_list_pop_first_link (&included->service_dirs)))
+    _dbus_list_append_link (&parser->service_dirs, link);
   
   return TRUE;
 }
@@ -256,6 +267,12 @@ bus_config_parser_unref (BusConfigParser *parser)
 
       _dbus_list_clear (&parser->listen_on);
 
+      _dbus_list_foreach (&parser->service_dirs,
+                          (DBusForeachFunction) dbus_free,
+                          NULL);
+
+      _dbus_list_clear (&parser->service_dirs);
+      
       dbus_free (parser);
     }
 }
@@ -463,6 +480,32 @@ start_busconfig_child (BusConfigParser   *parser,
 
       return TRUE;
     }
+  else if (strcmp (element_name, "includedir") == 0)
+    {
+      if (!check_no_attributes (parser, "includedir", attribute_names, attribute_values, error))
+        return FALSE;
+
+      if (push_element (parser, ELEMENT_INCLUDEDIR) == NULL)
+        {
+          dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+          return FALSE;
+        }
+
+      return TRUE;
+    }
+  else if (strcmp (element_name, "servicedir") == 0)
+    {
+      if (!check_no_attributes (parser, "servicedir", attribute_names, attribute_values, error))
+        return FALSE;
+
+      if (push_element (parser, ELEMENT_SERVICEDIR) == NULL)
+        {
+          dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+          return FALSE;
+        }
+
+      return TRUE;
+    }
   else if (strcmp (element_name, "include") == 0)
     {
       Element *e;
@@ -681,6 +724,8 @@ bus_config_parser_end_element (BusConfigParser   *parser,
     case ELEMENT_USER:
     case ELEMENT_LISTEN:
     case ELEMENT_AUTH:
+    case ELEMENT_SERVICEDIR:
+    case ELEMENT_INCLUDEDIR:
       if (!e->had_content)
         {
           dbus_set_error (error, DBUS_ERROR_FAILED,
@@ -714,6 +759,112 @@ all_whitespace (const DBusString *str)
   return i == _dbus_string_get_length (str);
 }
 
+static dbus_bool_t
+include_file (BusConfigParser   *parser,
+              const DBusString  *filename,
+              dbus_bool_t        ignore_missing,
+              DBusError         *error)
+{
+  /* FIXME good test case for this would load each config file in the
+   * test suite both alone, and as an include, and check
+   * that the result is the same
+   */
+  BusConfigParser *included;
+  DBusError tmp_error;
+        
+  dbus_error_init (&tmp_error);
+  included = bus_config_load (filename, &tmp_error);
+  if (included == NULL)
+    {
+      _DBUS_ASSERT_ERROR_IS_SET (&tmp_error);
+
+      if (dbus_error_has_name (&tmp_error, DBUS_ERROR_FILE_NOT_FOUND) &&
+          ignore_missing)
+        {
+          dbus_error_free (&tmp_error);
+          return TRUE;
+        }
+      else
+        {
+          dbus_move_error (&tmp_error, error);
+          return FALSE;
+        }
+    }
+  else
+    {
+      _DBUS_ASSERT_ERROR_IS_CLEAR (&tmp_error);
+
+      if (!merge_included (parser, included, error))
+        {
+          bus_config_parser_unref (included);
+          return FALSE;
+        }
+
+      bus_config_parser_unref (included);
+      return TRUE;
+    }
+}
+
+static dbus_bool_t
+include_dir (BusConfigParser   *parser,
+             const DBusString  *dirname,
+             DBusError         *error)
+{
+  DBusString filename;
+  dbus_bool_t retval;
+  DBusError tmp_error;
+  DBusDirIter *dir;
+  
+  if (!_dbus_string_init (&filename))
+    {
+      dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+      return FALSE;
+    }
+
+  retval = FALSE;
+  
+  dir = _dbus_directory_open (dirname, error);
+    
+  /* FIXME this is just so the tests pass for now, it needs to come out
+   * once I implement make-dirname-relative-to-currently-parsed-files-dir
+   */
+  if (dir == NULL)
+    {
+      if (error)
+        dbus_error_free (error);
+      _dbus_string_free (&filename);
+      return TRUE;
+    }
+
+  if (dir == NULL)
+    goto failed;
+  
+  while (_dbus_directory_get_next_file (dir, &filename, &tmp_error))
+    {
+      if (_dbus_string_ends_with_c_str (&filename, ".conf"))
+        {
+          if (!include_file (parser, &filename, TRUE, error))
+            goto failed;
+        }
+    }
+
+  if (dbus_error_is_set (&tmp_error))
+    {
+      dbus_move_error (&tmp_error, error);
+      goto failed;
+    }
+  
+  retval = TRUE;
+  
+ failed:
+  _dbus_string_free (&filename);
+  
+  if (dir)
+    _dbus_directory_close (dir);
+
+  return retval;
+}
+
 dbus_bool_t
 bus_config_parser_content (BusConfigParser   *parser,
                            const DBusString  *content,
@@ -770,45 +921,23 @@ bus_config_parser_content (BusConfigParser   *parser,
 
     case ELEMENT_INCLUDE:
       {
-        /* FIXME good test case for this would load each config file in the
-         * test suite both alone, and as an include, and check
-         * that the result is the same
-         */
-        BusConfigParser *included;
-        DBusError tmp_error;
-
         e->had_content = TRUE;
-        
-        dbus_error_init (&tmp_error);
-        included = bus_config_load (content, &tmp_error);
-        if (included == NULL)
-          {
-            _DBUS_ASSERT_ERROR_IS_SET (&tmp_error);
-            if (dbus_error_has_name (&tmp_error, DBUS_ERROR_FILE_NOT_FOUND) &&
-                e->d.include.ignore_missing)
-              {
-                dbus_error_free (&tmp_error);
-                return TRUE;
-              }
-            else
-              {
-                dbus_move_error (&tmp_error, error);
-                return FALSE;
-              }
-          }
-        else
-          {
-            _DBUS_ASSERT_ERROR_IS_CLEAR (&tmp_error);
 
-            if (!merge_included (parser, included, error))
-              return FALSE;
-
-            bus_config_parser_unref (included);
-            return TRUE;
-          }
+        if (!include_file (parser, content,
+                           e->d.include.ignore_missing, error))
+          return FALSE;
       }
       break;
 
+    case ELEMENT_INCLUDEDIR:
+      {
+        e->had_content = TRUE;
+        
+        if (!include_dir (parser, content, error))
+          return FALSE;
+      }
+      break;
+      
     case ELEMENT_USER:
       {
         char *s;
@@ -858,6 +987,24 @@ bus_config_parser_content (BusConfigParser   *parser,
           }
       }
       break;
+
+    case ELEMENT_SERVICEDIR:
+      {
+        char *s;
+
+        e->had_content = TRUE;
+        
+        if (!_dbus_string_copy_data (content, &s))
+          goto nomem;
+
+        if (!_dbus_list_append (&parser->service_dirs,
+                                s))
+          {
+            dbus_free (s);
+            goto nomem;
+          }
+      }
+      break;
     }
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
@@ -911,6 +1058,12 @@ bus_config_parser_get_mechanisms (BusConfigParser *parser)
   return &parser->mechanisms;
 }
 
+DBusList**
+bus_config_parser_get_service_dirs (BusConfigParser *parser)
+{
+  return &parser->service_dirs;
+}
+
 dbus_bool_t
 bus_config_parser_get_fork (BusConfigParser   *parser)
 {
diff --git a/bus/config-parser.h b/bus/config-parser.h
index 101b4c6f..d06cde08 100644
--- a/bus/config-parser.h
+++ b/bus/config-parser.h
@@ -55,10 +55,11 @@ dbus_bool_t      bus_config_parser_finished      (BusConfigParser   *parser,
                                                   DBusError         *error);
 
 /* Functions for extracting the parse results */
-const char*      bus_config_parser_get_user       (BusConfigParser   *parser);
-DBusList**       bus_config_parser_get_addresses  (BusConfigParser   *parser);
-DBusList**       bus_config_parser_get_mechanisms (BusConfigParser   *parser);
-dbus_bool_t      bus_config_parser_get_fork       (BusConfigParser   *parser);
+const char* bus_config_parser_get_user         (BusConfigParser *parser);
+DBusList**  bus_config_parser_get_addresses    (BusConfigParser *parser);
+DBusList**  bus_config_parser_get_mechanisms   (BusConfigParser *parser);
+dbus_bool_t bus_config_parser_get_fork         (BusConfigParser *parser);
+DBusList**  bus_config_parser_get_service_dirs (BusConfigParser *parser);
 
 /* Loader functions (backended off one of the XML parsers).  Returns a
  * finished ConfigParser.
diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
index d63bc3e8..b9203404 100644
--- a/dbus/dbus-keyring.c
+++ b/dbus/dbus-keyring.c
@@ -41,6 +41,10 @@
  * file in the user's homedir. However they are transient (only used
  * by a single server instance for a fixed period of time, then
  * discarded). Also, the keys are not sent over the wire.
+ *
+ * @todo there's a memory leak on some codepath in here, I saw it once
+ * when running make check - probably some specific initial cookies
+ * present in the cookie file, then depending on what we do with them.
  */
 
 /**
diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
index 71863ef6..8aa91811 100644
--- a/dbus/dbus-sysdeps.c
+++ b/dbus/dbus-sysdeps.c
@@ -3185,18 +3185,21 @@ _dbus_change_identity  (unsigned long  uid,
                         unsigned long  gid,
                         DBusError     *error)
 {
-  if (setuid (uid) < 0)
+  /* Set GID first, or the setuid may remove our permission
+   * to change the GID
+   */
+  if (setgid (gid) < 0)
     {
       dbus_set_error (error, _dbus_error_from_errno (errno),
-                      "Failed to set UID to %lu: %s", uid,
+                      "Failed to set GID to %lu: %s", gid,
                       _dbus_strerror (errno));
       return FALSE;
     }
-
-  if (setgid (gid) < 0)
+  
+  if (setuid (uid) < 0)
     {
       dbus_set_error (error, _dbus_error_from_errno (errno),
-                      "Failed to set GID to %lu: %s", gid,
+                      "Failed to set UID to %lu: %s", uid,
                       _dbus_strerror (errno));
       return FALSE;
     }
diff --git a/test/data/valid-config-files/basic.conf b/test/data/valid-config-files/basic.conf
index 222122cf..d109d71d 100644
--- a/test/data/valid-config-files/basic.conf
+++ b/test/data/valid-config-files/basic.conf
@@ -4,6 +4,8 @@
   <user>mybususer</user>
   <listen>unix:path=/foo/bar</listen>
   <listen>tcp:port=1234</listen>
+  <includedir>basic.d</includedir>
+  <servicedir>/usr/share/foo</servicedir>
   <include ignore_missing="yes">nonexistent.conf</include>
   <policy context="default">
     <allow user="*"/>