Re: [igt-dev] [PATCH i-g-t v2 6/9] tests/kms_chamelium: add name parameter to audio_state_start

[Martin Peres <martin.peres@linux.intel.com>]

On 24/05/2019 18:03, Simon Ser wrote:
> This identifies the audio test name. This is required for adding multiple
> audio tests.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  tests/kms_chamelium.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 063840721f46..14262831c3ff 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -824,6 +824,7 @@ struct audio_state {
>  		int rate;
>  	} playback, capture;
>  
> +	const char *name;
>  	struct audio_signal *signal;
>  	int channel_mapping[8];
>  
> @@ -871,7 +872,7 @@ static void *run_audio_thread(void *data)
>  	return NULL;
>  }
>  
> -static void audio_state_start(struct audio_state *state)
> +static void audio_state_start(struct audio_state *state, const char *name)
>  {
>  	int ret;
>  	bool ok;
> @@ -879,12 +880,13 @@ static void audio_state_start(struct audio_state *state)
>  	enum chamelium_stream_realtime_mode stream_mode;
>  	char dump_suffix[64];
>  
> +	state->name = name;

This is a little dangerous since the name could have been allocated on
the stack of the caller, which could lead to a use after free later on.

With this replaced with strdup and a free(state->name) in the fini
function, the patch is:

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

>  	state->recv_pages = 0;
>  	state->msec = 0;
>  
> -	igt_debug("Starting test with playback format %s, sampling rate %d Hz "
> -		  "and %d channels\n",
> -		  snd_pcm_format_name(state->playback.format),
> +	igt_debug("Starting %s test with playback format %s, "
> +		  "sampling rate %d Hz and %d channels\n",
> +		  name, snd_pcm_format_name(state->playback.format),
>  		  state->playback.rate, state->playback.channels);
>  
>  	chamelium_start_capturing_audio(state->chamelium, state->port, false);
> @@ -930,8 +932,8 @@ static void audio_state_start(struct audio_state *state)
>  
>  	if (igt_frame_dump_is_enabled()) {
>  		snprintf(dump_suffix, sizeof(dump_suffix),
> -			 "capture-%s-%dch-%dHz",
> -			 snd_pcm_format_name(state->playback.format),
> +			 "capture-%s-%s-%dch-%dHz",
> +			 name, snd_pcm_format_name(state->playback.format),
>  			 state->playback.channels, state->playback.rate);
>  
>  		state->dump_fd = audio_create_wav_file_s32_le(dump_suffix,
> @@ -1001,9 +1003,9 @@ static void audio_state_stop(struct audio_state *state, bool success)
>  		state->dump_path = NULL;
>  	}
>  
> -	igt_debug("Audio test result for format %s, sampling rate %d Hz and "
> -		  "%d channels: %s\n",
> -		  snd_pcm_format_name(state->playback.format),
> +	igt_debug("Audio %s test result for format %s, sampling rate %d Hz "
> +		  "and %d channels: %s\n",
> +		  state->name, snd_pcm_format_name(state->playback.format),
>  		  state->playback.rate, state->playback.channels,
>  		  success ? "ALL GREEN" : "FAILED");
>  }
> @@ -1062,7 +1064,7 @@ static bool test_audio_frequencies(struct audio_state *state)
>  				      audio_output_frequencies_callback, state,
>  				      PLAYBACK_SAMPLES);
>  
> -	audio_state_start(state);
> +	audio_state_start(state, "frequencies");
>  
>  	igt_assert(state->capture.rate == state->playback.rate);
>  
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev