From: Arnd Bergmann <arnd@arndb.de>
To: "Russell King - ARM Linux" <linux@arm.linux.org.uk>
Cc: Mark Brown <broonie@kernel.org>,
Brian Austin <brian.austin@cirrus.com>,
Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
Liam Girdwood <lgirdwood@gmail.com>,
Paul Handrigan <Paul.Handrigan@cirrus.com>,
linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] ASoC: cs35l32: avoid uninitialized variable access
Date: Sun, 6 Mar 2016 21:43:25 +0100 [thread overview]
Message-ID: <201603062143.25158.arnd@arndb.de> (raw)
In-Reply-To: <20160305145400.GF19428@n2100.arm.linux.org.uk>
On Saturday 05 March 2016, Russell King - ARM Linux wrote:
> On Mon, Jan 25, 2016 at 06:07:32PM +0100, Arnd Bergmann wrote:
> > gcc warns about the possibilty of accessing a property read from
> > devicetree in cs35l32_i2c_probe() when it has not been initialized
> > because CONFIG_OF is disabled:
> >
> > sound/soc/codecs/cs35l32.c: In function 'cs35l32_i2c_probe':
> > sound/soc/codecs/cs35l32.c:278:2: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >
> > The code is actually correct because it checks the dev->of_node
> > variable first and we know this is NULL here, but by adding a
> > check for IS_ENABLED(CONFIG_OF), we can let the compiler know
> > as well, and also generate smaller object code.
>
> No, the code is buggy, and the compiler is very correct in warning about
> it.
...
>
> Note that 'out_values' is not written to if of_find_property_value_of_size()
> returns an error. Therefore, if cirrus,boost-manager is missing, the
> resulting value of 'val' is left uninitialised.
You are right, this is an actual bug in the driver, and my patch just
hides it. Interestingly, this is not the case that the warning was about,
as we get the warning only when CONFIG_OF is disabled and the code
is correct (because dev->of_node is guaranteed to be NULL), but we
don't get the warning when CONFIG_OF is enabled and we can actually
run into the problem.
Fixing the driver to have correct error handling on the property
functions will make the warning go away and fix the bug you pointed
out, so we should definitely do that and drop my patch.
Arnd
prev parent reply other threads:[~2016-03-06 20:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-25 17:07 [PATCH 1/2] ASoC: cs35l32: avoid uninitialized variable access Arnd Bergmann
2016-01-25 17:07 ` [PATCH 2/2] ASoC: s3c24xx: use const snd_soc_component_driver pointer Arnd Bergmann
2016-01-26 4:09 ` Krzysztof Kozlowski
2016-03-05 5:39 ` [PATCH 1/2] ASoC: cs35l32: avoid uninitialized variable access Mark Brown
2016-03-05 14:54 ` Russell King - ARM Linux
2016-03-06 20:43 ` Arnd Bergmann [this message]
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=201603062143.25158.arnd@arndb.de \
--to=arnd@arndb.de \
--cc=Paul.Handrigan@cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=brian.austin@cirrus.com \
--cc=broonie@kernel.org \
--cc=kuninori.morimoto.gx@renesas.com \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox