From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>

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 <tvrtko.ursu...@intel.com>
---
 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 609e64f3d9c8..ef97311a6879 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -289,6 +289,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)
 {
@@ -319,14 +340,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;
@@ -335,14 +351,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;
@@ -352,25 +363,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++;
                                }
@@ -381,15 +384,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;
@@ -398,14 +396,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;
@@ -414,14 +407,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;
@@ -430,14 +418,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;
@@ -450,31 +433,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++;
                                }
@@ -492,13 +464,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++;
@@ -519,13 +486,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) {
@@ -535,25 +497,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;
@@ -566,13 +522,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++;
                }
@@ -580,25 +531,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;
 
@@ -643,15 +585,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;
                }
        }
@@ -660,14 +597,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);
                }
        }
 
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to