2007-07-24 Havoc Pennington <hp@redhat.com>

	* configure.in: add AM_PROG_CC_C_O to allow per-target CPPFLAGS

	* bus/dispatch.c (bus_dispatch_test_conf): Fix up setting
	TEST_LAUNCH_HELPER_CONFIG to include the full path, and enable
	test shell_fail_service_auto_start when use_launcher==TRUE

	* bus/activation-helper-bin.c (convert_error_to_exit_code): pass
	through the INVALID_ARGS error so the test suite works

	* bus/activation.c (handle_activation_exit_error): return
	DBUS_ERROR_NO_MEMORY if we get BUS_SPAWN_EXIT_CODE_NO_MEMORY

	* dbus/dbus-spawn.c (_dbus_babysitter_get_child_exit_status):
	return only the exit code of the child, not the entire thingy from
	waitpid(), and make the return value indicate whether the child
	exited normally (with a status code)

	* bus/bus.c (process_config_first_time_only): _dbus_strdup works
	on NULL so no need to check
	(process_config_every_time): move servicehelper init here, so we
	reload it on HUP or config file change

	* bus/Makefile.am (install-data-hook): remove comment because
	Emacs make mode seems to be grumpy about it

10 files changed, 132 insertions, 40 deletions

diff --git a/ChangeLog b/ChangeLog
index f580a32f..3f7918ef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+2007-07-24  Havoc Pennington  <hp@redhat.com>
+
+	* configure.in: add AM_PROG_CC_C_O to allow per-target CPPFLAGS
+
+	* bus/dispatch.c (bus_dispatch_test_conf): Fix up setting
+	TEST_LAUNCH_HELPER_CONFIG to include the full path, and enable
+	test shell_fail_service_auto_start when use_launcher==TRUE
+
+	* bus/activation-helper-bin.c (convert_error_to_exit_code): pass
+	through the INVALID_ARGS error so the test suite works
+
+	* bus/activation.c (handle_activation_exit_error): return
+	DBUS_ERROR_NO_MEMORY if we get BUS_SPAWN_EXIT_CODE_NO_MEMORY
+
+	* dbus/dbus-spawn.c (_dbus_babysitter_get_child_exit_status):
+	return only the exit code of the child, not the entire thingy from 
+	waitpid(), and make the return value indicate whether the child 
+	exited normally (with a status code)
+
+	* bus/bus.c (process_config_first_time_only): _dbus_strdup works
+	on NULL so no need to check
+	(process_config_every_time): move servicehelper init here, so we
+	reload it on HUP or config file change
+
+	* bus/Makefile.am (install-data-hook): remove comment because
+	Emacs make mode seems to be grumpy about it
+
 2007-07-24  Richard Hughes  <richard@hughsie.com>
 
 	* bus/Makefile.am:
diff --git a/bus/Makefile.am b/bus/Makefile.am
index d4eae307..d521fa7b 100644
--- a/bus/Makefile.am
+++ b/bus/Makefile.am
@@ -188,7 +188,6 @@ install-data-hook:
 	$(mkinstalldirs) $(DESTDIR)$(datadir)/dbus-1/system-services
 	$(mkinstalldirs) $(DESTDIR)$(libexecdir)/dbus-1
 	$(INSTALL_PROGRAM) dbus-daemon-launch-helper $(DESTDIR)$(libexecdir)
-	## This is SETUID! <insert suitable warning here...>
 	if test `id -u` -eq 0; then \
 		chown root:$(DBUS_USER) $(DESTDIR)$(libexecdir)/dbus-daemon-launch-helper; \
 		chmod 4750 $(DESTDIR)$(libexecdir)/dbus-daemon-launch-helper; \
diff --git a/bus/activation-exit-codes.h b/bus/activation-exit-codes.h
index 0e858f14..86a005ce 100644
--- a/bus/activation-exit-codes.h
+++ b/bus/activation-exit-codes.h
@@ -34,5 +34,6 @@
 #define BUS_SPAWN_EXIT_CODE_PERMISSIONS_INVALID  6
 #define BUS_SPAWN_EXIT_CODE_FILE_INVALID         7
 #define BUS_SPAWN_EXIT_CODE_EXEC_FAILED          8
+#define BUS_SPAWN_EXIT_CODE_INVALID_ARGS         9
 
 #endif /* BUS_ACTIVATION_EXIT_CODES_H */
diff --git a/bus/activation-helper-bin.c b/bus/activation-helper-bin.c
index 6b9ec1f5..248a86f7 100644
--- a/bus/activation-helper-bin.c
+++ b/bus/activation-helper-bin.c
@@ -56,7 +56,12 @@ convert_error_to_exit_code (DBusError *error)
   if (dbus_error_has_name (error, DBUS_ERROR_SPAWN_EXEC_FAILED))
     return BUS_SPAWN_EXIT_CODE_EXEC_FAILED;
 
+  if (dbus_error_has_name (error, DBUS_ERROR_INVALID_ARGS))
+    return BUS_SPAWN_EXIT_CODE_INVALID_ARGS;
+  
   /* should we assert? */
+  fprintf(stderr, "%s: %s\n", error->name, error->message);
+  
   return BUS_SPAWN_EXIT_CODE_SETUP_FAILED;
 }
 
diff --git a/bus/activation.c b/bus/activation.c
index c49bc881..d5162ec2 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -1122,13 +1122,14 @@ pending_activation_failed (BusPendingActivation *pending_activation,
  * Depending on the exit code of the helper, set the error accordingly
  */
 static void
-handle_activation_exit_error (int exit_code, DBusError *error)
+handle_activation_exit_error (int        exit_code,
+                              DBusError *error)
 {
   switch (exit_code)
     {
     case BUS_SPAWN_EXIT_CODE_NO_MEMORY:
-      dbus_set_error (error, DBUS_ERROR_SPAWN_SETUP_FAILED,
-                      "Launcher could not run as out of memory");
+      dbus_set_error (error, DBUS_ERROR_NO_MEMORY,
+                      "Launcher could not run (out of memory)");
       break;
     case BUS_SPAWN_EXIT_CODE_SETUP_FAILED:
       dbus_set_error (error, DBUS_ERROR_SPAWN_SETUP_FAILED,
@@ -1154,6 +1155,10 @@ handle_activation_exit_error (int exit_code, DBusError *error)
       dbus_set_error (error, DBUS_ERROR_SPAWN_EXEC_FAILED,
                       "Cannot launch daemon, file not found or permissions invalid");
       break;
+    case BUS_SPAWN_EXIT_CODE_INVALID_ARGS:
+      dbus_set_error (error, DBUS_ERROR_INVALID_ARGS,
+                      "Invalid arguments to command line");
+      break;
     default:
       dbus_set_error (error, DBUS_ERROR_SPAWN_CHILD_EXITED,
                       "Launch helper exited with unknown return code %i", exit_code);
diff --git a/bus/bus.c b/bus/bus.c
index 627e49d3..21e8037e 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -254,8 +254,9 @@ setup_server (BusContext *context,
 }
 
 /* This code only gets executed the first time the
-   config files are parsed.  It is not executed
-   when config files are reloaded.*/
+ * config files are parsed.  It is not executed
+ * when config files are reloaded.
+ */
 static dbus_bool_t
 process_config_first_time_only (BusContext      *context,
 				BusConfigParser *parser,
@@ -263,7 +264,7 @@ process_config_first_time_only (BusContext      *context,
 {
   DBusList *link;
   DBusList **addresses;
-  const char *user, *pidfile, *servicehelper;
+  const char *user, *pidfile;
   char **auth_mechanisms;
   DBusList **auth_mechanisms_list;
   int len;
@@ -299,11 +300,6 @@ process_config_first_time_only (BusContext      *context,
   /* keep around the pid filename so we can delete it later */
   context->pidfile = _dbus_strdup (pidfile);
 
-  /* we need to configure this so we can test the service helper */
-  servicehelper = bus_config_parser_get_servicehelper (parser);
-  if (servicehelper != NULL)
-    context->servicehelper = _dbus_strdup (servicehelper);
-
   /* Build an array of auth mechanisms */
   
   auth_mechanisms_list = bus_config_parser_get_mechanisms (parser);
@@ -398,8 +394,11 @@ process_config_first_time_only (BusContext      *context,
 }
 
 /* This code gets executed every time the config files
-   are parsed: both during BusContext construction
-   and on reloads. */
+ * are parsed: both during BusContext construction
+ * and on reloads. This function is slightly screwy
+ * since it can do a "half reload" in out-of-memory
+ * situations. Realistically, unlikely to ever matter.
+ */
 static dbus_bool_t
 process_config_every_time (BusContext      *context,
 			   BusConfigParser *parser,
@@ -411,7 +410,9 @@ process_config_every_time (BusContext      *context,
   DBusList **dirs;
   BusActivation *new_activation;
   char *addr;
-
+  const char *servicehelper;
+  char *s;
+  
   dbus_bool_t retval;
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
@@ -477,6 +478,21 @@ process_config_every_time (BusContext      *context,
   /* get the service directories */
   dirs = bus_config_parser_get_service_dirs (parser);
 
+  /* and the service helper */
+  servicehelper = bus_config_parser_get_servicehelper (parser);
+
+  s = _dbus_strdup(servicehelper);
+  if (s == NULL && servicehelper != NULL)
+    {
+      BUS_SET_OOM (error);
+      goto failed;
+    }
+  else
+    {
+      dbus_free(context->servicehelper);
+      context->servicehelper = s;
+    }
+  
   /* Create activation subsystem */
   new_activation = bus_activation_new (context, &full_address,
                                        dirs, error);
diff --git a/bus/config-parser.c b/bus/config-parser.c
index 43e516e3..d6df4abb 100644
--- a/bus/config-parser.c
+++ b/bus/config-parser.c
@@ -2073,15 +2073,8 @@ servicehelper_path (BusConfigParser   *parser,
 {
   const char *filename_str;
   char *servicehelper;
-  filename_str = _dbus_string_get_const_data (filename);
 
-  /* check if helper exists... */
-  if (!_dbus_file_exists (filename_str))
-    {
-      dbus_set_error (error, DBUS_ERROR_FILE_NOT_FOUND,
-                      "setuid helper '%s'not found", filename_str);
-      return FALSE;
-    }
+  filename_str = _dbus_string_get_const_data (filename);
 
   /* copy to avoid overwriting with NULL on OOM */
   servicehelper = _dbus_strdup (filename_str);
@@ -2097,6 +2090,13 @@ servicehelper_path (BusConfigParser   *parser,
   dbus_free (parser->servicehelper);
   parser->servicehelper = servicehelper;
 
+  /* We don't check whether the helper exists; instead we
+   * would just fail to ever activate anything if it doesn't.
+   * This allows an admin to fix the problem if it doesn't exist.
+   * It also allows the parser test suite to successfully parse
+   * test cases without installing the helper. ;-)
+   */
+  
   return TRUE;
 }
 
diff --git a/bus/dispatch.c b/bus/dispatch.c
index b020bb0b..d434e808 100644
--- a/bus/dispatch.c
+++ b/bus/dispatch.c
@@ -4408,6 +4408,40 @@ check2_try_iterations (BusContext     *context,
 }
 
 static dbus_bool_t
+setenv_TEST_LAUNCH_HELPER_CONFIG(const DBusString *test_data_dir,
+                                 const char       *filename)
+{
+  DBusString full;
+  DBusString file;
+
+  if (!_dbus_string_init (&full))
+    return FALSE;
+
+  if (!_dbus_string_copy (test_data_dir, 0, &full, 0))
+    {
+      _dbus_string_free (&full);
+      return FALSE;
+    }      
+
+  _dbus_string_init_const (&file, filename);
+  
+  if (!_dbus_concat_dir_and_file (&full, &file))
+    {
+      _dbus_string_free (&full);
+      return FALSE;
+    }
+
+  _dbus_verbose ("Setting TEST_LAUNCH_HELPER_CONFIG to '%s'\n", 
+                 _dbus_string_get_const_data (&full));
+  
+  _dbus_setenv ("TEST_LAUNCH_HELPER_CONFIG", _dbus_string_get_const_data (&full));
+
+  _dbus_string_free (&full);
+
+  return TRUE;
+}
+
+static dbus_bool_t
 bus_dispatch_test_conf (const DBusString *test_data_dir,
 		        const char       *filename,
 		        dbus_bool_t       use_launcher)
@@ -4419,7 +4453,8 @@ bus_dispatch_test_conf (const DBusString *test_data_dir,
   DBusError error;
 
   /* save the config name for the activation helper */
-  _dbus_setenv ("TEST_LAUNCH_HELPER_CONFIG", filename);
+  if (!setenv_TEST_LAUNCH_HELPER_CONFIG (test_data_dir, filename))
+    _dbus_assert_not_reached ("no memory setting TEST_LAUNCH_HELPER_CONFIG");
 
   dbus_error_init (&error);
   
@@ -4524,9 +4559,8 @@ bus_dispatch_test_conf (const DBusString *test_data_dir,
 #endif
 
   /* only do the shell fail test if we are not using the launcher */
-  if (!use_launcher)
-    check2_try_iterations (context, foo, "shell_fail_service_auto_start",
-                           check_shell_fail_service_auto_start);
+  check2_try_iterations (context, foo, "shell_fail_service_auto_start",
+                         check_shell_fail_service_auto_start);
 
   /* specific to launcher */
   if (use_launcher)
@@ -4572,8 +4606,9 @@ bus_dispatch_test_conf_fail (const DBusString *test_data_dir,
   DBusError error;
 
   /* save the config name for the activation helper */
-  _dbus_setenv ("TEST_LAUNCH_HELPER_CONFIG", filename);
-
+  if (!setenv_TEST_LAUNCH_HELPER_CONFIG (test_data_dir, filename))
+    _dbus_assert_not_reached ("no memory setting TEST_LAUNCH_HELPER_CONFIG");
+  
   dbus_error_init (&error);
   
   context = bus_context_new_test (test_data_dir, filename);
diff --git a/configure.in b/configure.in
index d999f43b..7b7cf262 100644
--- a/configure.in
+++ b/configure.in
@@ -41,6 +41,7 @@ AC_SUBST(LT_AGE)
 
 
 AC_PROG_CC
+AM_PROG_CC_C_O
 AC_PROG_CXX
 AC_ISC_POSIX
 AC_HEADER_STDC
diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c
index 125aea7c..810536fd 100644
--- a/dbus/dbus-spawn.c
+++ b/dbus/dbus-spawn.c
@@ -417,7 +417,7 @@ read_data (DBusBabysitter *sitter,
                   {
                     sitter->have_child_status = TRUE;
                     sitter->status = arg;
-                    sitter->errnum = WEXITSTATUS (sitter->status);
+                    sitter->errnum = 0;
                     _dbus_verbose ("recorded child status exited = %d signaled = %d exitstatus = %d termsig = %d\n",
                                    WIFEXITED (sitter->status), WIFSIGNALED (sitter->status),
                                    WEXITSTATUS (sitter->status), WTERMSIG (sitter->status));
@@ -624,26 +624,29 @@ _dbus_babysitter_get_child_exited (DBusBabysitter *sitter)
 }
 
 /**
- * Gets the exit status of the child. We do this so implimentation specific
- * detail is not cluttering up dbus, for example the system laucher code.
+ * Gets the exit status of the child. We do this so implementation specific
+ * detail is not cluttering up dbus, for example the system launcher code.
+ * This can only be called if the child has exited, i.e. call
+ * _dbus_babysitter_get_child_exited(). It returns FALSE if the child
+ * did not return a status code, e.g. because the child was signaled
+ * or we failed to ever launch the child in the first place.
  *
  * @param sitter the babysitter
  * @param status the returned status code
  * @returns #FALSE on failure
  */
 dbus_bool_t
-_dbus_babysitter_get_child_exit_status (DBusBabysitter *sitter, int *status)
+_dbus_babysitter_get_child_exit_status (DBusBabysitter *sitter,
+                                        int            *status)
 {
   if (!_dbus_babysitter_get_child_exited (sitter))
     _dbus_assert_not_reached ("Child has not exited");
+  
+  if (!sitter->have_child_status ||
+      !(WIFEXITED (sitter->status)))
+    return FALSE;
 
-  if (sitter->errnum != WEXITSTATUS (sitter->status))
-    _dbus_assert_not_reached ("Status is not exit!");
-
-  if (!sitter->have_child_status)
-    _dbus_assert_not_reached ("Not a child!");
-
-  *status = sitter->status;
+  *status = WEXITSTATUS (sitter->status);
   return TRUE;
 }