From 3d92b1dfe4ed34cf3d6daff06681d69ee96804a2 Mon Sep 17 00:00:00 2001 From: Chengwei Yang 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