[bug report] ASoC: dapm: Implement and instantiate DAI widgets

[bug report] ASoC: dapm: Implement and instantiate DAI widgets

[Dan Carpenter]

Hello Mark Brown,

The patch 888df395ebc5: "ASoC: dapm: Implement and instantiate DAI
widgets" from Feb 16, 2012, leads to the following static checker
warning:

	sound/soc/soc-dapm.c:3944 snd_soc_dapm_new_dai_widgets()
	error: 'w' dereferencing possible ERR_PTR()

sound/soc/soc-dapm.c
  3929          if (dai->driver->playback.stream_name) {
  3930                  template.id = snd_soc_dapm_dai_in;
  3931                  template.name = dai->driver->playback.stream_name;
  3932                  template.sname = dai->driver->playback.stream_name;
  3933  
  3934                  dev_dbg(dai->dev, "ASoC: adding %s widget\n",
  3935                          template.name);
  3936  
  3937                  w = snd_soc_dapm_new_control_unlocked(dapm, &template);

This can either return NULL or ERR_PTR(-EPROBE_DEFER).

  3938                  if (!w) {
  3939                          dev_err(dapm->dev, "ASoC: Failed to create %s widget\n",
  3940                                  dai->driver->playback.stream_name);
  3941                          return -ENOMEM;
  3942                  }
  3943  
  3944                  w->priv = dai;
                        ^^^^^^^^^^^^^

  3945                  dai->playback_widget = w;
  3946          }

regards,
dan carpenter

Re: [bug report] ASoC: dapm: Implement and instantiate DAI widgets

[Dan Carpenter]

There is another warning some lines later as well:

    sound/soc/soc-dapm.c:3963 snd_soc_dapm_new_dai_widgets()
    error: 'w' dereferencing possible ERR_PTR()

regards,
dan carpenter

Re: [bug report] ASoC: dapm: Implement and instantiate DAI widgets

[Vinod Koul]

On Thu, Jan 19, 2017 at 10:38:26AM +0300, Dan Carpenter wrote:
> Hello Mark Brown,

Dan you need to update Mark's email in your address book, its been a while
since he moved away from wolfson..

> 
> The patch 888df395ebc5: "ASoC: dapm: Implement and instantiate DAI
> widgets" from Feb 16, 2012, leads to the following static checker
> warning:
> 
> 	sound/soc/soc-dapm.c:3944 snd_soc_dapm_new_dai_widgets()
> 	error: 'w' dereferencing possible ERR_PTR()
> 
> sound/soc/soc-dapm.c
>   3929          if (dai->driver->playback.stream_name) {
>   3930                  template.id = snd_soc_dapm_dai_in;
>   3931                  template.name = dai->driver->playback.stream_name;
>   3932                  template.sname = dai->driver->playback.stream_name;
>   3933  
>   3934                  dev_dbg(dai->dev, "ASoC: adding %s widget\n",
>   3935                          template.name);
>   3936  
>   3937                  w = snd_soc_dapm_new_control_unlocked(dapm, &template);
> 
> This can either return NULL or ERR_PTR(-EPROBE_DEFER).
> 
>   3938                  if (!w) {
>   3939                          dev_err(dapm->dev, "ASoC: Failed to create %s widget\n",
>   3940                                  dai->driver->playback.stream_name);
>   3941                          return -ENOMEM;
>   3942                  }
>   3943  
>   3944                  w->priv = dai;
>                         ^^^^^^^^^^^^^
> 
>   3945                  dai->playback_widget = w;
>   3946          }
> 
> regards,
> dan carpenter
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
~Vinod

Re: [bug report] ASoC: dapm: Implement and instantiate DAI widgets

[Dan Carpenter]

On Thu, Jan 19, 2017 at 02:50:46PM +0530, Vinod Koul wrote:
> On Thu, Jan 19, 2017 at 10:38:26AM +0300, Dan Carpenter wrote:
> > Hello Mark Brown,
> 
> Dan you need to update Mark's email in your address book, its been a while
> since he moved away from wolfson..
> 

Oh crap!  This warning is from 2012...  I really need to update my
scripts so I stop sending really ancient warnings.

regards,
dan carpenter

Re: [bug report] ASoC: dapm: Implement and instantiate DAI widgets

[Lars-Peter Clausen]

On 01/19/2017 11:30 AM, Dan Carpenter wrote:
> On Thu, Jan 19, 2017 at 02:50:46PM +0530, Vinod Koul wrote:
>> On Thu, Jan 19, 2017 at 10:38:26AM +0300, Dan Carpenter wrote:
>>> Hello Mark Brown,
>>
>> Dan you need to update Mark's email in your address book, its been a while
>> since he moved away from wolfson..
>>
> 
> Oh crap!  This warning is from 2012...  I really need to update my
> scripts so I stop sending really ancient warnings.

The warning is correct. It's just the wrong commit. The issue was introduced
with commit 37e1df8c95e2 ("ASoC: dapm: handle probe deferrals"). The commit
introduces ERR_PTR() as a possible return value for the function, but does
not update all callers.

- Lars

Re: [bug report] ASoC: dapm: Implement and instantiate DAI widgets

[Linus Walleij]

On Thu, Jan 19, 2017 at 1:25 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 01/19/2017 11:30 AM, Dan Carpenter wrote:
>> On Thu, Jan 19, 2017 at 02:50:46PM +0530, Vinod Koul wrote:
>>> On Thu, Jan 19, 2017 at 10:38:26AM +0300, Dan Carpenter wrote:
>>>> Hello Mark Brown,
>>>
>>> Dan you need to update Mark's email in your address book, its been a while
>>> since he moved away from wolfson..
>>>
>>
>> Oh crap!  This warning is from 2012...  I really need to update my
>> scripts so I stop sending really ancient warnings.
>
> The warning is correct. It's just the wrong commit. The issue was introduced
> with commit 37e1df8c95e2 ("ASoC: dapm: handle probe deferrals"). The commit
> introduces ERR_PTR() as a possible return value for the function, but does
> not update all callers.

Yeah so the DAPM code was violating the regulator and clock
APIs by ignoring potential error pointers (just doing NULL checks), so I
made it aware of this, but oops it created a downstream problem that everyone
using DAPM gets the cascaded error pointer.

I just have to bit the bullet and make a patch updating all DAPM
consumers, it's just work, I'll keep you in CC.

Yours,
Linus Walleij

Re: [bug report] ASoC: dapm: Implement and instantiate DAI widgets

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 933 bytes --]

On Fri, Jan 20, 2017 at 10:52:31AM +0100, Linus Walleij wrote:
> On Thu, Jan 19, 2017 at 1:25 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:

> > The warning is correct. It's just the wrong commit. The issue was introduced
> > with commit 37e1df8c95e2 ("ASoC: dapm: handle probe deferrals"). The commit
> > introduces ERR_PTR() as a possible return value for the function, but does
> > not update all callers.

> Yeah so the DAPM code was violating the regulator and clock
> APIs by ignoring potential error pointers (just doing NULL checks), so I
> made it aware of this, but oops it created a downstream problem that everyone
> using DAPM gets the cascaded error pointer.

> I just have to bit the bullet and make a patch updating all DAPM
> consumers, it's just work, I'll keep you in CC.

Yeah, the error handling was already broken - all your patch did was
make it easier for a static checker to spot that there were problems.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]