* [PATCH 0/4] ASoC trivial fixes
@ 2014-10-07 16:19 Takashi Iwai
2014-10-07 16:19 ` [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object Takashi Iwai
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 16:19 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
Hi,
this is a series of fixes for the bugs I pointed yesterday.
Since no fix action was seen yet, I quickly put band-aid over them.
thanks,
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 16:19 [PATCH 0/4] ASoC trivial fixes Takashi Iwai
@ 2014-10-07 16:19 ` Takashi Iwai
2014-10-07 17:09 ` Mark Brown
2014-10-07 16:19 ` [PATCH 2/4] ASoC: eukrea-tlv320: " Takashi Iwai
` (3 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 16:19 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
The commit [a28d167fbbef: ASoC: mc13783: Add missing of_node_put]
fixed the leak of OF node, but it calls of_node_put() unconditionally
at error path where the passed pointer might be uninitialized. It was
caught by a compiler warning:
sound/soc/codecs/mc13783.c:787:13: warning: 'np' may be used uninitialized in this function [-Wuninitialized]
This patch adds the NULL initialization properly for fixing this.
Fixes: a28d167fbbef ('ASoC: mc13783: Add missing of_node_put')
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/codecs/mc13783.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
index 388f90a597fa..f54fdf6fc20d 100644
--- a/sound/soc/codecs/mc13783.c
+++ b/sound/soc/codecs/mc13783.c
@@ -749,7 +749,7 @@ static int __init mc13783_codec_probe(struct platform_device *pdev)
{
struct mc13783_priv *priv;
struct mc13xxx_codec_platform_data *pdata = pdev->dev.platform_data;
- struct device_node *np;
+ struct device_node *np = NULL;
int ret;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
--
2.1.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/4] ASoC: eukrea-tlv320: Fix of_node_put() call with uninitialized object
2014-10-07 16:19 [PATCH 0/4] ASoC trivial fixes Takashi Iwai
2014-10-07 16:19 ` [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object Takashi Iwai
@ 2014-10-07 16:19 ` Takashi Iwai
2014-10-07 17:18 ` Mark Brown
2014-10-07 16:19 ` [PATCH 3/4] ASoC: imx-es8328: " Takashi Iwai
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 16:19 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
The of_node_put() call in eukrea_tlv320_probe() may take an
uninitialized pointer, as compiler spotted out:
sound/soc/codecs/mc13783.c:787:13: warning: 'np' may be used uninitialized in this function [-Wuninitialized]
This patch adds the proper NULL initialization as a fix.
Fixes: 66f232908de2 ('ASoC: eukrea-tlv320: Add DT support')
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/fsl/eukrea-tlv320.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/eukrea-tlv320.c b/sound/soc/fsl/eukrea-tlv320.c
index eb093d5b85c4..d53b002595b6 100644
--- a/sound/soc/fsl/eukrea-tlv320.c
+++ b/sound/soc/fsl/eukrea-tlv320.c
@@ -105,7 +105,7 @@ static int eukrea_tlv320_probe(struct platform_device *pdev)
int ret;
int int_port = 0, ext_port;
struct device_node *np = pdev->dev.of_node;
- struct device_node *ssi_np, *codec_np;
+ struct device_node *ssi_np = NULL, *codec_np;
eukrea_tlv320.dev = &pdev->dev;
if (np) {
--
2.1.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/4] ASoC: imx-es8328: Fix of_node_put() call with uninitialized object
2014-10-07 16:19 [PATCH 0/4] ASoC trivial fixes Takashi Iwai
2014-10-07 16:19 ` [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object Takashi Iwai
2014-10-07 16:19 ` [PATCH 2/4] ASoC: eukrea-tlv320: " Takashi Iwai
@ 2014-10-07 16:19 ` Takashi Iwai
2014-10-07 22:52 ` Mark Brown
2014-10-07 16:19 ` [PATCH 4/4] ASoC: imx-es8328: Fix missing return code in imx_es8328_probe() Takashi Iwai
2014-10-07 17:11 ` [PATCH 0/4] ASoC trivial fixes Mark Brown
4 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 16:19 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
The of_node_put() calls in imx_es8328_probe() may take uninitialized
pointers when reached though the early error path. This patch adds
the proper NULL initialization for fixing these.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/fsl/imx-es8328.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/imx-es8328.c b/sound/soc/fsl/imx-es8328.c
index 653e66d150c8..d6c55c88a069 100644
--- a/sound/soc/fsl/imx-es8328.c
+++ b/sound/soc/fsl/imx-es8328.c
@@ -78,7 +78,7 @@ static const struct snd_soc_dapm_widget imx_es8328_dapm_widgets[] = {
static int imx_es8328_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct device_node *ssi_np, *codec_np;
+ struct device_node *ssi_np = NULL, *codec_np = NULL;
struct platform_device *ssi_pdev;
struct imx_es8328_data *data;
u32 int_port, ext_port;
--
2.1.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/4] ASoC: imx-es8328: Fix missing return code in imx_es8328_probe()
2014-10-07 16:19 [PATCH 0/4] ASoC trivial fixes Takashi Iwai
` (2 preceding siblings ...)
2014-10-07 16:19 ` [PATCH 3/4] ASoC: imx-es8328: " Takashi Iwai
@ 2014-10-07 16:19 ` Takashi Iwai
2014-10-07 17:10 ` Mark Brown
2014-10-07 17:11 ` [PATCH 0/4] ASoC trivial fixes Mark Brown
4 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 16:19 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
An error code was forgotten to be passed in the error path of
imx_es8328_probe().
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/fsl/imx-es8328.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/fsl/imx-es8328.c b/sound/soc/fsl/imx-es8328.c
index d6c55c88a069..f8cf10e16ce9 100644
--- a/sound/soc/fsl/imx-es8328.c
+++ b/sound/soc/fsl/imx-es8328.c
@@ -104,6 +104,7 @@ static int imx_es8328_probe(struct platform_device *pdev)
if (ext_port > MUX_PORT_MAX || ext_port == 0) {
dev_err(dev, "mux-ext-port: hardware only has %d mux ports\n",
MUX_PORT_MAX);
+ ret = -EINVAL;
goto fail;
}
--
2.1.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 16:19 ` [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object Takashi Iwai
@ 2014-10-07 17:09 ` Mark Brown
2014-10-07 17:17 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2014-10-07 17:09 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 221 bytes --]
On Tue, Oct 07, 2014 at 06:19:51PM +0200, Takashi Iwai wrote:
> - struct device_node *np;
> + struct device_node *np = NULL;
No, unconditional initialisations like this are worse than the problem
they're trying to fix.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] ASoC: imx-es8328: Fix missing return code in imx_es8328_probe()
2014-10-07 16:19 ` [PATCH 4/4] ASoC: imx-es8328: Fix missing return code in imx_es8328_probe() Takashi Iwai
@ 2014-10-07 17:10 ` Mark Brown
0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2014-10-07 17:10 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 166 bytes --]
On Tue, Oct 07, 2014 at 06:19:54PM +0200, Takashi Iwai wrote:
> An error code was forgotten to be passed in the error path of
> imx_es8328_probe().
Applied, thanks.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/4] ASoC trivial fixes
2014-10-07 16:19 [PATCH 0/4] ASoC trivial fixes Takashi Iwai
` (3 preceding siblings ...)
2014-10-07 16:19 ` [PATCH 4/4] ASoC: imx-es8328: Fix missing return code in imx_es8328_probe() Takashi Iwai
@ 2014-10-07 17:11 ` Mark Brown
2014-10-07 17:17 ` Takashi Iwai
4 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2014-10-07 17:11 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 305 bytes --]
On Tue, Oct 07, 2014 at 06:19:50PM +0200, Takashi Iwai wrote:
> this is a series of fixes for the bugs I pointed yesterday.
> Since no fix action was seen yet, I quickly put band-aid over them.
So, I now checked who you'd Cced - you didn't CC the Freescale guys so
I'm not surprised you got no reply...
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 17:09 ` Mark Brown
@ 2014-10-07 17:17 ` Takashi Iwai
2014-10-07 17:23 ` Mark Brown
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 17:17 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
At Tue, 7 Oct 2014 18:09:38 +0100,
Mark Brown wrote:
>
> On Tue, Oct 07, 2014 at 06:19:51PM +0200, Takashi Iwai wrote:
>
> > - struct device_node *np;
> > + struct device_node *np = NULL;
>
> No, unconditional initialisations like this are worse than the problem
> they're trying to fix.
Which problem they're trying to fix...?
Initializing with NULL for the of_node_put() at error path is a
standard idiom. An alternative fix would be to add "if (!pdata)"
before of_node_put(np). But this isn't really intuitive, either (and
even more error-prone, IMO).
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/4] ASoC trivial fixes
2014-10-07 17:11 ` [PATCH 0/4] ASoC trivial fixes Mark Brown
@ 2014-10-07 17:17 ` Takashi Iwai
0 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 17:17 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
At Tue, 7 Oct 2014 18:11:34 +0100,
Mark Brown wrote:
>
> On Tue, Oct 07, 2014 at 06:19:50PM +0200, Takashi Iwai wrote:
>
> > this is a series of fixes for the bugs I pointed yesterday.
> > Since no fix action was seen yet, I quickly put band-aid over them.
>
> So, I now checked who you'd Cced - you didn't CC the Freescale guys so
> I'm not surprised you got no reply...
Ah, right, I missed them for these parts.
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] ASoC: eukrea-tlv320: Fix of_node_put() call with uninitialized object
2014-10-07 16:19 ` [PATCH 2/4] ASoC: eukrea-tlv320: " Takashi Iwai
@ 2014-10-07 17:18 ` Mark Brown
2014-10-07 17:39 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2014-10-07 17:18 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 283 bytes --]
On Tue, Oct 07, 2014 at 06:19:52PM +0200, Takashi Iwai wrote:
> - struct device_node *ssi_np, *codec_np;
> + struct device_node *ssi_np = NULL, *codec_np;
As well as the NULL thing mixing initialized and unintialized things in
one declaration is something that's normally avoided.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 17:17 ` Takashi Iwai
@ 2014-10-07 17:23 ` Mark Brown
2014-10-07 17:39 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2014-10-07 17:23 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 905 bytes --]
On Tue, Oct 07, 2014 at 07:17:08PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > On Tue, Oct 07, 2014 at 06:19:51PM +0200, Takashi Iwai wrote:
> > > - struct device_node *np;
> > > + struct device_node *np = NULL;
> > No, unconditional initialisations like this are worse than the problem
> > they're trying to fix.
> Which problem they're trying to fix...?
Shutting up warnings - because they just brute forcing they mean that if
there's anything else we've missed the compiler won't be able to tell us
about it.
> Initializing with NULL for the of_node_put() at error path is a
> standard idiom. An alternative fix would be to add "if (!pdata)"
> before of_node_put(np). But this isn't really intuitive, either (and
> even more error-prone, IMO).
Well, in this case I'd just move the of_node_put() into the existing
check for pdata since we don't ever reference np outside of that anyway.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 17:23 ` Mark Brown
@ 2014-10-07 17:39 ` Takashi Iwai
2014-10-07 18:04 ` Mark Brown
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 17:39 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
At Tue, 7 Oct 2014 18:23:37 +0100,
Mark Brown wrote:
>
> On Tue, Oct 07, 2014 at 07:17:08PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
>
> > > On Tue, Oct 07, 2014 at 06:19:51PM +0200, Takashi Iwai wrote:
>
> > > > - struct device_node *np;
> > > > + struct device_node *np = NULL;
>
> > > No, unconditional initialisations like this are worse than the problem
> > > they're trying to fix.
>
> > Which problem they're trying to fix...?
>
> Shutting up warnings - because they just brute forcing they mean that if
> there's anything else we've missed the compiler won't be able to tell us
> about it.
>
> > Initializing with NULL for the of_node_put() at error path is a
> > standard idiom. An alternative fix would be to add "if (!pdata)"
> > before of_node_put(np). But this isn't really intuitive, either (and
> > even more error-prone, IMO).
>
> Well, in this case I'd just move the of_node_put() into the existing
> check for pdata since we don't ever reference np outside of that anyway.
Yeah, that's an option, too, but it'd make the code less readable.
So I chose the straightforward way.
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] ASoC: eukrea-tlv320: Fix of_node_put() call with uninitialized object
2014-10-07 17:18 ` Mark Brown
@ 2014-10-07 17:39 ` Takashi Iwai
2014-10-07 18:56 ` [PATCH v2 " Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 17:39 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
At Tue, 7 Oct 2014 18:18:00 +0100,
Mark Brown wrote:
>
> On Tue, Oct 07, 2014 at 06:19:52PM +0200, Takashi Iwai wrote:
>
> > - struct device_node *ssi_np, *codec_np;
> > + struct device_node *ssi_np = NULL, *codec_np;
>
> As well as the NULL thing mixing initialized and unintialized things in
> one declaration is something that's normally avoided.
I thought of that, too. For simplicity, we can do NULL
initializations for both, of course, if you prefer.
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 17:39 ` Takashi Iwai
@ 2014-10-07 18:04 ` Mark Brown
2014-10-07 18:14 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2014-10-07 18:04 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 513 bytes --]
On Tue, Oct 07, 2014 at 07:39:03PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > Well, in this case I'd just move the of_node_put() into the existing
> > check for pdata since we don't ever reference np outside of that anyway.
> Yeah, that's an option, too, but it'd make the code less readable.
> So I chose the straightforward way.
I don't actually see it as a readability concern - to me having the get
and release close to each other and especially having them at the same
level of indentation helps.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 18:04 ` Mark Brown
@ 2014-10-07 18:14 ` Takashi Iwai
2014-10-07 18:54 ` Mark Brown
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 18:14 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
At Tue, 7 Oct 2014 19:04:34 +0100,
Mark Brown wrote:
>
> On Tue, Oct 07, 2014 at 07:39:03PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
>
> > > Well, in this case I'd just move the of_node_put() into the existing
> > > check for pdata since we don't ever reference np outside of that anyway.
>
> > Yeah, that's an option, too, but it'd make the code less readable.
> > So I chose the straightforward way.
>
> I don't actually see it as a readability concern - to me having the get
> and release close to each other and especially having them at the same
> level of indentation helps.
I do understand the merit, but it looks uglier to my taste.
The success path goes again with if (ret). (Or we'd need two goto's
or deeper if-else blocks.)
Takashi
diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
index 388f90a597fa..cffbf09ba67c 100644
--- a/sound/soc/codecs/mc13783.c
+++ b/sound/soc/codecs/mc13783.c
@@ -749,7 +749,6 @@ static int __init mc13783_codec_probe(struct platform_device *pdev)
{
struct mc13783_priv *priv;
struct mc13xxx_codec_platform_data *pdata = pdev->dev.platform_data;
- struct device_node *np;
int ret;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -760,6 +759,8 @@ static int __init mc13783_codec_probe(struct platform_device *pdev)
priv->adc_ssi_port = pdata->adc_ssi_port;
priv->dac_ssi_port = pdata->dac_ssi_port;
} else {
+ struct device_node *np;
+
np = of_get_child_by_name(pdev->dev.parent->of_node, "codec");
if (!np)
return -ENOSYS;
@@ -771,6 +772,10 @@ static int __init mc13783_codec_probe(struct platform_device *pdev)
ret = of_property_read_u32(np, "dac-port", &priv->dac_ssi_port);
if (ret)
goto out;
+ out:
+ of_node_put(np);
+ if (ret)
+ return ret;
}
dev_set_drvdata(&pdev->dev, priv);
@@ -783,8 +788,6 @@ static int __init mc13783_codec_probe(struct platform_device *pdev)
ret = snd_soc_register_codec(&pdev->dev, &soc_codec_dev_mc13783,
mc13783_dai_async, ARRAY_SIZE(mc13783_dai_async));
-out:
- of_node_put(np);
return ret;
}
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 18:14 ` Takashi Iwai
@ 2014-10-07 18:54 ` Mark Brown
2014-10-07 18:58 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2014-10-07 18:54 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 585 bytes --]
On Tue, Oct 07, 2014 at 08:14:00PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > I don't actually see it as a readability concern - to me having the get
> > and release close to each other and especially having them at the same
> > level of indentation helps.
> I do understand the merit, but it looks uglier to my taste.
> The success path goes again with if (ret). (Or we'd need two goto's
> or deeper if-else blocks.)
That looks ugly, yes - I'd be doing a check for ret before the second
property call or something. Or just put the pdata check in the existing
error path.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/4] ASoC: eukrea-tlv320: Fix of_node_put() call with uninitialized object
2014-10-07 17:39 ` Takashi Iwai
@ 2014-10-07 18:56 ` Takashi Iwai
2014-10-07 19:10 ` Mark Brown
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 18:56 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
The of_node_put() call in eukrea_tlv320_probe() may take an
uninitialized pointer, as compiler spotted out:
sound/soc/fsl/eukrea-tlv320.c:221:14: warning: 'ssi_np' may be used uninitialized in this function [-Wuninitialized]
This patch adds the proper NULL initializations as a fix.
(codec_np is also NULL initialized just for consistency.)
Fixes: 66f232908de2 ('ASoC: eukrea-tlv320: Add DT support')
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/fsl/eukrea-tlv320.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/eukrea-tlv320.c b/sound/soc/fsl/eukrea-tlv320.c
index eb093d5b85c4..d53b002595b6 100644
--- a/sound/soc/fsl/eukrea-tlv320.c
+++ b/sound/soc/fsl/eukrea-tlv320.c
@@ -105,7 +105,7 @@ static int eukrea_tlv320_probe(struct platform_device *pdev)
int ret;
int int_port = 0, ext_port;
struct device_node *np = pdev->dev.of_node;
- struct device_node *ssi_np, *codec_np;
+ struct device_node *ssi_np = NULL, *codec_np = NULL;
eukrea_tlv320.dev = &pdev->dev;
if (np) {
--
2.1.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 18:54 ` Mark Brown
@ 2014-10-07 18:58 ` Takashi Iwai
2014-10-07 23:01 ` Mark Brown
0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2014-10-07 18:58 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
At Tue, 7 Oct 2014 19:54:43 +0100,
Mark Brown wrote:
>
> On Tue, Oct 07, 2014 at 08:14:00PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
>
> > > I don't actually see it as a readability concern - to me having the get
> > > and release close to each other and especially having them at the same
> > > level of indentation helps.
>
> > I do understand the merit, but it looks uglier to my taste.
> > The success path goes again with if (ret). (Or we'd need two goto's
> > or deeper if-else blocks.)
>
> That looks ugly, yes - I'd be doing a check for ret before the second
> property call or something. Or just put the pdata check in the existing
> error path.
Well, I don't mind much how it'll be fixed, so I rather toss this fix
to you. Feel free to cook :)
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/4] ASoC: eukrea-tlv320: Fix of_node_put() call with uninitialized object
2014-10-07 18:56 ` [PATCH v2 " Takashi Iwai
@ 2014-10-07 19:10 ` Mark Brown
0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2014-10-07 19:10 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 471 bytes --]
On Tue, Oct 07, 2014 at 08:56:29PM +0200, Takashi Iwai wrote:
> The of_node_put() call in eukrea_tlv320_probe() may take an
> uninitialized pointer, as compiler spotted out:
> sound/soc/fsl/eukrea-tlv320.c:221:14: warning: 'ssi_np' may be used uninitialized in this function [-Wuninitialized]
Applied, but please don't send new patches in the middle of e-mail
threads - it can get really confusing trying to work out what the
current version of the series looks like.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/4] ASoC: imx-es8328: Fix of_node_put() call with uninitialized object
2014-10-07 16:19 ` [PATCH 3/4] ASoC: imx-es8328: " Takashi Iwai
@ 2014-10-07 22:52 ` Mark Brown
0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2014-10-07 22:52 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 272 bytes --]
On Tue, Oct 07, 2014 at 06:19:53PM +0200, Takashi Iwai wrote:
> The of_node_put() calls in imx_es8328_probe() may take uninitialized
> pointers when reached though the early error path. This patch adds
> the proper NULL initialization for fixing these.
Applied, thanks.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 18:58 ` Takashi Iwai
@ 2014-10-07 23:01 ` Mark Brown
2014-10-08 5:28 ` Takashi Iwai
0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2014-10-07 23:01 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
[-- Attachment #1.1: Type: text/plain, Size: 431 bytes --]
On Tue, Oct 07, 2014 at 08:58:38PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > That looks ugly, yes - I'd be doing a check for ret before the second
> > property call or something. Or just put the pdata check in the existing
> > error path.
> Well, I don't mind much how it'll be fixed, so I rather toss this fix
> to you. Feel free to cook :)
Well, I can't even see the warning so as far as I can tell it's all
fixed!
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object
2014-10-07 23:01 ` Mark Brown
@ 2014-10-08 5:28 ` Takashi Iwai
0 siblings, 0 replies; 23+ messages in thread
From: Takashi Iwai @ 2014-10-08 5:28 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel
At Wed, 8 Oct 2014 00:01:50 +0100,
Mark Brown wrote:
>
> On Tue, Oct 07, 2014 at 08:58:38PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
>
> > > That looks ugly, yes - I'd be doing a check for ret before the second
> > > property call or something. Or just put the pdata check in the existing
> > > error path.
>
> > Well, I don't mind much how it'll be fixed, so I rather toss this fix
> > to you. Feel free to cook :)
>
> Well, I can't even see the warning so as far as I can tell it's all
> fixed!
Oh, rather trust your eyes, the fault is there obviously.
It's a real bug that can be easily triggered, not in an exceptional
error path.
BTW, putting pdata check in the exit path is more error-prone, as
already mentioned. If anyone puts a new code with "goto out" before
assignment of np, it hits again. So, a NULL initialization would be
safer in the end.
Takashi
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2014-10-08 5:28 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-07 16:19 [PATCH 0/4] ASoC trivial fixes Takashi Iwai
2014-10-07 16:19 ` [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object Takashi Iwai
2014-10-07 17:09 ` Mark Brown
2014-10-07 17:17 ` Takashi Iwai
2014-10-07 17:23 ` Mark Brown
2014-10-07 17:39 ` Takashi Iwai
2014-10-07 18:04 ` Mark Brown
2014-10-07 18:14 ` Takashi Iwai
2014-10-07 18:54 ` Mark Brown
2014-10-07 18:58 ` Takashi Iwai
2014-10-07 23:01 ` Mark Brown
2014-10-08 5:28 ` Takashi Iwai
2014-10-07 16:19 ` [PATCH 2/4] ASoC: eukrea-tlv320: " Takashi Iwai
2014-10-07 17:18 ` Mark Brown
2014-10-07 17:39 ` Takashi Iwai
2014-10-07 18:56 ` [PATCH v2 " Takashi Iwai
2014-10-07 19:10 ` Mark Brown
2014-10-07 16:19 ` [PATCH 3/4] ASoC: imx-es8328: " Takashi Iwai
2014-10-07 22:52 ` Mark Brown
2014-10-07 16:19 ` [PATCH 4/4] ASoC: imx-es8328: Fix missing return code in imx_es8328_probe() Takashi Iwai
2014-10-07 17:10 ` Mark Brown
2014-10-07 17:11 ` [PATCH 0/4] ASoC trivial fixes Mark Brown
2014-10-07 17:17 ` Takashi Iwai
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.