[alsa-devel] [PATCH 1/4] ASoC: mc13783: Fix of_node_put() call with uninitialized object

[Takashi Iwai]

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;
 }