diff --git a/gio/gsubprocess.c b/gio/gsubprocess.c index 665d729ed..77a23efc3 100644 --- a/gio/gsubprocess.c +++ b/gio/gsubprocess.c @@ -179,160 +179,6 @@ enum N_PROPS }; -#ifdef G_OS_UNIX -typedef struct -{ - gint fds[3]; - GSpawnChildSetupFunc child_setup_func; - gpointer child_setup_data; - GArray *basic_fd_assignments; - GArray *needdup_fd_assignments; -} ChildData; - -static void -unset_cloexec (int fd) -{ - int flags; - int result; - - flags = fcntl (fd, F_GETFD, 0); - - if (flags != -1) - { - int errsv; - flags &= (~FD_CLOEXEC); - do - { - result = fcntl (fd, F_SETFD, flags); - errsv = errno; - } - while (result == -1 && errsv == EINTR); - } -} - -static int -dupfd_cloexec (int parent_fd) -{ - int fd, errsv; -#ifdef F_DUPFD_CLOEXEC - do - { - fd = fcntl (parent_fd, F_DUPFD_CLOEXEC, 3); - errsv = errno; - } - while (fd == -1 && errsv == EINTR); -#else - /* OS X Snow Lion and earlier don't have F_DUPFD_CLOEXEC: - * https://bugzilla.gnome.org/show_bug.cgi?id=710962 - */ - int result, flags; - do - { - fd = fcntl (parent_fd, F_DUPFD, 3); - errsv = errno; - } - while (fd == -1 && errsv == EINTR); - flags = fcntl (fd, F_GETFD, 0); - if (flags != -1) - { - flags |= FD_CLOEXEC; - do - { - result = fcntl (fd, F_SETFD, flags); - errsv = errno; - } - while (result == -1 && errsv == EINTR); - } -#endif - return fd; -} - -/* - * Based on code derived from - * gnome-terminal:src/terminal-screen.c:terminal_screen_child_setup(), - * used under the LGPLv2+ with permission from author. - */ -static void -child_setup (gpointer user_data) -{ - ChildData *child_data = user_data; - guint i; - gint result; - int errsv; - - /* We're on the child side now. "Rename" the file descriptors in - * child_data.fds[] to stdin/stdout/stderr. - * - * We don't close the originals. It's possible that the originals - * should not be closed and if they should be closed then they should - * have been created O_CLOEXEC. - */ - for (i = 0; i < 3; i++) - if (child_data->fds[i] != -1 && child_data->fds[i] != (gint) i) - { - do - { - result = dup2 (child_data->fds[i], i); - errsv = errno; - } - while (result == -1 && errsv == EINTR); - } - - /* Basic fd assignments we can just unset FD_CLOEXEC */ - if (child_data->basic_fd_assignments) - { - for (i = 0; i < child_data->basic_fd_assignments->len; i++) - { - gint fd = g_array_index (child_data->basic_fd_assignments, int, i); - - unset_cloexec (fd); - } - } - - /* If we're doing remapping fd assignments, we need to handle - * the case where the user has specified e.g.: - * 5 -> 4, 4 -> 6 - * - * We do this by duping the source fds temporarily. - */ - if (child_data->needdup_fd_assignments) - { - for (i = 0; i < child_data->needdup_fd_assignments->len; i += 2) - { - gint parent_fd = g_array_index (child_data->needdup_fd_assignments, int, i); - gint new_parent_fd; - - new_parent_fd = dupfd_cloexec (parent_fd); - - g_array_index (child_data->needdup_fd_assignments, int, i) = new_parent_fd; - } - for (i = 0; i < child_data->needdup_fd_assignments->len; i += 2) - { - gint parent_fd = g_array_index (child_data->needdup_fd_assignments, int, i); - gint child_fd = g_array_index (child_data->needdup_fd_assignments, int, i+1); - - if (parent_fd == child_fd) - { - unset_cloexec (parent_fd); - } - else - { - do - { - result = dup2 (parent_fd, child_fd); - errsv = errno; - } - while (result == -1 && errsv == EINTR); - (void) close (parent_fd); - } - } - } - - if (child_data->child_setup_func) - child_data->child_setup_func (child_data->child_setup_data); -} -#endif - static GInputStream * platform_input_stream_from_spawn_fd (gint fd) { @@ -450,12 +296,12 @@ initable_init (GInitable *initable, GError **error) { GSubprocess *self = G_SUBPROCESS (initable); -#ifdef G_OS_UNIX - ChildData child_data = { { -1, -1, -1 }, 0, NULL, NULL, NULL }; -#endif gint *pipe_ptrs[3] = { NULL, NULL, NULL }; gint pipe_fds[3] = { -1, -1, -1 }; gint close_fds[3] = { -1, -1, -1 }; +#ifdef G_OS_UNIX + gint stdin_fd = -1, stdout_fd = -1, stderr_fd = -1; +#endif GSpawnFlags spawn_flags = 0; gboolean success = FALSE; gint i; @@ -480,11 +326,11 @@ initable_init (GInitable *initable, else if (self->launcher) { if (self->launcher->stdin_fd != -1) - child_data.fds[0] = self->launcher->stdin_fd; + stdin_fd = self->launcher->stdin_fd; else if (self->launcher->stdin_path != NULL) { - child_data.fds[0] = close_fds[0] = unix_open_file (self->launcher->stdin_path, O_RDONLY, error); - if (child_data.fds[0] == -1) + stdin_fd = close_fds[0] = unix_open_file (self->launcher->stdin_path, O_RDONLY, error); + if (stdin_fd == -1) goto out; } } @@ -499,11 +345,11 @@ initable_init (GInitable *initable, else if (self->launcher) { if (self->launcher->stdout_fd != -1) - child_data.fds[1] = self->launcher->stdout_fd; + stdout_fd = self->launcher->stdout_fd; else if (self->launcher->stdout_path != NULL) { - child_data.fds[1] = close_fds[1] = unix_open_file (self->launcher->stdout_path, O_CREAT | O_WRONLY, error); - if (child_data.fds[1] == -1) + stdout_fd = close_fds[1] = unix_open_file (self->launcher->stdout_path, O_CREAT | O_WRONLY, error); + if (stdout_fd == -1) goto out; } } @@ -516,29 +362,21 @@ initable_init (GInitable *initable, pipe_ptrs[2] = &pipe_fds[2]; #ifdef G_OS_UNIX else if (self->flags & G_SUBPROCESS_FLAGS_STDERR_MERGE) - /* This will work because stderr gets setup after stdout. */ - child_data.fds[2] = 1; + /* This will work because stderr gets set up after stdout. */ + stderr_fd = 1; else if (self->launcher) { if (self->launcher->stderr_fd != -1) - child_data.fds[2] = self->launcher->stderr_fd; + stderr_fd = self->launcher->stderr_fd; else if (self->launcher->stderr_path != NULL) { - child_data.fds[2] = close_fds[2] = unix_open_file (self->launcher->stderr_path, O_CREAT | O_WRONLY, error); - if (child_data.fds[2] == -1) + stderr_fd = close_fds[2] = unix_open_file (self->launcher->stderr_path, O_CREAT | O_WRONLY, error); + if (stderr_fd == -1) goto out; } } #endif -#ifdef G_OS_UNIX - if (self->launcher) - { - child_data.basic_fd_assignments = self->launcher->basic_fd_assignments; - child_data.needdup_fd_assignments = self->launcher->needdup_fd_assignments; - } -#endif - /* argv0 has no '/' in it? We better do a PATH lookup. */ if (strchr (self->argv[0], G_DIR_SEPARATOR) == NULL) { @@ -554,23 +392,25 @@ initable_init (GInitable *initable, spawn_flags |= G_SPAWN_DO_NOT_REAP_CHILD; spawn_flags |= G_SPAWN_CLOEXEC_PIPES; + success = g_spawn_async_with_pipes_and_fds (self->launcher ? self->launcher->cwd : NULL, + (const gchar * const *) self->argv, + (const gchar * const *) (self->launcher ? self->launcher->envp : NULL), + spawn_flags, #ifdef G_OS_UNIX - child_data.child_setup_func = self->launcher ? self->launcher->child_setup_func : NULL; - child_data.child_setup_data = self->launcher ? self->launcher->child_setup_user_data : NULL; -#endif - - success = g_spawn_async_with_pipes (self->launcher ? self->launcher->cwd : NULL, - self->argv, - self->launcher ? self->launcher->envp : NULL, - spawn_flags, -#ifdef G_OS_UNIX - child_setup, &child_data, + self->launcher ? self->launcher->child_setup_func : NULL, + self->launcher ? self->launcher->child_setup_user_data : NULL, + stdin_fd, stdout_fd, stderr_fd, + self->launcher ? (const gint *) self->launcher->source_fds->data : NULL, + self->launcher ? (const gint *) self->launcher->target_fds->data : NULL, + self->launcher ? self->launcher->source_fds->len : 0, #else - NULL, NULL, + NULL, NULL, + -1, -1, -1, + NULL, NULL, 0, #endif - &self->pid, - pipe_ptrs[0], pipe_ptrs[1], pipe_ptrs[2], - error); + &self->pid, + pipe_ptrs[0], pipe_ptrs[1], pipe_ptrs[2], + error); g_assert (success == (self->pid != 0)); { diff --git a/gio/gsubprocesslauncher-private.h b/gio/gsubprocesslauncher-private.h index 95431a0ab..f8a6516c5 100644 --- a/gio/gsubprocesslauncher-private.h +++ b/gio/gsubprocesslauncher-private.h @@ -42,8 +42,8 @@ struct _GSubprocessLauncher gint stderr_fd; gchar *stderr_path; - GArray *basic_fd_assignments; - GArray *needdup_fd_assignments; + GArray *source_fds; + GArray *target_fds; /* always the same length as source_fds */ gboolean closed_fd; GSpawnChildSetupFunc child_setup_func; diff --git a/gio/gsubprocesslauncher.c b/gio/gsubprocesslauncher.c index 9e077bd20..b7257f453 100644 --- a/gio/gsubprocesslauncher.c +++ b/gio/gsubprocesslauncher.c @@ -164,8 +164,8 @@ g_subprocess_launcher_init (GSubprocessLauncher *self) self->stdin_fd = -1; self->stdout_fd = -1; self->stderr_fd = -1; - self->basic_fd_assignments = g_array_new (FALSE, 0, sizeof (int)); - self->needdup_fd_assignments = g_array_new (FALSE, 0, sizeof (int)); + self->source_fds = g_array_new (FALSE, 0, sizeof (int)); + self->target_fds = g_array_new (FALSE, 0, sizeof (int)); #endif } @@ -613,18 +613,10 @@ g_subprocess_launcher_take_fd (GSubprocessLauncher *self, gint source_fd, gint target_fd) { - if (source_fd == target_fd) + if (self->source_fds != NULL && self->target_fds != NULL) { - if (self->basic_fd_assignments) - g_array_append_val (self->basic_fd_assignments, source_fd); - } - else - { - if (self->needdup_fd_assignments) - { - g_array_append_val (self->needdup_fd_assignments, source_fd); - g_array_append_val (self->needdup_fd_assignments, target_fd); - } + g_array_append_val (self->source_fds, source_fd); + g_array_append_val (self->target_fds, target_fd); } } @@ -664,17 +656,18 @@ g_subprocess_launcher_close (GSubprocessLauncher *self) close (self->stderr_fd); self->stderr_fd = -1; - if (self->basic_fd_assignments) + if (self->source_fds) { - for (i = 0; i < self->basic_fd_assignments->len; i++) - (void) close (g_array_index (self->basic_fd_assignments, int, i)); - g_clear_pointer (&self->basic_fd_assignments, g_array_unref); - } - if (self->needdup_fd_assignments) - { - for (i = 0; i < self->needdup_fd_assignments->len; i += 2) - (void) close (g_array_index (self->needdup_fd_assignments, int, i)); - g_clear_pointer (&self->needdup_fd_assignments, g_array_unref); + g_assert (self->target_fds != NULL); + g_assert (self->source_fds->len == self->target_fds->len); + + for (i = 0; i < self->source_fds->len; i++) + { + (void) close (g_array_index (self->source_fds, int, i)); + (void) close (g_array_index (self->target_fds, int, i)); + } + g_clear_pointer (&self->source_fds, g_array_unref); + g_clear_pointer (&self->target_fds, g_array_unref); } self->closed_fd = TRUE;