On 1/23/23 06:01, Charles Keepax wrote:

On Mon, Jan 23, 2023 at 03:08:49AM +0000, Kuninori Morimoto wrote:

...

Hi ALSA ML

I'm not familiar with soc-compress.c, but I think dpcm_capture case don't need "else" ? I think it is same as soc-pcm.c :: soc_new_pcm(). I'm happy to post patch if someone acknowledge this question.

--- soc-compress.c -------- int snd_soc_new_compress(...) { ... if (rtd->dai_link->dynamic) { ... if (rtd->dai_link->dpcm_playback) be_pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream->private_data = rtd;

• 	else if (rtd->dai_link->dpcm_capture)
  

• 	if (rtd->dai_link->dpcm_capture)
  	be_pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream->private_data = rtd;
  ...
  

  } ... }

  --- soc-pcm.c --------------- int soc_new_pcm(...) { ... if (rtd->dai_link->no_pcm || rtd->dai_link->params) { if (playback) pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream->private_data = rtd;

=> if (capture) pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream->private_data = rtd; goto out; } ... }

I would certainly agree, pretty sure the else is redundant.

Agree as well on paper, but the two cases are different.

For the soc-pcm.c case, the configuration is for a Back-End (BE). We do have usages where both .dpcm_playback and .dpcm_capture are set.

In the soc-compress.c case, the configuration is for a Front-End (FE). I am not sure if there are any FE dailinks with both capture and playback support or if there are any hidden restrictions?

We'd need to check that this has no impact on platforms with compressed support - Srinivas and Vinod can you chime in?

At any rate this change would need a Fixes tag

Fixes: d3268a40d4b1 ("ASoC: soc-compress.c: fix NULL dereference")