[PATCH i-g-t 1/2] lib/igt_drm_fdinfo: Use longest match first

[Lucas De Marchi]

On Thu, Feb 01, 2024 at 01:43:21PM +0100, Kamil Konieczny wrote:
>Hi Lucas,
>On 2024-01-31 at 16:46:24 -0800, Lucas De Marchi wrote:
>> Instead of checking twice the match, just use longest match first.
>
>Nice catch, LGTM.
>
>Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  lib/igt_drm_fdinfo.c | 19 +++++++++----------
>>  1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
>> index 222ccbfb1..5c0ccf624 100644
>> --- a/lib/igt_drm_fdinfo.c
>> +++ b/lib/igt_drm_fdinfo.c
>> @@ -239,8 +239,15 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
>>  					 strlen("drm-client-id")))) {
>>  			info->id = atol(v);
>>  			good++;
>> -		} else if (!strncmp(l, "drm-engine-", 11) &&
>> -			   strncmp(l, "drm-engine-capacity-", 20)) {
>> +		} else if (!strncmp(l, "drm-engine-capacity-", 20)) {
>
>One more thing to eventually fix is removing "20" which is exact
>len of string, with something like
>
>bool str_begins(char *src, char *pattern)
>
>One source of errors is counting letters in strings or changing
>string itself without changing corresponding len. This can be
>done in separate patch.

I had a patch for something similar. I usually use a strstartswith()
that's commonly used in other projects and also matches the python name.

However I decided to drop that for now. I may have a bigger refactor in
this function soon reworking parse_engine() and other functions: Worse
than manually counting the chars for the strncmp() is to call strlen()
in the same string just in the line below that to feed to parse_engine()
:-/. As I said, still undecided what to change here and if it's worth
it.


Lucas De Marchi

>
>Regards,
>Kamil
>
>> +			idx = parse_engine(l, info,
>> +					   strlen("drm-engine-capacity-"),
>> +					   name_map, map_entries, &val);
>> +			if (idx >= 0) {
>> +				info->capacity[idx] = val;
>> +				num_capacity++;
>> +			}
>> +		} else if (!strncmp(l, "drm-engine-", 11)) {
>>  			idx = parse_engine(l, info, strlen("drm-engine-"),
>>  					   name_map, map_entries, &val);
>>  			if (idx >= 0) {
>> @@ -251,14 +258,6 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
>>  				if (idx > info->last_engine_index)
>>  					info->last_engine_index = idx;
>>  			}
>> -		} else if (!strncmp(l, "drm-engine-capacity-", 20)) {
>> -			idx = parse_engine(l, info,
>> -					   strlen("drm-engine-capacity-"),
>> -					   name_map, map_entries, &val);
>> -			if (idx >= 0) {
>> -				info->capacity[idx] = val;
>> -				num_capacity++;
>> -			}
>>  		} else if (!strncmp(l, "drm-total-", 10)) {
>>  			idx = parse_region(l, info, strlen("drm-total-"),
>>  					   region_map, region_entries, &val);
>> --
>> 2.43.0
>>