[PATCH 0/3] Move call to PTR_ERR after reassignment

[PATCH 0/3] Move call to PTR_ERR after reassignment

[Julia Lawall]

These patches fix cases where a location is reset to NULL before being
passed to PTR_ERR.

[PATCH 1/3 v2] drivers/power/pda_power.c: Move call to PTR_ERR after reassignment

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

PTR_ERR should be called before its argument is cleared.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e,e1;
constant c;
@@

*e = c
... when != e = e1
    when != &e
    when != true IS_ERR(e)
*PTR_ERR(e)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Reported-by: Josh Triplett <josh@joshtriplett.org>

---
This patch probably only half way solves the problem with this code.  It would
seem that it should have a goto at the end, but I am not sure to where.

v2: correct commit message.

 drivers/power/pda_power.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/pda_power.c b/drivers/power/pda_power.c
index fd49689..16611c3 100644
--- a/drivers/power/pda_power.c
+++ b/drivers/power/pda_power.c
@@ -316,8 +316,8 @@ static int pda_power_probe(struct platform_device *pdev)
 	ac_draw = regulator_get(dev, "ac_draw");
 	if (IS_ERR(ac_draw)) {
 		dev_dbg(dev, "couldn't get ac_draw regulator\n");
-		ac_draw = NULL;
 		ret = PTR_ERR(ac_draw);
+		ac_draw = NULL;
 	}
 
 #ifdef CONFIG_USB_OTG_UTILS

[PATCH 2/3 v2] drivers/net/ethernet/ti: Move call to PTR_ERR after reassignment

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

PTR_ERR should be called before its argument is cleared.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e,e1;
constant c;
@@

*e = c
... when != e = e1
    when != &e
    when != true IS_ERR(e)
*PTR_ERR(e)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Reported-by: Josh Triplett <josh@joshtriplett.org>

---
v2: correct commit message.

 drivers/net/ethernet/ti/davinci_emac.c |    3 ++-
 drivers/net/ethernet/ti/davinci_mdio.c |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index efd4f3e..922a937 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1600,8 +1600,9 @@ static int emac_dev_open(struct net_device *ndev)
 		if (IS_ERR(priv->phydev)) {
 			dev_err(emac_dev, "could not connect to phy %s\n",
 				priv->phy_id);
+			ret = PTR_ERR(priv->phydev);
 			priv->phydev = NULL;
-			return PTR_ERR(priv->phydev);
+			return ret;
 		}
 
 		priv->link = 0;
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index ef7c9c1..af8b8fc 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -318,9 +318,9 @@ static int __devinit davinci_mdio_probe(struct platform_device *pdev)
 
 	data->clk = clk_get(dev, NULL);
 	if (IS_ERR(data->clk)) {
-		data->clk = NULL;
 		dev_err(dev, "failed to get device clock\n");
 		ret = PTR_ERR(data->clk);
+		data->clk = NULL;
 		goto bail_out;
 	}
 

[PATCH 3/3 v2] kernel/rcutorture.c: Move call to PTR_ERR after reassignment

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

PTR_ERR should be called before its argument is cleared.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e,e1;
constant c;
@@

*e = c
... when != e = e1
    when != &e
    when != true IS_ERR(e)
*PTR_ERR(e)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Reported-by: Josh Triplett <josh@joshtriplett.org>

---
v2: correct commit message.

 kernel/rcutorture.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index a58ac28..4b3cd87 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -1450,12 +1450,15 @@ rcu_torture_onoff(void *arg)
 static int __cpuinit
 rcu_torture_onoff_init(void)
 {
+	int ret;
+
 	if (onoff_interval <= 0)
 		return 0;
 	onoff_task = kthread_run(rcu_torture_onoff, NULL, "rcu_torture_onoff");
 	if (IS_ERR(onoff_task)) {
+		ret = PTR_ERR(onoff_task);
 		onoff_task = NULL;
-		return PTR_ERR(onoff_task);
+		return ret;
 	}
 	return 0;
 }

Re: [PATCH 3/3 v2] kernel/rcutorture.c: Move call to PTR_ERR after reassignment

[Paul E. McKenney]

Queued, thank you both!

							Thanx, Paul

On Thu, Feb 02, 2012 at 03:53:02PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> PTR_ERR should be called before its argument is cleared.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e,e1;
> constant c;
> @@
> 
> *e = c
> ... when != e = e1
>     when != &e
>     when != true IS_ERR(e)
> *PTR_ERR(e)
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> Reported-by: Josh Triplett <josh@joshtriplett.org>
> 
> ---
> v2: correct commit message.
> 
>  kernel/rcutorture.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> index a58ac28..4b3cd87 100644
> --- a/kernel/rcutorture.c
> +++ b/kernel/rcutorture.c
> @@ -1450,12 +1450,15 @@ rcu_torture_onoff(void *arg)
>  static int __cpuinit
>  rcu_torture_onoff_init(void)
>  {
> +	int ret;
> +
>  	if (onoff_interval <= 0)
>  		return 0;
>  	onoff_task = kthread_run(rcu_torture_onoff, NULL, "rcu_torture_onoff");
>  	if (IS_ERR(onoff_task)) {
> +		ret = PTR_ERR(onoff_task);
>  		onoff_task = NULL;
> -		return PTR_ERR(onoff_task);
> +		return ret;
>  	}
>  	return 0;
>  }
> 

Re: [PATCH 2/3 v2] drivers/net/ethernet/ti: Move call to PTR_ERR after reassignment

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3234 bytes --]

On Thu, 2 Feb 2012, D A wrote:

> hello all,
> 
> im in the janitors for a while now and i keep looking at the emails you keep
> sending to eachother, but i m lost, i have a back ground in C how can i be
> part of this, any body can give some help to start, a small task that i can
> begin with ?

There are hundreds of places where the various devm_ functions, eg 
devm_kzalloc, could be used.  Proceed slowly and carefully...

Documentation/driver-model/devres.txt

julia


> 
> there must be a way or a strategy to absorb new members, i m 3 months now or
> more.
> 
> 
> On Thu, Feb 2, 2012 at 4:53 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
>       From: Julia Lawall <Julia.Lawall@lip6.fr>
>
>       PTR_ERR should be called before its argument is cleared.
>
>       The semantic match that finds this problem is as follows:
>       (http://coccinelle.lip6.fr/)
>
>       // <smpl>
>       @@
>       expression e,e1;
>       constant c;
>       @@
>
>       *e = c
>       ... when != e = e1
>          when != &e
>          when != true IS_ERR(e)
>       *PTR_ERR(e)
>       // </smpl>
>
>       Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>       Reported-by: Josh Triplett <josh@joshtriplett.org>
>
>       ---
>       v2: correct commit message.
>
>        drivers/net/ethernet/ti/davinci_emac.c |    3 ++-
>        drivers/net/ethernet/ti/davinci_mdio.c |    2 +-
>        2 files changed, 3 insertions(+), 2 deletions(-)
>
>       diff --git a/drivers/net/ethernet/ti/davinci_emac.c
>       b/drivers/net/ethernet/ti/davinci_emac.c
>       index efd4f3e..922a937 100644
>       --- a/drivers/net/ethernet/ti/davinci_emac.c
>       +++ b/drivers/net/ethernet/ti/davinci_emac.c
>       @@ -1600,8 +1600,9 @@ static int emac_dev_open(struct net_device
>       *ndev)
>                      if (IS_ERR(priv->phydev)) {
>                              dev_err(emac_dev, "could not connect to
>       phy %s\n",
>                                      priv->phy_id);
>       +                       ret = PTR_ERR(priv->phydev);
>                              priv->phydev = NULL;
>       -                       return PTR_ERR(priv->phydev);
>       +                       return ret;
>                      }
>
>                      priv->link = 0;
>       diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
>       b/drivers/net/ethernet/ti/davinci_mdio.c
>       index ef7c9c1..af8b8fc 100644
>       --- a/drivers/net/ethernet/ti/davinci_mdio.c
>       +++ b/drivers/net/ethernet/ti/davinci_mdio.c
>       @@ -318,9 +318,9 @@ static int __devinit
>       davinci_mdio_probe(struct platform_device *pdev)
>
>              data->clk = clk_get(dev, NULL);
>              if (IS_ERR(data->clk)) {
>       -               data->clk = NULL;
>                      dev_err(dev, "failed to get device clock\n");
>                      ret = PTR_ERR(data->clk);
>       +               data->clk = NULL;
>                      goto bail_out;
>              }
> 
>
>       --
>       To unsubscribe from this list: send the line "unsubscribe
>       kernel-janitors" in
>       the body of a message to majordomo@vger.kernel.org
>       More majordomo info at
>        http://vger.kernel.org/majordomo-info.html
> 
> 
> 
>

Re: [PATCH 2/3 v2] drivers/net/ethernet/ti: Move call to PTR_ERR after reassignment

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Thu,  2 Feb 2012 15:53:01 +0100

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> PTR_ERR should be called before its argument is cleared.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> Reported-by: Josh Triplett <josh@joshtriplett.org>

Applied, thanks.