Re: [PATCH 4/5] ASoC: soc-pcm.c: use temporary variable at soc_get_playback_capture()

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

On 5/30/2023 2:50 AM, Kuninori Morimoto wrote:
> soc_get_playback_capture() (A) returns number of substreams for
> playback/capture (B).
> 
> (A)	static int soc_get_playback_capture(...,
> (B)					int *playback, int *capture)
> 	{
> 		...
> 		for_each_xxx(...) {
> 			if (xxx)
> 				return -EINVAL;
> =>			*playback = 1;
> 			...
> =>			*capture = 1;
> 			...
> 		}
> 		...
> 	}
> 
> But, it is directly updating playback/capture which is the result of this
> function even though it might be error. It should be updated in case of
> succeed only. This patch updates it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   sound/soc/soc-pcm.c | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 47da3be0ff46..b3d569e7ba61 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -2733,6 +2733,8 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
>   {
>   	struct snd_soc_dai_link *dai_link = rtd->dai_link;
>   	struct snd_soc_dai *cpu_dai;
> +	int has_playback = 0;
> +	int has_capture  = 0;
>   	int i;
>   
>   	if (dai_link->dynamic && dai_link->num_cpus > 1) {
> @@ -2748,11 +2750,11 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
>   
>   			for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
>   				if (snd_soc_dai_stream_valid(cpu_dai, stream)) {
> -					*playback = 1;
> +					has_playback = 1;
>   					break;
>   				}
>   			}
> -			if (!*playback) {
> +			if (!has_playback) {
>   				dev_err(rtd->card->dev,
>   					"No CPU DAIs support playback for stream %s\n",
>   					dai_link->stream_name);
> @@ -2764,12 +2766,12 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
>   
>   			for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
>   				if (snd_soc_dai_stream_valid(cpu_dai, stream)) {
> -					*capture = 1;
> +					has_capture = 1;
>   					break;
>   				}
>   			}
>   
> -			if (!*capture) {
> +			if (!has_capture) {
>   				dev_err(rtd->card->dev,
>   					"No CPU DAIs support capture for stream %s\n",
>   					dai_link->stream_name);
> @@ -2798,30 +2800,33 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd,
>   
>   			if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
>   			    snd_soc_dai_stream_valid(cpu_dai,   cpu_playback))
> -				*playback = 1;
> +				has_playback = 1;
>   			if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) &&
>   			    snd_soc_dai_stream_valid(cpu_dai,   cpu_capture))
> -				*capture = 1;
> +				has_capture = 1;
>   		}
>   	}
>   
>   	if (dai_link->playback_only) {
> -		*playback = 1;
> -		*capture = 0;
> +		has_playback = 1;
> +		has_capture = 0;
>   	}
>   
>   	if (dai_link->capture_only) {
> -		*playback = 0;
> -		*capture = 1;
> +		has_playback = 0;
> +		has_capture = 1;
>   	}
>   
> -	if (!*playback && !*capture) {
> +	if (!has_playback && !has_capture) {
>   		dev_err(rtd->dev, "substream %s has no playback, no capture\n",
>   			dai_link->stream_name);
>   
>   		return -EINVAL;
>   	}
>   
> +	*playback = has_playback;
> +	*capture  = has_capture;
> +
>   	return 0;
>   }
>   

Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>