Re: [igt-dev] [PATCH i-g-t 10/21] gem_wsim: Extract str to engine lookup

Tvrtko Ursulin
2019-05-13T13:08:16Z

On 10/05/2019 14:20, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-08 13:10:47)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> benchmarks/gem_wsim.c | 34 +++++++++++++++++++++-------------
>> 1 file changed, 21 insertions(+), 13 deletions(-)
>>
>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>> index 5245692df6eb..f654decb24cc 100644
>> --- a/benchmarks/gem_wsim.c
>> +++ b/benchmarks/gem_wsim.c
>> @@ -318,6 +318,18 @@ wsim_err(const char *fmt, ...)
>> } \
>> }
>>
>> +static int str_to_engine(const char *str)
>> +{
>> + unsigned int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(ring_str_map); i++) {
>> + if (!strcasecmp(str, ring_str_map[i]))
>> + return i;
>> + }
>> +
>> + return -1;
>> +}
>> +
>> static struct workload *
>> parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w)
>> {
>> @@ -480,22 +492,18 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w)
>> }
>>
>> if ((field = strtok_r(fstart, ".", &fctx)) != NULL) {
>> - unsigned int old_valid = valid;
>> -
>> fstart = NULL;
>>
>> - for (i = 0; i < ARRAY_SIZE(ring_str_map); i++) {
>> - if (!strcasecmp(field, ring_str_map[i])) {
>> - step.engine = i;
>> - if (step.engine == BCS)
>> - bcs_used = true;
>> - valid++;
>> - break;
>> - }
>> - }
>> -
>> - check_arg(old_valid == valid,
>> + i = str_to_engine(field);
>> + check_arg(i < 0,
>> "Invalid engine id at step %u!\n", nr_steps);
>> + if (i >= 0)
>> + valid++;
>
> check_arg() returned already for all i < 0, no?

Yes, and it looks the very next patch removes the if. I'll pull it here.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks!

Regards,

Tvrtko