[PATCH 0/14] adjust duplicate test

[PATCH 0/14] adjust duplicate test

[Julia Lawall]

These patches fix cases where a value is tested that was previously
tested.  Often the problem is that the tested value has not been updated
properly.  Sometimes the test is simply duplicated.  These problems were
found using the following semantic match (http://coccinelle.lip6.fr/):

// <smpl>
@r exists@
expression e1,e2;
identifier f,g;
position p1,p2;
@@

if@p1 ( \(e1->f = NULL\|IS_ERR(e1->f)\) ) { ... when forall
   return ...; }
... when != \(e1->f = e2\|e1->f += e2\|e1->f -= e2\|e1->f |= e2\|e1->f &= e2\|e1->f++\|e1->f--\|g(...,e1,...)\)
if@p2 ( \(e1->f = NULL\|IS_ERR(e1->f)\) ) { ... when forall
   return ...; }

@rsame exists@
expression e;
position r.p1,r.p2;
statement S1,S2;
@@

if@p1 ( e ) S1
... when any
if@p2 ( e ) S2

@rok depends on rsame exists@
expression e1,e2;
identifier f;
position r.p1,r.p2;
statement S1,S2;
@@

if@p1 ( \(e1->f = NULL\|IS_ERR(e1->f)\) ) S1
... when any
 \(e1->f = e2\|e1->f += e2\|e1->f -= e2\|e1->f |= e2\|e1->f &= e2\|e1->f++\|e1->f--\|g(...,e1,...)\)
... when any
if@p2 ( ... ) S2

@rother depends on rsame exists@
position r.p1,r.p2;
statement S;
@@

... when != if@p1 (...) S
if@p2 ( ... ) { ... return ...; }

@script:python depends on rsame && !rok && !rother@
p1 << r.p1;
p2 << r.p2;
@@

if (p1[0].line != p2[0].line):
  cocci.print_main("",p1)
  cocci.print_secs("",p2)

@s exists@
local idexpression y;
expression e;
position p1,p2;
@@


if@p1 ( \(y = NULL\|IS_ERR(y)\|y != 0\) ) { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\|XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
if@p2 ( \(y = NULL\|IS_ERR(y)\|y != 0\) ) { ... when forall
   return ...; }

@ssame exists@
expression e;
position s.p1,s.p2;
statement S1,S2;
@@

if@p1 ( e ) S1
... when any
if@p2 ( e ) S2

@sok depends on ssame exists@
local idexpression y;
expression e;
position s.p1,s.p2;
statement S1,S2;
@@

if@p1 ( \(y = NULL\|IS_ERR(y)\|y != 0\) ) S1
... when any
 \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\|XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
... when any
if@p2 ( ... ) S2

@sother depends on ssame exists@
position s.p1,s.p2;
statement S;
@@

... when != if@p1 (...) S
if@p2 ( ... ) { ... return ...; }

@script:python depends on ssame && !sok && !sother@
p1 << s.p1;
p2 << s.p2;
@@

if (p1[0].line != p2[0].line):
  cocci.print_main("",p1)
  cocci.print_secs("",p2)

// </smpl>

[PATCH 0/14] adjust duplicate test

[Julia Lawall]

These patches fix cases where a value is tested that was previously
tested.  Often the problem is that the tested value has not been updated
properly.  Sometimes the test is simply duplicated.  These problems were
found using the following semantic match (http://coccinelle.lip6.fr/):

// <smpl>
@r exists@
expression e1,e2;
identifier f,g;
position p1,p2;
@@

if@p1 ( \(e1->f == NULL\|IS_ERR(e1->f)\) ) { ... when forall
   return ...; }
... when != \(e1->f = e2\|e1->f += e2\|e1->f -= e2\|e1->f |= e2\|e1->f &= e2\|e1->f++\|e1->f--\|g(...,e1,...)\)
if@p2 ( \(e1->f == NULL\|IS_ERR(e1->f)\) ) { ... when forall
   return ...; }

@rsame exists@
expression e;
position r.p1,r.p2;
statement S1,S2;
@@

if@p1 ( e ) S1
... when any
if@p2 ( e ) S2

@rok depends on rsame exists@
expression e1,e2;
identifier f;
position r.p1,r.p2;
statement S1,S2;
@@

if@p1 ( \(e1->f == NULL\|IS_ERR(e1->f)\) ) S1
... when any
 \(e1->f = e2\|e1->f += e2\|e1->f -= e2\|e1->f |= e2\|e1->f &= e2\|e1->f++\|e1->f--\|g(...,e1,...)\)
... when any
if@p2 ( ... ) S2

@rother depends on rsame exists@
position r.p1,r.p2;
statement S;
@@

... when != if@p1 (...) S
if@p2 ( ... ) { ... return ...; }

@script:python depends on rsame && !rok && !rother@
p1 << r.p1;
p2 << r.p2;
@@

if (p1[0].line != p2[0].line):
  cocci.print_main("",p1)
  cocci.print_secs("",p2)

@s exists@
local idexpression y;
expression e;
position p1,p2;
@@


if@p1 ( \(y == NULL\|IS_ERR(y)\|y != 0\) ) { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\|XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
if@p2 ( \(y == NULL\|IS_ERR(y)\|y != 0\) ) { ... when forall
   return ...; }

@ssame exists@
expression e;
position s.p1,s.p2;
statement S1,S2;
@@

if@p1 ( e ) S1
... when any
if@p2 ( e ) S2

@sok depends on ssame exists@
local idexpression y;
expression e;
position s.p1,s.p2;
statement S1,S2;
@@

if@p1 ( \(y == NULL\|IS_ERR(y)\|y != 0\) ) S1
... when any
 \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\|XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
... when any
if@p2 ( ... ) S2

@sother depends on ssame exists@
position s.p1,s.p2;
statement S;
@@

... when != if@p1 (...) S
if@p2 ( ... ) { ... return ...; }

@script:python depends on ssame && !sok && !sother@
p1 << s.p1;
p2 << s.p2;
@@

if (p1[0].line != p2[0].line):
  cocci.print_main("",p1)
  cocci.print_secs("",p2)

// </smpl>

[PATCH 11/15] tools/lib/traceevent/event-parse.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  Field was already tested.
Alloc_arg can fail, so test that value instead.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 tools/lib/traceevent/event-parse.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 5a824e3..7538a1f 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2481,7 +2481,7 @@ process_dynamic_array(struct event_format *event, struct print_arg *arg, char **
 
 	free_token(token);
 	arg = alloc_arg();
-	if (!field) {
+	if (!arg) {
 		do_warning("%s: not enough memory!", __func__);
 		*tok = NULL;
 		return EVENT_ERROR;

[PATCH 11/15] tools/lib/traceevent/event-parse.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  Field was already tested.
Alloc_arg can fail, so test that value instead.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 tools/lib/traceevent/event-parse.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 5a824e3..7538a1f 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2481,7 +2481,7 @@ process_dynamic_array(struct event_format *event, struct print_arg *arg, char **
 
 	free_token(token);
 	arg = alloc_arg();
-	if (!field) {
+	if (!arg) {
 		do_warning("%s: not enough memory!", __func__);
 		*tok = NULL;
 		return EVENT_ERROR;

Re: [PATCH 11/15] tools/lib/traceevent/event-parse.c: adjust duplicate test

[Steven Rostedt]

On Mon, 2013-01-21 at 14:02 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  Field was already tested.
> Alloc_arg can fail, so test that value instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> ---
>  tools/lib/traceevent/event-parse.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 5a824e3..7538a1f 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -2481,7 +2481,7 @@ process_dynamic_array(struct event_format *event, struct print_arg *arg, char **
>  
>  	free_token(token);
>  	arg = alloc_arg();
> -	if (!field) {
> +	if (!arg) {
>  		do_warning("%s: not enough memory!", __func__);
>  		*tok = NULL;
>  		return EVENT_ERROR;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 11/15] tools/lib/traceevent/event-parse.c: adjust duplicate test

[Steven Rostedt]

On Mon, 2013-01-21 at 14:02 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  Field was already tested.
> Alloc_arg can fail, so test that value instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> ---
>  tools/lib/traceevent/event-parse.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 5a824e3..7538a1f 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -2481,7 +2481,7 @@ process_dynamic_array(struct event_format *event, struct print_arg *arg, char **
>  
>  	free_token(token);
>  	arg = alloc_arg();
> -	if (!field) {
> +	if (!arg) {
>  		do_warning("%s: not enough memory!", __func__);
>  		*tok = NULL;
>  		return EVENT_ERROR;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 11/15] tools/lib/traceevent/event-parse.c: adjust duplicate test

[Arnaldo Carvalho de Melo]

Em Wed, Jan 23, 2013 at 09:42:17AM -0500, Steven Rostedt escreveu:
> On Mon, 2013-01-21 at 14:02 +0100, Julia Lawall wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > 
> > Delete successive tests to the same location.  Field was already tested.
> > Alloc_arg can fail, so test that value instead.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)

This one was fixed already by the patch below, that is in my perf/core
branch:

commit 9b9af6013045d58e95ecce62a1cfb34b150b7a8d
Author: Sasha Levin <sasha.levin@oracle.com>
Date:   Fri Dec 21 15:00:58 2012 -0500

    tools lib traceevent: test correct variable after allocation
    
    we've tested the wrong variable for allocation failure, fix it to
    test the right one.

Re: [PATCH 11/15] tools/lib/traceevent/event-parse.c: adjust duplicate test

[Arnaldo Carvalho de Melo]

Em Wed, Jan 23, 2013 at 09:42:17AM -0500, Steven Rostedt escreveu:
> On Mon, 2013-01-21 at 14:02 +0100, Julia Lawall wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > 
> > Delete successive tests to the same location.  Field was already tested.
> > Alloc_arg can fail, so test that value instead.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)

This one was fixed already by the patch below, that is in my perf/core
branch:

commit 9b9af6013045d58e95ecce62a1cfb34b150b7a8d
Author: Sasha Levin <sasha.levin@oracle.com>
Date:   Fri Dec 21 15:00:58 2012 -0500

    tools lib traceevent: test correct variable after allocation
    
    we've tested the wrong variable for allocation failure, fix it to
    test the right one.

[PATCH 5/15] drivers/scsi/bnx2fc/bnx2fc_io.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/scsi/bnx2fc/bnx2fc_io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 8d4626c..8bea53d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -654,7 +654,7 @@ int bnx2fc_init_mp_req(struct bnx2fc_cmd *io_req)
 	mp_req->mp_resp_bd = dma_alloc_coherent(&hba->pcidev->dev, sz,
 						 &mp_req->mp_resp_bd_dma,
 						 GFP_ATOMIC);
-	if (!mp_req->mp_req_bd) {
+	if (!mp_req->mp_resp_bd) {
 		printk(KERN_ERR PFX "unable to alloc MP resp bd\n");
 		bnx2fc_free_mp_resc(io_req);
 		return FAILED;

[PATCH 5/15] drivers/scsi/bnx2fc/bnx2fc_io.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/scsi/bnx2fc/bnx2fc_io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 8d4626c..8bea53d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -654,7 +654,7 @@ int bnx2fc_init_mp_req(struct bnx2fc_cmd *io_req)
 	mp_req->mp_resp_bd = dma_alloc_coherent(&hba->pcidev->dev, sz,
 						 &mp_req->mp_resp_bd_dma,
 						 GFP_ATOMIC);
-	if (!mp_req->mp_req_bd) {
+	if (!mp_req->mp_resp_bd) {
 		printk(KERN_ERR PFX "unable to alloc MP resp bd\n");
 		bnx2fc_free_mp_resc(io_req);
 		return FAILED;

Re: [PATCH 5/15] drivers/scsi/bnx2fc/bnx2fc_io.c: adjust duplicate test

[Bhanu Prakash Gollapudi]

On 01/21/2013 05:02 AM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Delete successive tests to the same location.  The code tested the result
> of a previous allocation, that itself was already tested.  It is changed to
> test the result of the most recent allocation.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
>
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>      when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
>   drivers/scsi/bnx2fc/bnx2fc_io.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 8d4626c..8bea53d 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -654,7 +654,7 @@ int bnx2fc_init_mp_req(struct bnx2fc_cmd *io_req)
>   	mp_req->mp_resp_bd = dma_alloc_coherent(&hba->pcidev->dev, sz,
>   						 &mp_req->mp_resp_bd_dma,
>   						 GFP_ATOMIC);
> -	if (!mp_req->mp_req_bd) {
> +	if (!mp_req->mp_resp_bd) {
>   		printk(KERN_ERR PFX "unable to alloc MP resp bd\n");
>   		bnx2fc_free_mp_resc(io_req);
>   		return FAILED;
>
>
Acked-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>

Re: [PATCH 5/15] drivers/scsi/bnx2fc/bnx2fc_io.c: adjust duplicate test

[Bhanu Prakash Gollapudi]

On 01/21/2013 05:02 AM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Delete successive tests to the same location.  The code tested the result
> of a previous allocation, that itself was already tested.  It is changed to
> test the result of the most recent allocation.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
>
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>      when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
>   drivers/scsi/bnx2fc/bnx2fc_io.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 8d4626c..8bea53d 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -654,7 +654,7 @@ int bnx2fc_init_mp_req(struct bnx2fc_cmd *io_req)
>   	mp_req->mp_resp_bd = dma_alloc_coherent(&hba->pcidev->dev, sz,
>   						 &mp_req->mp_resp_bd_dma,
>   						 GFP_ATOMIC);
> -	if (!mp_req->mp_req_bd) {
> +	if (!mp_req->mp_resp_bd) {
>   		printk(KERN_ERR PFX "unable to alloc MP resp bd\n");
>   		bnx2fc_free_mp_resc(io_req);
>   		return FAILED;
>
>
Acked-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>

[PATCH 4/15] drivers/net/ethernet/sfc/ptp.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  rc was previously tested and
not subsequently updated.  efx_phc_adjtime can return an error code, so the
call is updated so that is tested instead.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Not tested.  I'm not familiar with this function, so I don't know whether it
is desirable to test its result.

 drivers/net/ethernet/sfc/ptp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 0767043..3f93624 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1439,7 +1439,7 @@ static int efx_phc_settime(struct ptp_clock_info *ptp,
 
 	delta = timespec_sub(*e_ts, time_now);
 
-	efx_phc_adjtime(ptp, timespec_to_ns(&delta));
+	rc = efx_phc_adjtime(ptp, timespec_to_ns(&delta));
 	if (rc != 0)
 		return rc;
 

[PATCH 4/15] drivers/net/ethernet/sfc/ptp.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  rc was previously tested and
not subsequently updated.  efx_phc_adjtime can return an error code, so the
call is updated so that is tested instead.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Not tested.  I'm not familiar with this function, so I don't know whether it
is desirable to test its result.

 drivers/net/ethernet/sfc/ptp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 0767043..3f93624 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1439,7 +1439,7 @@ static int efx_phc_settime(struct ptp_clock_info *ptp,
 
 	delta = timespec_sub(*e_ts, time_now);
 
-	efx_phc_adjtime(ptp, timespec_to_ns(&delta));
+	rc = efx_phc_adjtime(ptp, timespec_to_ns(&delta));
 	if (rc != 0)
 		return rc;
 

Re: [PATCH 4/15] drivers/net/ethernet/sfc/ptp.c: adjust duplicate test

[Ben Hutchings]

On Mon, 2013-01-21 at 14:02 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  rc was previously tested and
> not subsequently updated.  efx_phc_adjtime can return an error code, so the
> call is updated so that is tested instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Acked-by: Ben Hutchings <bhutchings@solarflare.com>

> ---
> Not tested.  I'm not familiar with this function, so I don't know whether it
> is desirable to test its result.

Yes, the result should be tested.  Thanks.

Ben.

>  drivers/net/ethernet/sfc/ptp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
> index 0767043..3f93624 100644
> --- a/drivers/net/ethernet/sfc/ptp.c
> +++ b/drivers/net/ethernet/sfc/ptp.c
> @@ -1439,7 +1439,7 @@ static int efx_phc_settime(struct ptp_clock_info *ptp,
>  
>  	delta = timespec_sub(*e_ts, time_now);
>  
> -	efx_phc_adjtime(ptp, timespec_to_ns(&delta));
> +	rc = efx_phc_adjtime(ptp, timespec_to_ns(&delta));
>  	if (rc != 0)
>  		return rc;
>  
> 

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 4/15] drivers/net/ethernet/sfc/ptp.c: adjust duplicate test

[Ben Hutchings]

On Mon, 2013-01-21 at 14:02 +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  rc was previously tested and
> not subsequently updated.  efx_phc_adjtime can return an error code, so the
> call is updated so that is tested instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Acked-by: Ben Hutchings <bhutchings@solarflare.com>

> ---
> Not tested.  I'm not familiar with this function, so I don't know whether it
> is desirable to test its result.

Yes, the result should be tested.  Thanks.

Ben.

>  drivers/net/ethernet/sfc/ptp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
> index 0767043..3f93624 100644
> --- a/drivers/net/ethernet/sfc/ptp.c
> +++ b/drivers/net/ethernet/sfc/ptp.c
> @@ -1439,7 +1439,7 @@ static int efx_phc_settime(struct ptp_clock_info *ptp,
>  
>  	delta = timespec_sub(*e_ts, time_now);
>  
> -	efx_phc_adjtime(ptp, timespec_to_ns(&delta));
> +	rc = efx_phc_adjtime(ptp, timespec_to_ns(&delta));
>  	if (rc != 0)
>  		return rc;
>  
> 

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 4/15] drivers/net/ethernet/sfc/ptp.c: adjust duplicate test

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Mon, 21 Jan 2013 14:02:48 +0100

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  rc was previously tested and
> not subsequently updated.  efx_phc_adjtime can return an error code, so the
> call is updated so that is tested instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied to net-next, thanks.

Re: [PATCH 4/15] drivers/net/ethernet/sfc/ptp.c: adjust duplicate test

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Mon, 21 Jan 2013 14:02:48 +0100

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  rc was previously tested and
> not subsequently updated.  efx_phc_adjtime can return an error code, so the
> call is updated so that is tested instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied to net-next, thanks.

[PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/crypto/atmel-aes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index c9d9d5c..6f22ba5 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -332,7 +332,7 @@ static int atmel_aes_crypt_cpu_start(struct atmel_aes_dev *dd)
 		return -EINVAL;
 
 	dd->nb_out_sg = atmel_aes_sg_length(dd->req, dd->out_sg);
-	if (!dd->nb_in_sg)
+	if (!dd->nb_out_sg)
 		return -EINVAL;
 
 	dd->bufcnt = sg_copy_to_buffer(dd->in_sg, dd->nb_in_sg,

[PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/crypto/atmel-aes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index c9d9d5c..6f22ba5 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -332,7 +332,7 @@ static int atmel_aes_crypt_cpu_start(struct atmel_aes_dev *dd)
 		return -EINVAL;
 
 	dd->nb_out_sg = atmel_aes_sg_length(dd->req, dd->out_sg);
-	if (!dd->nb_in_sg)
+	if (!dd->nb_out_sg)
 		return -EINVAL;
 
 	dd->bufcnt = sg_copy_to_buffer(dd->in_sg, dd->nb_in_sg,

[PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/crypto/atmel-aes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index c9d9d5c..6f22ba5 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -332,7 +332,7 @@ static int atmel_aes_crypt_cpu_start(struct atmel_aes_dev *dd)
 		return -EINVAL;
 
 	dd->nb_out_sg = atmel_aes_sg_length(dd->req, dd->out_sg);
-	if (!dd->nb_in_sg)
+	if (!dd->nb_out_sg)
 		return -EINVAL;
 
 	dd->bufcnt = sg_copy_to_buffer(dd->in_sg, dd->nb_in_sg,

Re: [PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Mon, 21 Jan 2013 14:02:51 +0100

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The code tested the result
> of a previous allocation, that itself was already tested.  It is changed to
> test the result of the most recent allocation.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Mon, 21 Jan 2013 14:02:51 +0100

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The code tested the result
> of a previous allocation, that itself was already tested.  It is changed to
> test the result of the most recent allocation.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[David Miller]

From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Mon, 21 Jan 2013 14:02:51 +0100

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The code tested the result
> of a previous allocation, that itself was already tested.  It is changed to
> test the result of the most recent allocation.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[Herbert Xu]

On Mon, Jan 21, 2013 at 01:19:12PM -0500, David Miller wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> Date: Mon, 21 Jan 2013 14:02:51 +0100
> 
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > 
> > Delete successive tests to the same location.  The code tested the result
> > of a previous allocation, that itself was already tested.  It is changed to
> > test the result of the most recent allocation.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
>  ...
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Patch applied.  Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[Herbert Xu]

On Mon, Jan 21, 2013 at 01:19:12PM -0500, David Miller wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> Date: Mon, 21 Jan 2013 14:02:51 +0100
> 
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > 
> > Delete successive tests to the same location.  The code tested the result
> > of a previous allocation, that itself was already tested.  It is changed to
> > test the result of the most recent allocation.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
>  ...
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Patch applied.  Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 7/15] drivers/crypto/atmel-aes.c: adjust duplicate test

[Herbert Xu]

On Mon, Jan 21, 2013 at 01:19:12PM -0500, David Miller wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> Date: Mon, 21 Jan 2013 14:02:51 +0100
> 
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > 
> > Delete successive tests to the same location.  The code tested the result
> > of a previous allocation, that itself was already tested.  It is changed to
> > test the result of the most recent allocation.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
>  ...
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Patch applied.  Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous call, that itself was already tested.  It is changed to test
the result of the most recent call.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 arch/powerpc/platforms/85xx/p1022_ds.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
index e346edf..af5853c 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -302,7 +302,7 @@ static void p1022ds_set_monitor_port(enum fsl_diu_monitor_port port)
 		goto exit;
 	}
 	cs1_addr = lbc_br_to_phys(ecm, num_laws, br1);
-	if (!cs0_addr) {
+	if (!cs1_addr) {
 		pr_err("p1022ds: could not determine physical address for CS1"
 		       " (BR1=%08x)\n", br1);
 		goto exit;

[PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous call, that itself was already tested.  It is changed to test
the result of the most recent call.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 arch/powerpc/platforms/85xx/p1022_ds.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
index e346edf..af5853c 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -302,7 +302,7 @@ static void p1022ds_set_monitor_port(enum fsl_diu_monitor_port port)
 		goto exit;
 	}
 	cs1_addr = lbc_br_to_phys(ecm, num_laws, br1);
-	if (!cs0_addr) {
+	if (!cs1_addr) {
 		pr_err("p1022ds: could not determine physical address for CS1"
 		       " (BR1=%08x)\n", br1);
 		goto exit;

[PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous call, that itself was already tested.  It is changed to test
the result of the most recent call.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 arch/powerpc/platforms/85xx/p1022_ds.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
index e346edf..af5853c 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -302,7 +302,7 @@ static void p1022ds_set_monitor_port(enum fsl_diu_monitor_port port)
 		goto exit;
 	}
 	cs1_addr = lbc_br_to_phys(ecm, num_laws, br1);
-	if (!cs0_addr) {
+	if (!cs1_addr) {
 		pr_err("p1022ds: could not determine physical address for CS1"
 		       " (BR1=%08x)\n", br1);
 		goto exit;

Re: [PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test

[Kumar Gala]

On Jan 21, 2013, at 7:02 AM, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The code tested the result
> of a previous call, that itself was already tested.  It is changed to test
> the result of the most recent call.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> arch/powerpc/platforms/85xx/p1022_ds.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

applied to next

- k

Re: [PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test

[Kumar Gala]

On Jan 21, 2013, at 7:02 AM, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The code tested the result
> of a previous call, that itself was already tested.  It is changed to test
> the result of the most recent call.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> arch/powerpc/platforms/85xx/p1022_ds.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

applied to next

- k

Re: [PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test

[Kumar Gala]

On Jan 21, 2013, at 7:02 AM, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The code tested the result
> of a previous call, that itself was already tested.  It is changed to test
> the result of the most recent call.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
>   return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> arch/powerpc/platforms/85xx/p1022_ds.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

applied to next

- k

[PATCH 12/15] fs/hostfs/hostfs_kern.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  This looks like simple code
duplication.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 fs/hostfs/hostfs_kern.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index 457addc..909fc21 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -727,8 +727,6 @@ static int hostfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode,
 	__putname(name);
 	if (err)
 		goto out_put;
-	if (err)
-		goto out_put;
 
 	d_instantiate(dentry, inode);
 	return 0;

[PATCH 12/15] fs/hostfs/hostfs_kern.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  This looks like simple code
duplication.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 fs/hostfs/hostfs_kern.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index 457addc..909fc21 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -727,8 +727,6 @@ static int hostfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode,
 	__putname(name);
 	if (err)
 		goto out_put;
-	if (err)
-		goto out_put;
 
 	d_instantiate(dentry, inode);
 	return 0;

[PATCH 10/15] drivers/scsi/csiostor/csio_lnode.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The current value of ln has
been tested already, and is clearly not NULL, given the enclosing if
condition.  Furthermore, the result of csio_lnode_alloc is currently
ignored.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Not tested.  I don't know if this is the right fix.

 drivers/scsi/csiostor/csio_lnode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/csiostor/csio_lnode.c b/drivers/scsi/csiostor/csio_lnode.c
index ffe9be0..d956f97 100644
--- a/drivers/scsi/csiostor/csio_lnode.c
+++ b/drivers/scsi/csiostor/csio_lnode.c
@@ -873,7 +873,7 @@ csio_handle_link_up(struct csio_hw *hw, uint8_t portid, uint32_t fcfi,
 		if (ln->vnp_flowid != CSIO_INVALID_IDX) {
 			/* New VN-Port */
 			spin_unlock_irq(&hw->lock);
-			csio_lnode_alloc(hw);
+			ln = csio_lnode_alloc(hw);
 			spin_lock_irq(&hw->lock);
 			if (!ln) {
 				csio_err(hw,

[PATCH 10/15] drivers/scsi/csiostor/csio_lnode.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The current value of ln has
been tested already, and is clearly not NULL, given the enclosing if
condition.  Furthermore, the result of csio_lnode_alloc is currently
ignored.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Not tested.  I don't know if this is the right fix.

 drivers/scsi/csiostor/csio_lnode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/csiostor/csio_lnode.c b/drivers/scsi/csiostor/csio_lnode.c
index ffe9be0..d956f97 100644
--- a/drivers/scsi/csiostor/csio_lnode.c
+++ b/drivers/scsi/csiostor/csio_lnode.c
@@ -873,7 +873,7 @@ csio_handle_link_up(struct csio_hw *hw, uint8_t portid, uint32_t fcfi,
 		if (ln->vnp_flowid != CSIO_INVALID_IDX) {
 			/* New VN-Port */
 			spin_unlock_irq(&hw->lock);
-			csio_lnode_alloc(hw);
+			ln = csio_lnode_alloc(hw);
 			spin_lock_irq(&hw->lock);
 			if (!ln) {
 				csio_err(hw,

Re: [PATCH 10/15] drivers/scsi/csiostor/csio_lnode.c: adjust duplicate test

[Naresh Kumar Inna]

On 1/21/2013 6:32 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The current value of ln has
> been tested already, and is clearly not NULL, given the enclosing if
> condition.  Furthermore, the result of csio_lnode_alloc is currently
> ignored.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Not tested.  I don't know if this is the right fix.
> 
>  drivers/scsi/csiostor/csio_lnode.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/csiostor/csio_lnode.c b/drivers/scsi/csiostor/csio_lnode.c
> index ffe9be0..d956f97 100644
> --- a/drivers/scsi/csiostor/csio_lnode.c
> +++ b/drivers/scsi/csiostor/csio_lnode.c
> @@ -873,7 +873,7 @@ csio_handle_link_up(struct csio_hw *hw, uint8_t portid, uint32_t fcfi,
>  		if (ln->vnp_flowid != CSIO_INVALID_IDX) {
>  			/* New VN-Port */
>  			spin_unlock_irq(&hw->lock);
> -			csio_lnode_alloc(hw);
> +			ln = csio_lnode_alloc(hw);
>  			spin_lock_irq(&hw->lock);
>  			if (!ln) {
>  				csio_err(hw,
> --

Acked-by: Naresh Kumar Inna <naresh@chelsio.com>

Thanks,
Naresh.

Re: [PATCH 10/15] drivers/scsi/csiostor/csio_lnode.c: adjust duplicate test

[Naresh Kumar Inna]

On 1/21/2013 6:32 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  The current value of ln has
> been tested already, and is clearly not NULL, given the enclosing if
> condition.  Furthermore, the result of csio_lnode_alloc is currently
> ignored.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Not tested.  I don't know if this is the right fix.
> 
>  drivers/scsi/csiostor/csio_lnode.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/csiostor/csio_lnode.c b/drivers/scsi/csiostor/csio_lnode.c
> index ffe9be0..d956f97 100644
> --- a/drivers/scsi/csiostor/csio_lnode.c
> +++ b/drivers/scsi/csiostor/csio_lnode.c
> @@ -873,7 +873,7 @@ csio_handle_link_up(struct csio_hw *hw, uint8_t portid, uint32_t fcfi,
>  		if (ln->vnp_flowid != CSIO_INVALID_IDX) {
>  			/* New VN-Port */
>  			spin_unlock_irq(&hw->lock);
> -			csio_lnode_alloc(hw);
> +			ln = csio_lnode_alloc(hw);
>  			spin_lock_irq(&hw->lock);
>  			if (!ln) {
>  				csio_err(hw,
> --

Acked-by: Naresh Kumar Inna <naresh@chelsio.com>

Thanks,
Naresh.

[PATCH 9/14] drivers/ipack/devices/ipoctal.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/ipack/devices/ipoctal.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index 8fb2e34..f3a83b1 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -282,7 +282,7 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 	ipoctal->mem8_space  		devm_ioremap_nocache(&ipoctal->dev->dev,
 				     region->start, 0x8000);
-	if (!addr) {
+	if (!ipoctal->mem8_space) {
 		dev_err(&ipoctal->dev->dev,
 			"Unable to map slot [%d:%d] MEM8 space!\n",
 			bus_nr, slot);

[PATCH 9/14] drivers/ipack/devices/ipoctal.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/ipack/devices/ipoctal.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index 8fb2e34..f3a83b1 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -282,7 +282,7 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 	ipoctal->mem8_space =
 		devm_ioremap_nocache(&ipoctal->dev->dev,
 				     region->start, 0x8000);
-	if (!addr) {
+	if (!ipoctal->mem8_space) {
 		dev_err(&ipoctal->dev->dev,
 			"Unable to map slot [%d:%d] MEM8 space!\n",
 			bus_nr, slot);

[PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  Data is the just previously
allocated and tested value.  Test the result of the allocation made here
instead.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
The previous code could be further simplified by removing the
initializations of ret to -ENOMEM, as is absent in this case.  But that is
probably for another patch.

 drivers/usb/musb/omap2430.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index da00af4..533c4fd 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -545,7 +545,7 @@ static int omap2430_probe(struct platform_device *pdev)
 		}
 
 		config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
-		if (!data) {
+		if (!config) {
 			dev_err(&pdev->dev,
 				"failed to allocate musb hdrc config\n");
 			goto err2;

[PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  Data is the just previously
allocated and tested value.  Test the result of the allocation made here
instead.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
The previous code could be further simplified by removing the
initializations of ret to -ENOMEM, as is absent in this case.  But that is
probably for another patch.

 drivers/usb/musb/omap2430.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index da00af4..533c4fd 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -545,7 +545,7 @@ static int omap2430_probe(struct platform_device *pdev)
 		}
 
 		config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
-		if (!data) {
+		if (!config) {
 			dev_err(&pdev->dev,
 				"failed to allocate musb hdrc config\n");
 			goto err2;

Re: [PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

Hi,

On Mon, Jan 21, 2013 at 02:02:57PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  Data is the just previously
> allocated and tested value.  Test the result of the allocation made here
> instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Already in my tree ;-)

commit b37457d80bc3e2a6bb86a6036c572574614a7631
Author: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date:   Tue Jan 8 22:11:14 2013 +0300

    usb: musb: omap2430: fix wrong devm_kzalloc() result check
    
    Commit 00a0b1d58af873d842580dcac55f3b156c3a4077 (usb: musb: omap: Add device
    tree support for omap musb glue) assigns result of devm_kzalloc() call to
    the 'config' variable but then checks for NULL the 'data' variable (already
    checked after previous call). Thus we risk a kernel oops further when data
    pointed by 'config' is written to by subsequent of_property_read_u32() calls
    iff the allocation happens to fail...
    
    Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
    Signed-off-by: Felipe Balbi <balbi@ti.com>

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index b15bb05..bb48796 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -543,7 +543,7 @@ static int omap2430_probe(struct platform_device *pdev)
 		}
 
 		config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
-		if (!data) {
+		if (!config) {
 			dev_err(&pdev->dev,
 				"failed to allocate musb hdrc config\n");
 			goto err2;

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

Hi,

On Mon, Jan 21, 2013 at 02:02:57PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  Data is the just previously
> allocated and tested value.  Test the result of the allocation made here
> instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Already in my tree ;-)

commit b37457d80bc3e2a6bb86a6036c572574614a7631
Author: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date:   Tue Jan 8 22:11:14 2013 +0300

    usb: musb: omap2430: fix wrong devm_kzalloc() result check
    
    Commit 00a0b1d58af873d842580dcac55f3b156c3a4077 (usb: musb: omap: Add device
    tree support for omap musb glue) assigns result of devm_kzalloc() call to
    the 'config' variable but then checks for NULL the 'data' variable (already
    checked after previous call). Thus we risk a kernel oops further when data
    pointed by 'config' is written to by subsequent of_property_read_u32() calls
    iff the allocation happens to fail...
    
    Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
    Signed-off-by: Felipe Balbi <balbi@ti.com>

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index b15bb05..bb48796 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -543,7 +543,7 @@ static int omap2430_probe(struct platform_device *pdev)
 		}
 
 		config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
-		if (!data) {
+		if (!config) {
 			dev_err(&pdev->dev,
 				"failed to allocate musb hdrc config\n");
 			goto err2;

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

Hi,

On Mon, Jan 21, 2013 at 02:02:57PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  Data is the just previously
> allocated and tested value.  Test the result of the allocation made here
> instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Already in my tree ;-)

commit b37457d80bc3e2a6bb86a6036c572574614a7631
Author: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Date:   Tue Jan 8 22:11:14 2013 +0300

    usb: musb: omap2430: fix wrong devm_kzalloc() result check
    
    Commit 00a0b1d58af873d842580dcac55f3b156c3a4077 (usb: musb: omap: Add device
    tree support for omap musb glue) assigns result of devm_kzalloc() call to
    the 'config' variable but then checks for NULL the 'data' variable (already
    checked after previous call). Thus we risk a kernel oops further when data
    pointed by 'config' is written to by subsequent of_property_read_u32() calls
    iff the allocation happens to fail...
    
    Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
    Signed-off-by: Felipe Balbi <balbi@ti.com>

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index b15bb05..bb48796 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -543,7 +543,7 @@ static int omap2430_probe(struct platform_device *pdev)
 		}
 
 		config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
-		if (!data) {
+		if (!config) {
 			dev_err(&pdev->dev,
 				"failed to allocate musb hdrc config\n");
 			goto err2;

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

[Sergei Shtylyov]

On 21-01-2013 17:02, Julia Lawall wrote:

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

> Delete successive tests to the same location.  Data is the just previously
> allocated and tested value.  Test the result of the allocation made here
> instead.

> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)

> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@

> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>      when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> // </smpl>

> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

> ---
> The previous code could be further simplified by removing the
> initializations of ret to -ENOMEM, as is absent in this case.  But that is
> probably for another patch.

    I've already sent such patch and Felipe has queued it.

>   drivers/usb/musb/omap2430.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index da00af4..533c4fd 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -545,7 +545,7 @@ static int omap2430_probe(struct platform_device *pdev)
>   		}
>
>   		config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
> -		if (!data) {
> +		if (!config) {

    Same about this change. You're a bit late this time. :-)

WBR, Sergei

Re: [PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

[Sergei Shtylyov]

On 21-01-2013 17:02, Julia Lawall wrote:

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

> Delete successive tests to the same location.  Data is the just previously
> allocated and tested value.  Test the result of the allocation made here
> instead.

> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)

> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@

> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>      when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>   { ... when forall
>     return ...; }
> // </smpl>

> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

> ---
> The previous code could be further simplified by removing the
> initializations of ret to -ENOMEM, as is absent in this case.  But that is
> probably for another patch.

    I've already sent such patch and Felipe has queued it.

>   drivers/usb/musb/omap2430.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index da00af4..533c4fd 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -545,7 +545,7 @@ static int omap2430_probe(struct platform_device *pdev)
>   		}
>
>   		config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
> -		if (!data) {
> +		if (!config) {

    Same about this change. You're a bit late this time. :-)

WBR, Sergei

[PATCH 14/15] drivers/infiniband/hw/mlx4/sysfs.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/infiniband/hw/mlx4/sysfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx4/sysfs.c b/drivers/infiniband/hw/mlx4/sysfs.c
index 5b2a01d..97516eb 100644
--- a/drivers/infiniband/hw/mlx4/sysfs.c
+++ b/drivers/infiniband/hw/mlx4/sysfs.c
@@ -732,7 +732,7 @@ int mlx4_ib_device_register_sysfs(struct mlx4_ib_dev *dev)
 	dev->ports_parent  		kobject_create_and_add("ports",
 				       kobject_get(dev->iov_parent));
-	if (!dev->iov_parent) {
+	if (!dev->ports_parent) {
 		ret = -ENOMEM;
 		goto err_ports;
 	}

[PATCH 14/15] drivers/infiniband/hw/mlx4/sysfs.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  The code tested the result
of a previous allocation, that itself was already tested.  It is changed to
test the result of the most recent allocation.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/infiniband/hw/mlx4/sysfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx4/sysfs.c b/drivers/infiniband/hw/mlx4/sysfs.c
index 5b2a01d..97516eb 100644
--- a/drivers/infiniband/hw/mlx4/sysfs.c
+++ b/drivers/infiniband/hw/mlx4/sysfs.c
@@ -732,7 +732,7 @@ int mlx4_ib_device_register_sysfs(struct mlx4_ib_dev *dev)
 	dev->ports_parent =
 		kobject_create_and_add("ports",
 				       kobject_get(dev->iov_parent));
-	if (!dev->iov_parent) {
+	if (!dev->ports_parent) {
 		ret = -ENOMEM;
 		goto err_ports;
 	}

[PATCH 6/15] drivers/net/wireless/iwlegacy/4965-mac.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  This looks like simple code
duplication.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/net/wireless/iwlegacy/4965-mac.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index 10fc249..0195190 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -6572,9 +6572,6 @@ il4965_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto out_free_eeprom;
 
-	if (err)
-		goto out_free_eeprom;
-
 	/* extract MAC Address */
 	il4965_eeprom_get_mac(il, il->addresses[0].addr);
 	D_INFO("MAC address: %pM\n", il->addresses[0].addr);

[PATCH 6/15] drivers/net/wireless/iwlegacy/4965-mac.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  This looks like simple code
duplication.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/net/wireless/iwlegacy/4965-mac.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index 10fc249..0195190 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -6572,9 +6572,6 @@ il4965_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto out_free_eeprom;
 
-	if (err)
-		goto out_free_eeprom;
-
 	/* extract MAC Address */
 	il4965_eeprom_get_mac(il, il->addresses[0].addr);
 	D_INFO("MAC address: %pM\n", il->addresses[0].addr);

Re: [PATCH 6/15] drivers/net/wireless/iwlegacy/4965-mac.c: adjust duplicate test

[Stanislaw Gruszka]

On Mon, Jan 21, 2013 at 02:02:50PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  This looks like simple code
> duplication.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

ACK

Re: [PATCH 6/15] drivers/net/wireless/iwlegacy/4965-mac.c: adjust duplicate test

[Stanislaw Gruszka]

On Mon, Jan 21, 2013 at 02:02:50PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  This looks like simple code
> duplication.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

ACK

[PATCH 2/15] drivers/usb/chipidea/core.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  In this case res has already
been tested for being NULL, and calling devm_request_and_ioremap will not
make it NULL.  On the other hand, devm_request_and_ioremap can return NULL
on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
This code could be improved in other ways.  The previous NULL test on res
is not needed, and the dev_err in the error-handling code for the call to
devm_request_and_ioremap is redundant with respect to the logging already
done by devm_request_and_ioremap.  But these should probably be done
separately.

 drivers/usb/chipidea/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index aebf695..57cae1f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -411,7 +411,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	}
 
 	base = devm_request_and_ioremap(dev, res);
-	if (!res) {
+	if (!base) {
 		dev_err(dev, "can't request and ioremap resource\n");
 		return -ENOMEM;
 	}

[PATCH 2/15] drivers/usb/chipidea/core.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  In this case res has already
been tested for being NULL, and calling devm_request_and_ioremap will not
make it NULL.  On the other hand, devm_request_and_ioremap can return NULL
on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
This code could be improved in other ways.  The previous NULL test on res
is not needed, and the dev_err in the error-handling code for the call to
devm_request_and_ioremap is redundant with respect to the logging already
done by devm_request_and_ioremap.  But these should probably be done
separately.

 drivers/usb/chipidea/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index aebf695..57cae1f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -411,7 +411,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	}
 
 	base = devm_request_and_ioremap(dev, res);
-	if (!res) {
+	if (!base) {
 		dev_err(dev, "can't request and ioremap resource\n");
 		return -ENOMEM;
 	}

Re: [PATCH 2/15] drivers/usb/chipidea/core.c: adjust duplicate test

[Peter Chen]

On Mon, Jan 21, 2013 at 02:02:46PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  In this case res has already
> been tested for being NULL, and calling devm_request_and_ioremap will not
> make it NULL.  On the other hand, devm_request_and_ioremap can return NULL
> on failure.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> This code could be improved in other ways.  The previous NULL test on res
> is not needed, and the dev_err in the error-handling code for the call to
> devm_request_and_ioremap is redundant with respect to the logging already
> done by devm_request_and_ioremap.  But these should probably be done
> separately.
> 
>  drivers/usb/chipidea/core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index aebf695..57cae1f 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -411,7 +411,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	}
>  
>  	base = devm_request_and_ioremap(dev, res);
> -	if (!res) {
> +	if (!base) {
>  		dev_err(dev, "can't request and ioremap resource\n");
>  		return -ENOMEM;
>  	}
> 
Tested-and-reviewed-by: Peter Chen <peter.chen@freescale.com>

-- 

Best Regards,
Peter Chen

Re: [PATCH 2/15] drivers/usb/chipidea/core.c: adjust duplicate test

[Peter Chen]

On Mon, Jan 21, 2013 at 02:02:46PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  In this case res has already
> been tested for being NULL, and calling devm_request_and_ioremap will not
> make it NULL.  On the other hand, devm_request_and_ioremap can return NULL
> on failure.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> This code could be improved in other ways.  The previous NULL test on res
> is not needed, and the dev_err in the error-handling code for the call to
> devm_request_and_ioremap is redundant with respect to the logging already
> done by devm_request_and_ioremap.  But these should probably be done
> separately.
> 
>  drivers/usb/chipidea/core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index aebf695..57cae1f 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -411,7 +411,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	}
>  
>  	base = devm_request_and_ioremap(dev, res);
> -	if (!res) {
> +	if (!base) {
>  		dev_err(dev, "can't request and ioremap resource\n");
>  		return -ENOMEM;
>  	}
> 
Tested-and-reviewed-by: Peter Chen <peter.chen@freescale.com>

-- 

Best Regards,
Peter Chen

[PATCH 3/15] drivers/staging/omap-thermal/omap-bandgap.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  There was a previous test on
ret and it has not been updated since then.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Perhaps it was intended to test somthing, but I am not sure what.

 drivers/staging/omap-thermal/omap-bandgap.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c
index 8346e34..89f7180 100644
--- a/drivers/staging/omap-thermal/omap-bandgap.c
+++ b/drivers/staging/omap-thermal/omap-bandgap.c
@@ -568,8 +568,6 @@ int omap_bandgap_read_update_interval(struct omap_bandgap *bg_ptr, int id,
 
 	tsr = bg_ptr->conf->sensors[id].registers;
 	time = omap_bandgap_readl(bg_ptr, tsr->bgap_counter);
-	if (ret)
-		return ret;
 	time = (time & tsr->counter_mask) >> __ffs(tsr->counter_mask);
 	time = time * 1000 / bg_ptr->clk_rate;
 

[PATCH 3/15] drivers/staging/omap-thermal/omap-bandgap.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  There was a previous test on
ret and it has not been updated since then.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Perhaps it was intended to test somthing, but I am not sure what.

 drivers/staging/omap-thermal/omap-bandgap.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c
index 8346e34..89f7180 100644
--- a/drivers/staging/omap-thermal/omap-bandgap.c
+++ b/drivers/staging/omap-thermal/omap-bandgap.c
@@ -568,8 +568,6 @@ int omap_bandgap_read_update_interval(struct omap_bandgap *bg_ptr, int id,
 
 	tsr = bg_ptr->conf->sensors[id].registers;
 	time = omap_bandgap_readl(bg_ptr, tsr->bgap_counter);
-	if (ret)
-		return ret;
 	time = (time & tsr->counter_mask) >> __ffs(tsr->counter_mask);
 	time = time * 1000 / bg_ptr->clk_rate;
 

Re: [PATCH 3/15] drivers/staging/omap-thermal/omap-bandgap.c: adjust duplicate test

[walter harms]

Am 21.01.2013 14:02, schrieb Julia Lawall:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete successive tests to the same location.  There was a previous test on
> ret and it has not been updated since then.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
> 
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
>     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
>  { ... when forall
>    return ...; }
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Perhaps it was intended to test somthing, but I am not sure what.
> 
>  drivers/staging/omap-thermal/omap-bandgap.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c
> index 8346e34..89f7180 100644
> --- a/drivers/staging/omap-thermal/omap-bandgap.c
> +++ b/drivers/staging/omap-thermal/omap-bandgap.c
> @@ -568,8 +568,6 @@ int omap_bandgap_read_update_interval(struct omap_bandgap *bg_ptr, int id,
>  
>  	tsr = bg_ptr->conf->sensors[id].registers;
>  	time = omap_bandgap_readl(bg_ptr, tsr->bgap_counter);
> -	if (ret)
> -		return ret;
>  	time = (time & tsr->counter_mask) >> __ffs(tsr->counter_mask);
>  	time = time * 1000 / bg_ptr->clk_rate;
>  

since * and / have the same priority
  (time*1000)/bg_ptr->clk_rate
may be better

just to serve my paranoia
re,
 wh
> --
> 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 3/15] drivers/staging/omap-thermal/omap-bandgap.c: adjust duplicate test

[Dan Carpenter]

On Mon, Jan 21, 2013 at 01:24:18PM +0100, walter harms wrote:
> 
> 
> Am 21.01.2013 14:02, schrieb Julia Lawall:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > 
> > Delete successive tests to the same location.  There was a previous test on
> > ret and it has not been updated since then.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @s exists@
> > local idexpression y;
> > expression x,e;
> > @@
> > 
> > *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
> >  { ... when forall
> >    return ...; }
> > ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
> >     when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> > *if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
> >  { ... when forall
> >    return ...; }
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> > 
> > ---
> > Perhaps it was intended to test somthing, but I am not sure what.
> > 
> >  drivers/staging/omap-thermal/omap-bandgap.c |    2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c
> > index 8346e34..89f7180 100644
> > --- a/drivers/staging/omap-thermal/omap-bandgap.c
> > +++ b/drivers/staging/omap-thermal/omap-bandgap.c
> > @@ -568,8 +568,6 @@ int omap_bandgap_read_update_interval(struct omap_bandgap *bg_ptr, int id,
> >  
> >  	tsr = bg_ptr->conf->sensors[id].registers;
> >  	time = omap_bandgap_readl(bg_ptr, tsr->bgap_counter);
> > -	if (ret)
> > -		return ret;
> >  	time = (time & tsr->counter_mask) >> __ffs(tsr->counter_mask);
> >  	time = time * 1000 / bg_ptr->clk_rate;
> >  
> 
> since * and / have the same priority
>   (time*1000)/bg_ptr->clk_rate
> may be better
> 
> just to serve my paranoia

That's an unrelated change so it would have to go in a different
patch.

I think adding extra parenthesis is the wrong idea.  No one is ever
confused by '*' vs '/' precedence.

Mathematically, it doesn't matter which order you do it in.  In C,
the order matters because you care about round errors and integer
overflows.  If you put parenthesis around the multiply you're saying
that you care about rounding errors and not integer overflows.  If
you put it around the divide then it means the opposite.

In this case the multiply happens first obviously, but it's not
explicitly called out with extra parenthesis.

regards,
dan carpenter

Re: [PATCH 3/15] drivers/staging/omap-thermal/omap-bandgap.c: adjust duplicate test

[Julia Lawall]

> >  	tsr = bg_ptr->conf->sensors[id].registers;
> >  	time = omap_bandgap_readl(bg_ptr, tsr->bgap_counter);
> > -	if (ret)
> > -		return ret;
> >  	time = (time & tsr->counter_mask) >> __ffs(tsr->counter_mask);
> >  	time = time * 1000 / bg_ptr->clk_rate;
> >
>
> since * and / have the same priority
>   (time*1000)/bg_ptr->clk_rate
> may be better
>
> just to serve my paranoia

OK, I can do that, but maybe not in the same patch?

julia

[PATCH 1/15] drivers/sh/intc/of_intc.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  In this case intc_node has
already been tested for being NULL, and calling of_find_node_by_name will
not make it NULL.  On the other hand, of_find_node_by_name can return NULL
on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x = NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/sh/intc/of_intc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/sh/intc/of_intc.c b/drivers/sh/intc/of_intc.c
index 1d5f47e..8dfded3 100644
--- a/drivers/sh/intc/of_intc.c
+++ b/drivers/sh/intc/of_intc.c
@@ -330,7 +330,7 @@ static int __init of_sh_intc_get_prio(struct device_node *np,
 
 		reg_node = of_find_node_by_name(intc_node, node_name);
 
-		if (!intc_node) {
+		if (!reg_node) {
 			pr_err("INTC prio register not found\n");
 			ret = -EINVAL;
 			goto error;
@@ -386,7 +386,7 @@ static int __init of_sh_intc_get_sense(struct device_node *np,
 
 		reg_node = of_find_node_by_name(intc_node, node_name);
 
-		if (!intc_node) {
+		if (!reg_node) {
 			pr_err("INTC senses register not found\n");
 			ret = -EINVAL;
 			goto error;

[PATCH 1/15] drivers/sh/intc/of_intc.c: adjust duplicate test

[Julia Lawall]

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

Delete successive tests to the same location.  In this case intc_node has
already been tested for being NULL, and calling of_find_node_by_name will
not make it NULL.  On the other hand, of_find_node_by_name can return NULL
on failure.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
    when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/sh/intc/of_intc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/sh/intc/of_intc.c b/drivers/sh/intc/of_intc.c
index 1d5f47e..8dfded3 100644
--- a/drivers/sh/intc/of_intc.c
+++ b/drivers/sh/intc/of_intc.c
@@ -330,7 +330,7 @@ static int __init of_sh_intc_get_prio(struct device_node *np,
 
 		reg_node = of_find_node_by_name(intc_node, node_name);
 
-		if (!intc_node) {
+		if (!reg_node) {
 			pr_err("INTC prio register not found\n");
 			ret = -EINVAL;
 			goto error;
@@ -386,7 +386,7 @@ static int __init of_sh_intc_get_sense(struct device_node *np,
 
 		reg_node = of_find_node_by_name(intc_node, node_name);
 
-		if (!intc_node) {
+		if (!reg_node) {
 			pr_err("INTC senses register not found\n");
 			ret = -EINVAL;
 			goto error;