From patchwork Thu Sep 21 12:52:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Petri Latvala X-Patchwork-Id: 9963887 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A024C600C5 for ; Thu, 21 Sep 2017 12:55:19 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 82388294CB for ; Thu, 21 Sep 2017 12:55:19 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7741D294CD; Thu, 21 Sep 2017 12:55:19 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C58F6294CB for ; Thu, 21 Sep 2017 12:55:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 846196E21A; Thu, 21 Sep 2017 12:55:18 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from johanna4.inet.fi (mta-out1.inet.fi [62.71.2.233]) by gabe.freedesktop.org (Postfix) with ESMTP id E0F0E6E21A for ; Thu, 21 Sep 2017 12:55:16 +0000 (UTC) RazorGate-KAS: Status: not_detected RazorGate-KAS: Rate: 0 RazorGate-KAS: Envelope from: RazorGate-KAS: Version: 5.5.3 RazorGate-KAS: LuaCore: 215 2015-05-29_17-31-22 60ae4a1b4d01d14f868b20a55aced8d7df7b2e28 RazorGate-KAS: Lua profiles 78662 [Jun 02 2015] RazorGate-KAS: Method: none Received: from hufflepuff.adrinael.net (84.248.197.237) by johanna4.inet.fi (9.0.002.03-2-gbe5d057) id 59BF88C7005651DF; Thu, 21 Sep 2017 15:55:16 +0300 Received: from adrinael by hufflepuff.adrinael.net with local (Exim 4.84_2) (envelope-from ) id 1dv10e-0000vf-Cu; Thu, 21 Sep 2017 15:55:16 +0300 From: Petri Latvala To: intel-gfx@lists.freedesktop.org Date: Thu, 21 Sep 2017 15:52:28 +0300 Message-Id: <1505998348-3444-2-git-send-email-petri.latvala@intel.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1505998348-3444-1-git-send-email-petri.latvala@intel.com> References: <1505998348-3444-1-git-send-email-petri.latvala@intel.com> Subject: [Intel-gfx] [PATCH i-g-t 2/2] igt_core: Rework igt_system() X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Petri Latvala Reviewed-by: Abdiel Janulgue --- 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; }