Re: [PATCH i-g-t 03/12] lib/igt_drm_fdinfo: Fix wrong name len assert

[Tvrtko Ursulin <tursulin@ursulin.net>]

On 19/04/2024 08:48, Tvrtko Ursulin wrote:
> 
> On 05/04/2024 07:00, Lucas De Marchi wrote:
>> When parsing engine name, at this point we have something like:
>>
>>     "drm-engine-render:    232409619162 ns"
>>                 ^      ^
>>                 |      |
>>               name     p
>>
>> We are copying the engine name to the final destination, but
>> strlen(name) will actually contain the entire string. Since we already
>> know the name length, calculated previously, just use it for copying the
>> string.
>>
>> Since each name is 256-bytes long, the assert doesn't trigger easily,
>> but it's better to be correct. And more efficient.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>   lib/igt_drm_fdinfo.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
>> index 7a9ae94f0..9b1776775 100644
>> --- a/lib/igt_drm_fdinfo.c
>> +++ b/lib/igt_drm_fdinfo.c
>> @@ -96,8 +96,9 @@ static int parse_engine(const char *name, struct 
>> drm_client_fdinfo *info,
>>           if (found < 0) {
>>               assert((info->num_engines + 1) < ARRAY_SIZE(info->names));
>> -            assert((strlen(name) + 1) < sizeof(info->names[0]));
>> -            strncpy(info->names[info->num_engines], name, name_len);
>> +            assert(name_len  < sizeof(info->names[0]));
>> +            memcpy(info->names[info->num_engines], name, name_len);
>> +            info->names[info->num_engines][name_len] = '\0';
>>               found = info->num_engines;
>>           }
>>       }
> 
> Ah a good fix.
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Oooh the muscle memory.. I knew this would happen sooner or later! :))

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

> 
> Also, ideally fixes such as this and next few would be before 
> refactoring in the series, but okay, not asking you to re-order.
> 
> Regards,
> 
> Tvrtko
> 
> P.S. One extra space in the assert fwiw.