[PATCH i-g-t] lib/igt_core: Free log_buffer entries on exit

[Kamil Konieczny]

Hi Lucas,
On 2024-11-06 at 23:02:08 -0800, Lucas De Marchi wrote:
> Make sure the buffer entries are released, otherwise valgrind output
> becomes each used entry as still reachable on exit (up to 256).
> 
> Test:
> 	# valgrind --leak-check=full  ./build/tests/xe_live_ktest  --r xe_mocs
> 
> Before:
> 	==16082== LEAK SUMMARY:
> 	==16082==    definitely lost: 0 bytes in 0 blocks
> 	==16082==    indirectly lost: 0 bytes in 0 blocks
> 	==16082==      possibly lost: 0 bytes in 0 blocks
> 	==16082==    still reachable: 33,473 bytes in 59 blocks
> 
> After:
> 	==15992== LEAK SUMMARY:
> 	==15992==    definitely lost: 0 bytes in 0 blocks
> 	==15992==    indirectly lost: 0 bytes in 0 blocks
> 	==15992==      possibly lost: 0 bytes in 0 blocks
> 	==15992==    still reachable: 31,083 bytes in 42 blocks

Thank you for a patch, there are still some blocks left,
maybe because that buffer is a ring? e.g. it could have
end < start

> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  lib/igt_core.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 407f7b551..879d1ecd8 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -469,11 +469,21 @@ static void _igt_log_buffer_append(char *line)
>  	pthread_mutex_unlock(&log_buffer_mutex);
>  }
>  
> +static void _igt_log_buffer_reset_locked(void)
> +{
> +	for (; log_buffer.start != log_buffer.end; log_buffer.start++) {
> +		free(log_buffer.entries[log_buffer.start]);
> +		log_buffer.entries[log_buffer.start] = NULL;
> +	}

Here you should use the same loop as in _dump, so

	i = log_buffer.start;
	do {
		// do ops here
		i++;
        } while (i != log_buffer.start && i != log_buffer.end);

Or even simpler, init with NULLs and then just free all?
For example, if start=2, end=1, there is still a valid pointer
at end==1

Or do one more ops for case 'end < start'

Regards,
Kamil

> +
> +	log_buffer.start = log_buffer.end = 0;
> +}
> +
>  static void _igt_log_buffer_reset(void)
>  {
>  	pthread_mutex_lock(&log_buffer_mutex);
>  
> -	log_buffer.start = log_buffer.end = 0;
> +	_igt_log_buffer_reset_locked();
>  
>  	pthread_mutex_unlock(&log_buffer_mutex);
>  }
> @@ -624,8 +634,7 @@ static void _igt_log_buffer_dump(void)
>  		i++;
>  	} while (i != log_buffer.start && i != log_buffer.end);
>  
> -	/* reset the buffer */
> -	log_buffer.start = log_buffer.end = 0;
> +	_igt_log_buffer_reset_locked();
>  
>  	_log_line_fprintf(stderr, "****  END  ****\n");
>  	pthread_mutex_unlock(&log_buffer_mutex);
> -- 
> 2.47.0
>