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

[Tvrtko Ursulin <tursulin@ursulin.net>]

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>

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.