Re: [igt-dev] [PATCH i-g-t 13/21] gem_wsim: Compact int command parsing with a macro

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 10/05/2019 14:29, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-08 13:10:50)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Parsing an integer workload descriptor field is a common pattern which we
>> can extract to a helper macro and by doing so further improve the
>> readability of the main parsing loop.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   benchmarks/gem_wsim.c | 80 ++++++++++++++-----------------------------
>>   1 file changed, 25 insertions(+), 55 deletions(-)
>>
>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>> index 4dbfc3e922a9..c2e13d9939c2 100644
>> --- a/benchmarks/gem_wsim.c
>> +++ b/benchmarks/gem_wsim.c
>> @@ -370,6 +370,15 @@ static int parse_engine_map(struct w_step *step, const char *_str)
>>          return 0;
>>   }
>>   
>> +#define int_field(_STEP_, _FIELD_, _COND_, _ERR_) \
>> +       if ((field = strtok_r(fstart, ".", &fctx))) { \
>> +               tmp = atoi(field); \
>> +               check_arg(_COND_, _ERR_, nr_steps); \
>> +               step.type = _STEP_; \
>> +               step._FIELD_ = tmp; \
>> +               goto add_step; \
>> +       } \
> 
> More hidden control flow :-p

It's not the pretties I admit. It started as a quick project to test 
feasibility of userspace balancing and when it has shown itself somewhat 
useful I added more and more features to it. It's at the point where 
splitting int separate files and refactoring the data structures could 
be beneficial.

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

Thanks,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev