On Wed, 07 Dec 2022 15:35:12 +0100,
Cezary Rojewski wrote:
>
> On 2022-12-07 3:28 PM, Dan Carpenter wrote:
> > Hello Cezary Rojewski,
> >
> > The patch 3e9582267e3a: "ALSA: hda: Interrupt servicing and BDL setup
> > for compress streams" from Dec 2, 2022, leads to the following Smatch
> > static checker warning:
> >
> > sound/hda/hdac_stream.c:544 snd_hdac_stream_setup_periods()
> > error: uninitialized symbol 'dmab'.
> >
> > sound/hda/hdac_stream.c
> > 487 int snd_hdac_stream_setup_periods(struct hdac_stream *azx_dev)
> > 488 {
> > 489 struct hdac_bus *bus = azx_dev->bus;
> > 490 struct snd_pcm_substream *substream = azx_dev->substream;
> > 491 struct snd_compr_stream *cstream = azx_dev->cstream;
> > 492 struct snd_pcm_runtime *runtime = NULL;
> > 493 struct snd_dma_buffer *dmab;
> > 494 __le32 *bdl;
> > 495 int i, ofs, periods, period_bytes;
> > 496 int pos_adj, pos_align;
> > 497
> > 498 if (substream) {
> > 499 runtime = substream->runtime;
> > 500 dmab = snd_pcm_get_dma_buf(substream);
> > 501 } else if (cstream) {
> > 502 dmab = snd_pcm_get_dma_buf(cstream);
> > 503 }
> >
> > dmab is not initialized on else path.
>
> Hello,
>
> Thanks for the report. Perhaps I should just do: s/else if/else/ as
> the situation with ->substream AND ->cstream being uninitialized is
> invalid.
>
> Before support for compress stream was added in this part of the code,
> it was always assumed ->substream is valid.
>
> So, either s/else if/else or append:
> } else {
> return -EINVAL;
> }
>
> Takashi, what do you think?
The return -EINVAL would be OK. As this must not happen, it can be
even with WARN_ON().
thanks,
Takashi
