From patchwork Thu Oct 18 15:28:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 10647503 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3A274112B for ; Thu, 18 Oct 2018 15:28:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2B71428C20 for ; Thu, 18 Oct 2018 15:28:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2009028D0A; Thu, 18 Oct 2018 15:28:43 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 803BC28C20 for ; Thu, 18 Oct 2018 15:28:42 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CB1976E087; Thu, 18 Oct 2018 15:28:40 +0000 (UTC) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2A51C6E022 for ; Thu, 18 Oct 2018 15:28:31 +0000 (UTC) Received: by mail-wr1-x443.google.com with SMTP id y16so34184003wrw.3 for ; Thu, 18 Oct 2018 08:28:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=vLZ9XFT7yyUTuFaOcpkgrZN3DwIalaOSZw4YJbSi6cc=; b=XPVG+DqA+dUyKRJLyMIMHuEt4FqNWRLw0TgBfT0Y/grlF51ovg4fZkKX2TBEzxtHh5 hZVlyvI03cuV8SAevES4VzfKuxRrFTOUHbn8CCXhhjBjvtPQu7AoW/Two9m1kScK1+qE 3IpDGrWy59f3I7NU2V/jKBiinRKAR0j/QbdR9aehZ0jDLdZfcsKe9Yu0FxmgPgIr0tBx WdEXisypl4ccwBT5IW0dzLMU335Kcz+tQ2lnzs1nvqwxUwuBGp+3Ooh5uxOn3TljDSMS CPRStYr5zJSCSXa9bEvVCPYOHQNB7Ss46HjIyJKNKbE3Ei0s2ilKFhAYJX8woIJeO/5G YJXw== X-Gm-Message-State: ABuFfohjMhJkMD3skZmnXVnGoP8tsDqT0AoucnMZCKtbWS24QFcUSMW1 9xuhmEI6GPmpDRFGrYK8mRUEiw== X-Google-Smtp-Source: ACcGV63vuLuMXFW3O4goQsodiOe1Reh4JMAV00PCniRZoaWpZjIDUs2PtEwpUB1JTVq1IqmZStURJw== X-Received: by 2002:adf:d181:: with SMTP id h1-v6mr7184274wri.138.1539876509575; Thu, 18 Oct 2018 08:28:29 -0700 (PDT) Received: from localhost.localdomain ([91.110.193.16]) by smtp.gmail.com with ESMTPSA id i6-v6sm19530387wrq.4.2018.10.18.08.28.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Oct 2018 08:28:28 -0700 (PDT) From: Tvrtko Ursulin X-Google-Original-From: Tvrtko Ursulin To: igt-dev@lists.freedesktop.org Date: Thu, 18 Oct 2018 16:28:05 +0100 Message-Id: <20181018152815.31816-8-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181018152815.31816-1-tvrtko.ursulin@linux.intel.com> References: <20181018152815.31816-1-tvrtko.ursulin@linux.intel.com> Subject: [Intel-gfx] [PATCH i-g-t 07/17] gem_wsim: Factor out common error handling X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Intel-gfx@lists.freedesktop.org MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Tvrtko Ursulin There is a repeated pattern with error handling which can be moved to a macro to for better readability in the command parsing loop. Signed-off-by: Tvrtko Ursulin --- benchmarks/gem_wsim.c | 244 +++++++++++++++--------------------------- 1 file changed, 88 insertions(+), 156 deletions(-) diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c index 2561817622f6..a6ee6c493424 100644 --- a/benchmarks/gem_wsim.c +++ b/benchmarks/gem_wsim.c @@ -290,6 +290,27 @@ parse_dependencies(unsigned int nr_steps, struct w_step *w, char *_desc) return 0; } +static void __attribute__((format(printf, 1, 2))) +wsim_err(const char *fmt, ...) +{ + va_list ap; + + if (!verbose) + return; + + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); +} + +#define check_arg(cond, fmt, ...) \ +{ \ + if (cond) { \ + wsim_err(fmt, __VA_ARGS__); \ + return NULL; \ + } \ +} + static struct workload * parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) { @@ -320,14 +341,9 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp <= 0) { - if (verbose) - fprintf(stderr, - "Invalid delay at step %u!\n", - nr_steps); - return NULL; - } - + check_arg(tmp <= 0, + "Invalid delay at step %u!\n", + nr_steps); step.type = DELAY; step.delay = tmp; goto add_step; @@ -336,14 +352,9 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp <= 0) { - if (verbose) - fprintf(stderr, - "Invalid period at step %u!\n", - nr_steps); - return NULL; - } - + check_arg(tmp <= 0, + "Invalid period at step %u!\n", + nr_steps); step.type = PERIOD; step.period = tmp; goto add_step; @@ -353,25 +364,17 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) while ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp <= 0 && nr == 0) { - if (verbose) - fprintf(stderr, - "Invalid context at step %u!\n", - nr_steps); - return NULL; - } - - if (nr == 0) { + check_arg(nr == 0 && tmp <= 0, + "Invalid context at step %u!\n", + nr_steps); + check_arg(nr > 1, + "Invalid priority format at step %u!\n", + nr_steps); + + if (nr == 0) step.context = tmp; - } else if (nr == 1) { + else step.priority = tmp; - } else { - if (verbose) - fprintf(stderr, - "Invalid priority format at step %u!\n", - nr_steps); - return NULL; - } nr++; } @@ -382,15 +385,10 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp >= 0 || - ((int)nr_steps + tmp) < 0) { - if (verbose) - fprintf(stderr, - "Invalid sync target at step %u!\n", - nr_steps); - return NULL; - } - + check_arg(tmp >= 0 || + ((int)nr_steps + tmp) < 0, + "Invalid sync target at step %u!\n", + nr_steps); step.type = SYNC; step.target = tmp; goto add_step; @@ -399,14 +397,9 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp < 0) { - if (verbose) - fprintf(stderr, - "Invalid throttle at step %u!\n", - nr_steps); - return NULL; - } - + check_arg(tmp < 0, + "Invalid throttle at step %u!\n", + nr_steps); step.type = THROTTLE; step.throttle = tmp; goto add_step; @@ -415,14 +408,9 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp < 0) { - if (verbose) - fprintf(stderr, - "Invalid qd throttle at step %u!\n", - nr_steps); - return NULL; - } - + check_arg(tmp < 0, + "Invalid qd throttle at step %u!\n", + nr_steps); step.type = QD_THROTTLE; step.throttle = tmp; goto add_step; @@ -431,14 +419,9 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp >= 0) { - if (verbose) - fprintf(stderr, - "Invalid sw fence signal at step %u!\n", - nr_steps); - return NULL; - } - + check_arg(tmp >= 0, + "Invalid sw fence signal at step %u!\n", + nr_steps); step.type = SW_FENCE_SIGNAL; step.target = tmp; goto add_step; @@ -451,31 +434,20 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) while ((field = strtok_r(fstart, ".", &fctx)) != NULL) { tmp = atoi(field); - if (tmp <= 0 && nr == 0) { - if (verbose) - fprintf(stderr, - "Invalid context at step %u!\n", - nr_steps); - return NULL; - } else if (tmp < 0 && nr == 1) { - if (verbose) - fprintf(stderr, - "Invalid preemption period at step %u!\n", - nr_steps); - return NULL; - } - - if (nr == 0) { + check_arg(nr == 0 && tmp <= 0, + "Invalid context at step %u!\n", + nr_steps); + check_arg(nr == 1 && tmp < 0, + "Invalid preemption period at step %u!\n", + nr_steps); + check_arg(nr > 1, + "Invalid preemption format at step %u!\n", + nr_steps); + + if (nr == 0) step.context = tmp; - } else if (nr == 1) { + else step.period = tmp; - } else { - if (verbose) - fprintf(stderr, - "Invalid preemption format at step %u!\n", - nr_steps); - return NULL; - } nr++; } @@ -485,13 +457,8 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) } tmp = atoi(field); - if (tmp < 0) { - if (verbose) - fprintf(stderr, - "Invalid ctx id at step %u!\n", - nr_steps); - return NULL; - } + check_arg(tmp < 0, "Invalid ctx id at step %u!\n", + nr_steps); step.context = tmp; valid++; @@ -512,13 +479,8 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) } } - if (old_valid == valid) { - if (verbose) - fprintf(stderr, - "Invalid engine id at step %u!\n", - nr_steps); - return NULL; - } + check_arg(old_valid == valid, + "Invalid engine id at step %u!\n", nr_steps); } if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { @@ -528,25 +490,19 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) fstart = NULL; tmpl = strtol(field, &sep, 10); - if (tmpl <= 0 || tmpl == LONG_MIN || tmpl == LONG_MAX) { - if (verbose) - fprintf(stderr, - "Invalid duration at step %u!\n", - nr_steps); - return NULL; - } + check_arg(tmpl <= 0 || tmpl == LONG_MIN || + tmpl == LONG_MAX, + "Invalid duration at step %u!\n", nr_steps); step.duration.min = tmpl; if (sep && *sep == '-') { tmpl = strtol(sep + 1, NULL, 10); - if (tmpl <= 0 || tmpl <= step.duration.min || - tmpl == LONG_MIN || tmpl == LONG_MAX) { - if (verbose) - fprintf(stderr, - "Invalid duration range at step %u!\n", - nr_steps); - return NULL; - } + check_arg(tmpl <= 0 || + tmpl <= step.duration.min || + tmpl == LONG_MIN || + tmpl == LONG_MAX, + "Invalid duration range at step %u!\n", + nr_steps); step.duration.max = tmpl; } else { step.duration.max = step.duration.min; @@ -559,13 +515,8 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) fstart = NULL; tmp = parse_dependencies(nr_steps, &step, field); - if (tmp < 0) { - if (verbose) - fprintf(stderr, - "Invalid dependency at step %u!\n", - nr_steps); - return NULL; - } + check_arg(tmp < 0, + "Invalid dependency at step %u!\n", nr_steps); valid++; } @@ -573,25 +524,16 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w) if ((field = strtok_r(fstart, ".", &fctx)) != NULL) { fstart = NULL; - if (strlen(field) != 1 || - (field[0] != '0' && field[0] != '1')) { - if (verbose) - fprintf(stderr, - "Invalid wait boolean at step %u!\n", - nr_steps); - return NULL; - } + check_arg(strlen(field) != 1 || + (field[0] != '0' && field[0] != '1'), + "Invalid wait boolean at step %u!\n", + nr_steps); step.sync = field[0] - '0'; valid++; } - if (valid != 5) { - if (verbose) - fprintf(stderr, "Invalid record at step %u!\n", - nr_steps); - return NULL; - } + check_arg(valid != 5, "Invalid record at step %u!\n", nr_steps); step.type = BATCH; @@ -636,15 +578,10 @@ add_step: for (i = 0; i < nr_steps; i++) { for (j = 0; j < steps[i].fence_deps.nr; j++) { tmp = steps[i].idx + steps[i].fence_deps.list[j]; - if (tmp < 0 || tmp >= i || - (steps[tmp].type != BATCH && - steps[tmp].type != SW_FENCE)) { - if (verbose) - fprintf(stderr, - "Invalid dependency target %u!\n", - i); - return NULL; - } + check_arg(tmp < 0 || tmp >= i || + (steps[tmp].type != BATCH && + steps[tmp].type != SW_FENCE), + "Invalid dependency target %u!\n", i); steps[tmp].emit_fence = -1; } } @@ -653,14 +590,9 @@ add_step: for (i = 0; i < nr_steps; i++) { if (steps[i].type == SW_FENCE_SIGNAL) { tmp = steps[i].idx + steps[i].target; - if (tmp < 0 || tmp >= i || - steps[tmp].type != SW_FENCE) { - if (verbose) - fprintf(stderr, - "Invalid sw fence target %u!\n", - i); - return NULL; - } + check_arg(tmp < 0 || tmp >= i || + steps[tmp].type != SW_FENCE, + "Invalid sw fence target %u!\n", i); } }