Instead of redirecting output to pipes and forking, redirect after forking to avoid having to carefully unredirect before logging anything.
igt@tools_test@sysfs_l3_parity had a racy condition where it prints the output of intel_l3_parity prepended by [cmd], but that ended up being printed again prepended by [cmd] because output was redirected, causing outputs to appear multiple times. This patch fixes that. CC: Abdiel Janulgue <[email protected]> Signed-off-by: Petri Latvala <[email protected]> --- lib/igt_core.c | 115 ++++++++++++++++++++------------------------------------- 1 file changed, 40 insertions(+), 75 deletions(-) diff --git a/lib/igt_core.c b/lib/igt_core.c index 9f4ee68b..47b4682d 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -2237,58 +2237,23 @@ FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir, return fp; } -struct output_pipe { - int output_fd; - int save_fd; - int read_fd; - int write_fd; - bool redirected; - enum igt_log_level log_level; -}; - -static bool redirect_output(struct output_pipe *p, int output_fd, - enum igt_log_level level) +static void log_output(int *fd, enum igt_log_level level) { - int fds[2]; - - if (pipe(fds) == -1) - goto err; - - /* save output */ - if ((p->save_fd = dup(output_fd)) == -1) - goto err; - - /* Redirect output to our buffer */ - if (dup2(fds[1], output_fd) == -1) - goto err; - - p->output_fd = output_fd; - p->read_fd = fds[0]; - p->write_fd = fds[1]; - p->redirected = true; - p->log_level = level; - - return true; -err: - close(fds[0]); - close(fds[1]); - close(p->save_fd); + ssize_t len; + char buf[PIPE_BUF]; - return false; -} + if (*fd < 0) + return; -static void unredirect_output(struct output_pipe *p) -{ - if (!p->redirected) + memset(buf, 0, sizeof(buf)); + len = read(*fd, buf, sizeof(buf)); + if (len <= 0) { + close(*fd); + *fd = -1; return; + } - /* read_fd is closed separately. We still need to read its - * buffered contents after un-redirecting the stream. - */ - close(p->write_fd); - dup2(p->save_fd, p->output_fd); - close(p->save_fd); - p->redirected = false; + igt_log(IGT_LOG_DOMAIN, level, "[cmd] %s", buf); } /** @@ -2304,48 +2269,48 @@ static void unredirect_output(struct output_pipe *p) */ int igt_system(const char *command) { -#define OUT 0 -#define ERR 1 - struct output_pipe op[2]; - int i, status; + int outpipe[2] = { -1, -1 }; + int errpipe[2] = { -1, -1 }; + int status; struct igt_helper_process process = {}; - char buf[PIPE_BUF]; - if (!redirect_output(&op[OUT], STDOUT_FILENO, IGT_LOG_INFO)) + if (pipe(outpipe) < 0) goto err; - if (!redirect_output(&op[ERR], STDERR_FILENO, IGT_LOG_WARN)) + if (pipe(errpipe) < 0) goto err; igt_fork_helper(&process) { - igt_assert(execl("/bin/sh", "sh", "-c", command, - (char *) NULL) != -1); + close(outpipe[0]); + close(errpipe[0]); + + if (dup2(outpipe[1], STDOUT_FILENO) < 0) + goto child_err; + if (dup2(errpipe[1], STDERR_FILENO) < 0) + goto child_err; + + execl("/bin/sh", "sh", "-c", command, + (char *) NULL); + + child_err: + exit(EXIT_FAILURE); } - for (i = 0; i < ARRAY_SIZE(op); i++) { - struct output_pipe *current = &op[i]; + close(outpipe[1]); + close(errpipe[1]); - /* Unredirect so igt_log() works */ - unredirect_output(current); - memset(buf, 0, sizeof(buf)); - while (read(current->read_fd, buf, sizeof(buf)) > 0) { - igt_log(IGT_LOG_DOMAIN, current->log_level, - "[cmd] %s", buf); - memset(buf, 0, sizeof(buf)); - } - close(current->read_fd); + while (outpipe[0] >= 0 || errpipe[0] >= 0) { + log_output(&outpipe[0], IGT_LOG_INFO); + log_output(&errpipe[0], IGT_LOG_WARN); } + status = igt_wait_helper(&process); return WEXITSTATUS(status); err: - /* Failed to redirect one or both streams. Roll back changes. */ - for (i = 0; i < ARRAY_SIZE(op); i++) { - if (!op[i].redirected) - continue; - close(op[i].read_fd); - unredirect_output(&op[i]); - } - + close(outpipe[0]); + close(outpipe[1]); + close(errpipe[0]); + close(errpipe[1]); return -1; } -- 2.14.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
