* [PATCH 2/2] ASoC: s3c24xx: use const snd_soc_component_driver pointer
2016-01-25 17:07 [PATCH 1/2] ASoC: cs35l32: avoid uninitialized variable access Arnd Bergmann
@ 2016-01-25 17:07 ` 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
2 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-01-25 17:07 UTC (permalink / raw)
To: linux-arm-kernel
An older patch to convert the API in the s3c i2s driver
ended up passing a const pointer into a function that takes
a non-const pointer, so we now get a warning:
sound/soc/samsung/s3c2412-i2s.c: In function 's3c2412_iis_dev_probe':
sound/soc/samsung/s3c2412-i2s.c:172:9: error: passing argument 3 of 's3c_i2sv2_register_component' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
However, the s3c_i2sv2_register_component() function again
passes the pointer into another function taking a const, so
we just need to change its prototype.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: eca3b01d0885 ("ASoC: switch over to use snd_soc_register_component() on s3c i2s")
---
sound/soc/samsung/s3c-i2s-v2.c | 2 +-
sound/soc/samsung/s3c-i2s-v2.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/samsung/s3c-i2s-v2.c b/sound/soc/samsung/s3c-i2s-v2.c
index df65c5b494b1..b6ab3fc5789e 100644
--- a/sound/soc/samsung/s3c-i2s-v2.c
+++ b/sound/soc/samsung/s3c-i2s-v2.c
@@ -709,7 +709,7 @@ static int s3c2412_i2s_resume(struct snd_soc_dai *dai)
#endif
int s3c_i2sv2_register_component(struct device *dev, int id,
- struct snd_soc_component_driver *cmp_drv,
+ const struct snd_soc_component_driver *cmp_drv,
struct snd_soc_dai_driver *dai_drv)
{
struct snd_soc_dai_ops *ops = (struct snd_soc_dai_ops *)dai_drv->ops;
diff --git a/sound/soc/samsung/s3c-i2s-v2.h b/sound/soc/samsung/s3c-i2s-v2.h
index 90abab364b49..d0684145ed1f 100644
--- a/sound/soc/samsung/s3c-i2s-v2.h
+++ b/sound/soc/samsung/s3c-i2s-v2.h
@@ -101,7 +101,7 @@ extern int s3c_i2sv2_probe(struct snd_soc_dai *dai,
* soc core.
*/
extern int s3c_i2sv2_register_component(struct device *dev, int id,
- struct snd_soc_component_driver *cmp_drv,
+ const struct snd_soc_component_driver *cmp_drv,
struct snd_soc_dai_driver *dai_drv);
#endif /* __SND_SOC_S3C24XX_S3C_I2SV2_I2S_H */
--
2.7.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] ASoC: s3c24xx: use const snd_soc_component_driver pointer
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
0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-26 4:09 UTC (permalink / raw)
To: linux-arm-kernel
On 26.01.2016 02:07, Arnd Bergmann wrote:
> An older patch to convert the API in the s3c i2s driver
> ended up passing a const pointer into a function that takes
> a non-const pointer, so we now get a warning:
>
> sound/soc/samsung/s3c2412-i2s.c: In function 's3c2412_iis_dev_probe':
> sound/soc/samsung/s3c2412-i2s.c:172:9: error: passing argument 3 of 's3c_i2sv2_register_component' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
>
> However, the s3c_i2sv2_register_component() function again
> passes the pointer into another function taking a const, so
> we just need to change its prototype.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: eca3b01d0885 ("ASoC: switch over to use snd_soc_register_component() on s3c i2s")
> ---
> sound/soc/samsung/s3c-i2s-v2.c | 2 +-
> sound/soc/samsung/s3c-i2s-v2.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] ASoC: cs35l32: avoid uninitialized variable access
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-03-05 5:39 ` Mark Brown
2016-03-05 14:54 ` Russell King - ARM Linux
2 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2016-03-05 5:39 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 25, 2016 at 06:07:32PM +0100, Arnd Bergmann wrote:
> - if (i2c_client->dev.of_node) {
> + if (IS_ENABLED(CONFIG_OF) && i2c_client->dev.of_node) {
This feels it's going to be happening a lot and we should probably have
a dev_has_of_node() helper that does the IS_ENABLED() check.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160305/5f55b1dd/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] ASoC: cs35l32: avoid uninitialized variable access
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-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
2 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2016-03-05 14:54 UTC (permalink / raw)
To: linux-arm-kernel
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.
The code there is:
of_property_read_u32(np, "cirrus,boost-manager", &val);
switch (val) {
of_property_read_u32() is aliased to of_property_read_u32_array() via:
static inline int of_property_read_u32(const struct device_node *np,
const char *propname,
u32 *out_value)
{
return of_property_read_u32_array(np, propname, out_value, 1);
}
which does this:
int of_property_read_u32_array(const struct device_node *np,
const char *propname, u32 *out_values,
size_t sz)
{
const __be32 *val = of_find_property_value_of_size(np, propname,
(sz * sizeof(*out_values)));
if (IS_ERR(val))
return PTR_ERR(val);
while (sz--)
*out_values++ = be32_to_cpup(val++);
return 0;
}
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.
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] ASoC: cs35l32: avoid uninitialized variable access
2016-03-05 14:54 ` Russell King - ARM Linux
@ 2016-03-06 20:43 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-03-06 20:43 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread