Attachment 87645 Details for Bug 68559 – [PATCH v4 1/3] DBusBabysitter: change executable to log_name

[patch] [PATCH v4 1/3] DBusBabysitter: change executable to log_name

0001-DBusBabysitter-change-executable-to-log_name.patch (text/plain), 13.47 KB, created by Chengwei Yang on 2013-10-15 05:33:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chengwei Yang

Created: 2013-10-15 05:33:59 UTC

Size: 13.47 KB

patch

obsolete

>From 3d92b1dfe4ed34cf3d6daff06681d69ee96804a2 Mon Sep 17 00:00:00 2001
>From: Chengwei Yang <chengwei.yang@intel.com>
>Date: Tue, 15 Oct 2013 13:04:53 +0800
>Subject: [PATCH v4 1/3] 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
>---
> bus/activation.c      |    4 +++-
> dbus/dbus-spawn-win.c |   37 ++++++++++++++++++++++++-------------
> dbus/dbus-spawn.c     |   44 ++++++++++++++++++++++++++++----------------
> dbus/dbus-spawn.h     |    1 +
> test/spawn-test.c     |    2 +-
> 5 files changed, 57 insertions(+), 31 deletions(-)
>
>diff --git a/bus/activation.c b/bus/activation.c
>index 77357be..868ce05 100644
>--- 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 cd8ca66..7da7a43 100644
>--- 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 8965b58..f2386b5 100644
>--- a/dbus/dbus-spawn.c
>+++ b/dbus/dbus-spawn.c
>@@ -192,7 +192,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() */
>@@ -346,7 +347,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);
>     }
>@@ -701,34 +702,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);
>     }
> }
> 
>@@ -1118,8 +1119,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().
>  *
>@@ -1129,6 +1129,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 (not used on unix yet)
>  * @param child_setup function to call in child pre-exec()
>@@ -1138,6 +1139,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,
>@@ -1150,6 +1152,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;
>@@ -1163,8 +1166,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;
>@@ -1371,7 +1383,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))
>     {
>@@ -1420,7 +1432,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))
>     {
>@@ -1471,7 +1483,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))
>     {
>@@ -1522,7 +1534,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 a8814fb..e6baae9 100644
>--- 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 e6513fa..723a488 100644
>--- 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);
>-- 
>1.7.9.5
>

Actions: View | Splinter Review

Attachments on bug 68559: 84633 | 85486 | 85487 | 85488 | 85767 | 85768 | 85769 | 87645 | 87646 | 87647

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.