DBusBabysitter: change executable to log_name

DBusBabysitter->executable is defined as executable name to use in error
messages. However, if servicehelper used, then the executable name is
servicehelper. It's not much help because we couldn't figure out which
service we're trying to activated if error happens.

In the following patch, we'll use service name to be activated as the
child log identifier and add a parameter to
_dbus_spawn_async_with_babysitter() to pass the log identifier.  Since
this is not the case in test, so executable changed to log_name.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=68559
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>

diff --git a/bus/activation.c b/bus/activation.c
index 77357beab6f3b4b1646fb961ccbe0f589088d64a..868ce05c13809ef9fc0f08ac16b5f39b1dc23f07 100644 (file)
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -2104,7 +2104,9 @@ bus_activation_activate_service (BusActivation  *activation,
 
   dbus_error_init (&tmp_error);
 
-  if (!_dbus_spawn_async_with_babysitter (&pending_activation->babysitter, argv,
+  if (!_dbus_spawn_async_with_babysitter (&pending_activation->babysitter,
+                                          service_name,
+                                          argv,
                                           envp,
                                           NULL, activation,
                                           &tmp_error))

diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c
index cd8ca6668f51891e11793dc2f0ae9ac19c350862..7da7a431bf3e1158ee9ba27b4a67d5c48a861337 100644 (file)
--- a/dbus/dbus-spawn-win.c
+++ b/dbus/dbus-spawn-win.c
@@ -68,7 +68,7 @@ struct DBusBabysitter
     HANDLE end_sync_event;
 #endif
 
-    char *executable;
+    char *log_name;
     DBusSpawnChildSetupFunc child_setup;
     void *user_data;
 
@@ -258,7 +258,7 @@ _dbus_babysitter_unref (DBusBabysitter *sitter)
         }
 #endif
 
-      dbus_free (sitter->executable);
+      dbus_free (sitter->log_name);
 
       dbus_free (sitter);
     }
@@ -337,7 +337,7 @@ _dbus_babysitter_set_child_exit_error (DBusBabysitter *sitter,
       char *emsg = _dbus_win_error_string (sitter->spawn_errno);
       dbus_set_error (error, DBUS_ERROR_SPAWN_EXEC_FAILED,
                       "Failed to execute program %s: %s",
-                      sitter->executable, emsg);
+                      sitter->log_name, emsg);
       _dbus_win_free_error_string (emsg);
     }
   else if (sitter->have_child_status)
@@ -345,14 +345,14 @@ _dbus_babysitter_set_child_exit_error (DBusBabysitter *sitter,
       PING();
       dbus_set_error (error, DBUS_ERROR_SPAWN_CHILD_EXITED,
                       "Process %s exited with status %d",
-                      sitter->executable, sitter->child_status);
+                      sitter->log_name, sitter->child_status);
     }
   else
     {
       PING();
       dbus_set_error (error, DBUS_ERROR_FAILED,
                       "Process %s exited, status unknown",
-                      sitter->executable);
+                      sitter->log_name);
     }
   PING();
 }
@@ -593,10 +593,10 @@ babysitter (void *parameter)
       (*sitter->child_setup) (sitter->user_data);
     }
 
-  _dbus_verbose ("babysitter: spawning %s\n", sitter->executable);
+  _dbus_verbose ("babysitter: spawning %s\n", sitter->log_name);
 
   PING();
-  sitter->child_handle = spawn_program (sitter->executable,
+  sitter->child_handle = spawn_program (sitter->log_name,
                                        sitter->argv, sitter->envp);
 
   PING();
@@ -642,6 +642,7 @@ babysitter (void *parameter)
 
 dbus_bool_t
 _dbus_spawn_async_with_babysitter (DBusBabysitter           **sitter_p,
+                                   const char                *log_name,
                                    char                     **argv,
                                    char                     **envp,
                                    DBusSpawnChildSetupFunc    child_setup,
@@ -653,6 +654,7 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter           **sitter_p,
   DWORD sitter_thread_id;
   
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
+  _dbus_assert (argv[0] != NULL);
 
   *sitter_p = NULL;
 
@@ -667,8 +669,17 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter           **sitter_p,
   sitter->child_setup = child_setup;
   sitter->user_data = user_data;
 
-  sitter->executable = _dbus_strdup (argv[0]);
-  if (sitter->executable == NULL)
+  sitter->log_name = _dbus_strdup (log_name);
+  if (sitter->log_name == NULL && log_name != NULL)
+    {
+      _DBUS_SET_OOM (error);
+      goto out0;
+    }
+
+  if (sitter->log_name == NULL)
+    sitter->log_name = _dbus_strdup (argv[0]);
+
+  if (sitter->log_name == NULL)
     {
       _DBUS_SET_OOM (error);
       goto out0;
@@ -804,7 +815,7 @@ check_spawn_nonexistent (void *data)
   /*** Test launching nonexistent binary */
 
   argv[0] = "/this/does/not/exist/32542sdgafgafdg";
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv, NULL,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_nonexistent", argv, NULL,
                                          NULL, NULL,
                                          &error))
     {
@@ -857,7 +868,7 @@ check_spawn_segfault (void *data)
       return TRUE;
     }
 
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv, NULL,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_segfault", argv, NULL,
                                          NULL, NULL,
                                          &error))
     {
@@ -912,7 +923,7 @@ check_spawn_exit (void *data)
       return TRUE;
     }
 
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv, NULL,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_exit", argv, NULL,
                                          NULL, NULL,
                                          &error))
     {
@@ -967,7 +978,7 @@ check_spawn_and_kill (void *data)
       return TRUE;
     }
 
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv, NULL,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_and_kill", argv, NULL,
                                          NULL, NULL,
                                          &error))
     {

diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c
index 3d8ed2e8ca58aff50da4a1d66c7c4ed3711eba3f..a0ff2f4e551579f3fd40c77756e96ba01f160fa0 100644 (file)
--- a/dbus/dbus-spawn.c
+++ b/dbus/dbus-spawn.c
@@ -234,7 +234,8 @@ struct DBusBabysitter
 {
   int refcount; /**< Reference count */
 
-  char *executable; /**< executable name to use in error messages */
+  char *log_name; /**< the name under which to log messages about this
+                   process being spawned */
   
   int socket_to_babysitter; /**< Connection to the babysitter process */
   int error_pipe_from_child; /**< Connection to the process that does the exec() */
@@ -388,7 +389,7 @@ _dbus_babysitter_unref (DBusBabysitter *sitter)
       if (sitter->watches)
         _dbus_watch_list_free (sitter->watches);
 
-      dbus_free (sitter->executable);
+      dbus_free (sitter->log_name);
       
       dbus_free (sitter);
     }
@@ -743,34 +744,34 @@ _dbus_babysitter_set_child_exit_error (DBusBabysitter *sitter,
     {
       dbus_set_error (error, DBUS_ERROR_SPAWN_EXEC_FAILED,
                       "Failed to execute program %s: %s",
-                      sitter->executable, _dbus_strerror (sitter->errnum));
+                      sitter->log_name, _dbus_strerror (sitter->errnum));
     }
   else if (sitter->have_fork_errnum)
     {
       dbus_set_error (error, DBUS_ERROR_NO_MEMORY,
                       "Failed to fork a new process %s: %s",
-                      sitter->executable, _dbus_strerror (sitter->errnum));
+                      sitter->log_name, _dbus_strerror (sitter->errnum));
     }
   else if (sitter->have_child_status)
     {
       if (WIFEXITED (sitter->status))
         dbus_set_error (error, DBUS_ERROR_SPAWN_CHILD_EXITED,
                         "Process %s exited with status %d",
-                        sitter->executable, WEXITSTATUS (sitter->status));
+                        sitter->log_name, WEXITSTATUS (sitter->status));
       else if (WIFSIGNALED (sitter->status))
         dbus_set_error (error, DBUS_ERROR_SPAWN_CHILD_SIGNALED,
                         "Process %s received signal %d",
-                        sitter->executable, WTERMSIG (sitter->status));
+                        sitter->log_name, WTERMSIG (sitter->status));
       else
         dbus_set_error (error, DBUS_ERROR_FAILED,
                         "Process %s exited abnormally",
-                        sitter->executable);
+                        sitter->log_name);
     }
   else
     {
       dbus_set_error (error, DBUS_ERROR_FAILED,
                       "Process %s exited, reason unknown",
-                      sitter->executable);
+                      sitter->log_name);
     }
 }
 
@@ -1165,8 +1166,7 @@ babysit (pid_t grandchild_pid,
 }
 
 /**
- * Spawns a new process. The executable name and argv[0]
- * are the same, both are provided in argv[0]. The child_setup
+ * Spawns a new process. The child_setup
  * function is passed the given user_data and is run in the child
  * just before calling exec().
  *
@@ -1176,6 +1176,7 @@ babysit (pid_t grandchild_pid,
  * If sitter_p is #NULL, no babysitter is kept.
  *
  * @param sitter_p return location for babysitter or #NULL
+ * @log_name the name under which to log messages about this process being spawned
  * @param argv the executable and arguments
  * @param env the environment, or #NULL to copy the parent's
  * @param child_setup function to call in child pre-exec()
@@ -1185,6 +1186,7 @@ babysit (pid_t grandchild_pid,
  */
 dbus_bool_t
 _dbus_spawn_async_with_babysitter (DBusBabysitter          **sitter_p,
+                                   const char               *log_name,
                                    char                    **argv,
                                    char                    **env,
                                    DBusSpawnChildSetupFunc   child_setup,
@@ -1197,6 +1199,7 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter          **sitter_p,
   pid_t pid;
   
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
+  _dbus_assert (argv[0] != NULL);
 
   if (sitter_p != NULL)
     *sitter_p = NULL;
@@ -1210,8 +1213,17 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter          **sitter_p,
       return FALSE;
     }
 
-  sitter->executable = _dbus_strdup (argv[0]);
-  if (sitter->executable == NULL)
+  sitter->log_name = _dbus_strdup (log_name);
+  if (sitter->log_name == NULL && log_name != NULL)
+    {
+      dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+      goto cleanup_and_fail;
+    }
+
+  if (sitter->log_name == NULL)
+    sitter->log_name = _dbus_strdup (argv[0]);
+
+  if (sitter->log_name == NULL)
     {
       dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
       goto cleanup_and_fail;
@@ -1418,7 +1430,7 @@ check_spawn_nonexistent (void *data)
   /*** Test launching nonexistent binary */
   
   argv[0] = "/this/does/not/exist/32542sdgafgafdg";
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_nonexistent", argv,
                                          NULL, NULL, NULL,
                                          &error))
     {
@@ -1467,7 +1479,7 @@ check_spawn_segfault (void *data)
       return TRUE;
     }
 
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_segfault", argv,
                                          NULL, NULL, NULL,
                                          &error))
     {
@@ -1518,7 +1530,7 @@ check_spawn_exit (void *data)
       return TRUE;
     }
 
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_exit", argv,
                                          NULL, NULL, NULL,
                                          &error))
     {
@@ -1569,7 +1581,7 @@ check_spawn_and_kill (void *data)
       return TRUE;
     }
 
-  if (_dbus_spawn_async_with_babysitter (&sitter, argv,
+  if (_dbus_spawn_async_with_babysitter (&sitter, "spawn_and_kill", argv,
                                          NULL, NULL, NULL,
                                          &error))
     {

diff --git a/dbus/dbus-spawn.h b/dbus/dbus-spawn.h
index a8814fb9f814d5a69d5f02aff0d6a0928c9ddb19..e6baae97e3fa561b3f7c39f2a43b590e1b3e48fa 100644 (file)
--- a/dbus/dbus-spawn.h
+++ b/dbus/dbus-spawn.h
@@ -39,6 +39,7 @@ typedef void (* DBusBabysitterFinishedFunc) (DBusBabysitter *sitter,
                                              void           *user_data);
 
 dbus_bool_t _dbus_spawn_async_with_babysitter     (DBusBabysitter           **sitter_p,
+                                                   const char                *log_name,
                                                    char                     **argv,
                                                    char                     **env,
                                                    DBusSpawnChildSetupFunc    child_setup,

diff --git a/test/spawn-test.c b/test/spawn-test.c
index e6513fa664776a9f77bebcf1af7265bc10d7ab2a..723a4889afb664158e53291feeccd1abdad9aa83 100644 (file)
--- a/test/spawn-test.c
+++ b/test/spawn-test.c
@@ -30,7 +30,7 @@ main (int argc, char **argv)
     argv_copy [i] = argv[i + 1];
   argv_copy[argc - 1] = NULL;
   
-  if (!_dbus_spawn_async_with_babysitter (NULL, argv_copy, NULL, setup_func, NULL, &error))
+  if (!_dbus_spawn_async_with_babysitter (NULL, argv_copy[0], argv_copy, NULL, setup_func, NULL, &error))
     {
       fprintf (stderr, "Could not launch application: \"%s\"\n",
               error.message);