All of lore.kernel.org
 help / color / mirror / Atom feed
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: alsa-devel@alsa-project.org,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	linux-kernel@vger.kernel.org, Liam Girdwood <lgirdwood@gmail.com>,
	Takashi Iwai <tiwai@suse.com>, Mark Brown <broonie@kernel.org>,
	patches@opensource.cirrus.com,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>
Subject: Re: [PATCH] ASoC: wm97xx: fix uninitialized regmap pointer problem
Date: Fri, 2 Nov 2018 14:38:17 +0000	[thread overview]
Message-ID: <20181102143817.GP16508@imbe.wolfsonmicro.main> (raw)
In-Reply-To: <20181102112341.753642-1-arnd@arndb.de>

On Fri, Nov 02, 2018 at 12:23:08PM +0100, Arnd Bergmann wrote:
> gcc notices that without either the ac97 bus or the pdata, we never
> initialize the regmap pointer, which leads to an uninitialized variable
> access:
> 
> sound/soc/codecs/wm9712.c: In function 'wm9712_soc_probe':
> sound/soc/codecs/wm9712.c:666:2: error: 'regmap' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> Since that configuration is invalid, it's better to return an error
> here. I tried to avoid adding complexity to the conditions, and turned
> the #ifdef into a regular if(IS_ENABLED()) check for readability.
> This in turn requires moving some header file declarations out of
> an #ifdef.
> 
> The same code is used in three drivers, all of which I'm changing
> the same way.
> 
> Fixes: 2ed1a8e0ce8d ("ASoC: wm9712: add ac97 new bus support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  static void wm9713_soc_remove(struct snd_soc_component *component)
>  {
> -#ifdef CONFIG_SND_SOC_AC97_BUS
>  	struct wm9713_priv *wm9713 = snd_soc_component_get_drvdata(component);
>  
> -	if (!wm9713->mfd_pdata) {
> +	if (IS_ENABLED(CONFIG_SND_SOC_AC97_BUS)) {

Should this one not also have an && !wm9713->mfd_pdata?

Thanks,
Charles

>  		snd_soc_component_exit_regmap(component);
>  		snd_soc_free_ac97_component(wm9713->ac97);
>  	}
> -#endif
>  }
>  
>  static const struct snd_soc_component_driver soc_component_dev_wm9713 = {
> -- 
> 2.18.0

WARNING: multiple messages have this Message-ID (diff)
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	"Robert Jarzmik" <robert.jarzmik@free.fr>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	<alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
	<patches@opensource.cirrus.com>
Subject: Re: [PATCH] ASoC: wm97xx: fix uninitialized regmap pointer problem
Date: Fri, 2 Nov 2018 14:38:17 +0000	[thread overview]
Message-ID: <20181102143817.GP16508@imbe.wolfsonmicro.main> (raw)
In-Reply-To: <20181102112341.753642-1-arnd@arndb.de>

On Fri, Nov 02, 2018 at 12:23:08PM +0100, Arnd Bergmann wrote:
> gcc notices that without either the ac97 bus or the pdata, we never
> initialize the regmap pointer, which leads to an uninitialized variable
> access:
> 
> sound/soc/codecs/wm9712.c: In function 'wm9712_soc_probe':
> sound/soc/codecs/wm9712.c:666:2: error: 'regmap' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> Since that configuration is invalid, it's better to return an error
> here. I tried to avoid adding complexity to the conditions, and turned
> the #ifdef into a regular if(IS_ENABLED()) check for readability.
> This in turn requires moving some header file declarations out of
> an #ifdef.
> 
> The same code is used in three drivers, all of which I'm changing
> the same way.
> 
> Fixes: 2ed1a8e0ce8d ("ASoC: wm9712: add ac97 new bus support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  static void wm9713_soc_remove(struct snd_soc_component *component)
>  {
> -#ifdef CONFIG_SND_SOC_AC97_BUS
>  	struct wm9713_priv *wm9713 = snd_soc_component_get_drvdata(component);
>  
> -	if (!wm9713->mfd_pdata) {
> +	if (IS_ENABLED(CONFIG_SND_SOC_AC97_BUS)) {

Should this one not also have an && !wm9713->mfd_pdata?

Thanks,
Charles

>  		snd_soc_component_exit_regmap(component);
>  		snd_soc_free_ac97_component(wm9713->ac97);
>  	}
> -#endif
>  }
>  
>  static const struct snd_soc_component_driver soc_component_dev_wm9713 = {
> -- 
> 2.18.0

  reply	other threads:[~2018-11-02 14:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-02 11:23 [PATCH] ASoC: wm97xx: fix uninitialized regmap pointer problem Arnd Bergmann
2018-11-02 11:23 ` Arnd Bergmann
2018-11-02 14:38 ` Charles Keepax [this message]
2018-11-02 14:38   ` Charles Keepax
2018-11-02 15:15   ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181102143817.GP16508@imbe.wolfsonmicro.main \
    --to=ckeepax@opensource.cirrus.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=fabio.estevam@nxp.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=robert.jarzmik@free.fr \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.