Re: [bug report] ALSA: hda: Interrupt servicing and BDL setup for compress streams

[Takashi Iwai <tiwai@suse.de>]

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