From f7f597c8414f1da9bd5842a8fb26ee6631c18f45 Mon Sep 17 00:00:00 2001 From: Vasily Galkin Date: Thu, 27 Dec 2018 00:00:50 +0300 Subject: [PATCH] gspawn, win32: fix child stderr when coverage enabled This fixes test that were added in previous commit: checking for empty stderr failed with coverage enabled, since coverage warnings printed from gspawn-win32-helper process were treated as child output. This is fixed by removing redirection after child finishes execution. The dup_noninherited renamed to reopen_noninherited, since it actually always closes passed file descriptor. --- glib/gspawn-win32-helper.c | 13 ++++++++++--- glib/gspawn-win32.c | 8 ++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/glib/gspawn-win32-helper.c b/glib/gspawn-win32-helper.c index 045d90f71..bbb6161ef 100644 --- a/glib/gspawn-win32-helper.c +++ b/glib/gspawn-win32-helper.c @@ -179,6 +179,7 @@ main (int ignored_argc, char **ignored_argv) { int child_err_report_fd = -1; int helper_sync_fd = -1; + int saved_stderr_fd = -1; int i; int fd; int mode; @@ -280,6 +281,7 @@ main (int ignored_argc, char **ignored_argv) } } + saved_stderr_fd = reopen_noninherited (dup (2), _O_WRONLY); if (argv[ARG_STDERR][0] == '-') ; /* Nothing */ else if (argv[ARG_STDERR][0] == 'z') @@ -315,15 +317,15 @@ main (int ignored_argc, char **ignored_argv) */ if (argv[ARG_CLOSE_DESCRIPTORS][0] == 'y') for (i = 3; i < 1000; i++) /* FIXME real limit? */ - if (i != child_err_report_fd && i != helper_sync_fd) + if (i != child_err_report_fd && i != helper_sync_fd && i != saved_stderr_fd) if (_get_osfhandle (i) != -1) close (i); /* We don't want our child to inherit the error report and * helper sync fds. */ - child_err_report_fd = dup_noninherited (child_err_report_fd, _O_WRONLY); - helper_sync_fd = dup_noninherited (helper_sync_fd, _O_RDONLY); + child_err_report_fd = reopen_noninherited (child_err_report_fd, _O_WRONLY); + helper_sync_fd = reopen_noninherited (helper_sync_fd, _O_RDONLY); /* argv[ARG_WAIT] is "w" to wait for the program to exit */ if (argv[ARG_WAIT][0] == 'w') @@ -351,6 +353,11 @@ main (int ignored_argc, char **ignored_argv) saved_errno = errno; + /* Some coverage warnings may be printed on stderr during this process exit. + * Remove redirection so that they would go to original stderr + * instead of being treated as part of stderr of child process. + */ + dup2 (saved_stderr_fd, 2); if (handle == -1 && saved_errno != 0) { int ec = (saved_errno == ENOENT) diff --git a/glib/gspawn-win32.c b/glib/gspawn-win32.c index 3a746914b..c4cc8593b 100644 --- a/glib/gspawn-win32.c +++ b/glib/gspawn-win32.c @@ -105,8 +105,8 @@ enum { }; static int -dup_noninherited (int fd, - int mode) +reopen_noninherited (int fd, + int mode) { HANDLE filehandle; @@ -606,7 +606,7 @@ do_spawn_with_fds (gint *exit_status, * helper process, and the started actual user process. As such that * shouldn't harm, but it is unnecessary. */ - child_err_report_pipe[0] = dup_noninherited (child_err_report_pipe[0], _O_RDONLY); + child_err_report_pipe[0] = reopen_noninherited (child_err_report_pipe[0], _O_RDONLY); if (flags & G_SPAWN_FILE_AND_ARGV_ZERO) { @@ -625,7 +625,7 @@ do_spawn_with_fds (gint *exit_status, * process won't read but won't get any EOF either, as it has the * write end open itself. */ - helper_sync_pipe[1] = dup_noninherited (helper_sync_pipe[1], _O_WRONLY); + helper_sync_pipe[1] = reopen_noninherited (helper_sync_pipe[1], _O_WRONLY); if (stdin_fd != -1) {