diff mbox

ASoC: mc13783: Ensure we only try to dereference valid of_nodes

Message ID 1412779204-30876-1-git-send-email-broonie@kernel.org (mailing list archive)
State Accepted
Commit a66ae631a3cffb00f441b229a07fa1b4c72e738a
Headers show

Commit Message

Mark Brown Oct. 8, 2014, 2:40 p.m. UTC
Reported-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/mc13783.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Takashi Iwai Oct. 8, 2014, 3:06 p.m. UTC | #1
At Wed,  8 Oct 2014 15:40:04 +0100,
Mark Brown wrote:
> 
> Reported-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  sound/soc/codecs/mc13783.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
> index 388f90a597fa..71f775aad7c7 100644
> --- a/sound/soc/codecs/mc13783.c
> +++ b/sound/soc/codecs/mc13783.c
> @@ -765,12 +765,18 @@ static int __init mc13783_codec_probe(struct platform_device *pdev)
>  			return -ENOSYS;
>  
>  		ret = of_property_read_u32(np, "adc-port", &priv->adc_ssi_port);
> -		if (ret)
> -			goto out;
> +		if (ret) {
> +			of_node_put(np);
> +			return ret;
> +		}
>  
>  		ret = of_property_read_u32(np, "dac-port", &priv->dac_ssi_port);
> -		if (ret)
> -			goto out;
> +		if (ret) {
> +			of_node_put(np);
> +			return ret;
> +		}
> +
> +		of_node_put(np);
>  	}
>  
>  	dev_set_drvdata(&pdev->dev, priv);
> @@ -783,8 +789,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;
>  }

I'd move the declaration of np into if block, too, so that it won't be
referred in other places.


thanks,

Takashi
Mark Brown Oct. 8, 2014, 3:50 p.m. UTC | #2
On Wed, Oct 08, 2014 at 05:06:15PM +0200, Takashi Iwai wrote:

> I'd move the declaration of np into if block, too, so that it won't be
> referred in other places.

I generally dislike that style outside of very large functions; it's not
typical for C.
Takashi Iwai Oct. 8, 2014, 4:12 p.m. UTC | #3
At Wed, 8 Oct 2014 16:50:15 +0100,
Mark Brown wrote:
> 
> On Wed, Oct 08, 2014 at 05:06:15PM +0200, Takashi Iwai wrote:
> 
> > I'd move the declaration of np into if block, too, so that it won't be
> > referred in other places.
> 
> I generally dislike that style outside of very large functions; it's not
> typical for C.

OK, it's a matter of taste.  A typical bikeshed topic, we can spend
hours about this :)


Takashi
diff mbox

Patch

diff --git a/sound/soc/codecs/mc13783.c b/sound/soc/codecs/mc13783.c
index 388f90a597fa..71f775aad7c7 100644
--- a/sound/soc/codecs/mc13783.c
+++ b/sound/soc/codecs/mc13783.c
@@ -765,12 +765,18 @@  static int __init mc13783_codec_probe(struct platform_device *pdev)
 			return -ENOSYS;
 
 		ret = of_property_read_u32(np, "adc-port", &priv->adc_ssi_port);
-		if (ret)
-			goto out;
+		if (ret) {
+			of_node_put(np);
+			return ret;
+		}
 
 		ret = of_property_read_u32(np, "dac-port", &priv->dac_ssi_port);
-		if (ret)
-			goto out;
+		if (ret) {
+			of_node_put(np);
+			return ret;
+		}
+
+		of_node_put(np);
 	}
 
 	dev_set_drvdata(&pdev->dev, priv);
@@ -783,8 +789,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;
 }