[PATCH 0/13] make return of 0 explicit

[PATCH 0/13] make return of 0 explicit

[Julia Lawall]

Sometimes a local variable is used as a return value in a case where the
return value is always 0.  The result is more apparent if this variable is
just replaced by 0.  This is done by the following semantic patch, although
some cleanups may be needed. (http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression ret;
identifier reti;
expression e;
position p;
@@

ret@reti = 0;
... when != \(ret = e\|ret &= e\|ret |= e\|ret += e\|ret -= e\|ret *= e\|ret ^= e\)
    when != \(++ret\|--ret\|ret++\|ret--\)
    when != &ret
return ret;@p

@bad1 exists@
expression e != 0;
local idexpression r.ret;
position r.p;
@@

 \(ret = e\|ret &= e\|ret |= e\|ret += e\|ret -= e\|ret *= e\|ret ^= e\|++ret\|--ret\|ret++\|ret--\|&ret\)
 ...
 return ret;@p

@bad2 exists@
identifier r.reti;
position r.p;
identifier f;
type T;
@@

f(...,T reti,...) {
 ...
 return reti;@p
}

@change depends on !bad1 && !bad2@
position r.p;
local idexpression r.ret;
identifier f;
@@

f(...) { <+...
return 
-ret
+0
;@p
...+> }

@rewrite@
local idexpression r.ret;
expression e;
position p;
identifier change.f;
@@

f(...) { <+...
 \(ret@p = e\|&ret@p\)
...+> }

@depends on change@
local idexpression r.ret;
position p != rewrite.p;
identifier change.f;
@@

f(...) { <+...
(
break;
|
ret = 0;
... when exists
ret@p
... when any
|
- ret = 0;
)
...+> }

@depends on change@
identifier r.reti;
type T;
constant C;
identifier change.f;
@@

f(...) { ... when any
-T reti = C;
 ... when != reti
     when strict
 }

@depends on change@
identifier r.reti;
type T;
identifier change.f;
@@

f(...) { ... when any
-T reti;
 ... when != reti
     when strict
 }
// </smpl>

The first four rules detect cases where only 0 reaches a return.  The
remaining rules clean up any variable initializations or declarations that
are made unnecessary by this change.

[PATCH 9/13] md: make return of 0 explicit

[Julia Lawall]

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

Delete unnecessary local variable whose value is always 0 and that hides
the fact that the result is always 0.

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

// <smpl>
@r exists@
local idexpression ret;
expression e;
position p;
@@

-ret = 0;
... when != ret = e
return 
- ret
+ 0
  ;
// </smpl>

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

---
Alternatively, was an error code intended in the MD_MAX_BADBLOCKS case?

 drivers/md/md.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index f477e4c..23b7fee 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8297,7 +8297,6 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
 	u64 *p;
 	int lo, hi;
 	sector_t target = s + sectors;
-	int rv = 0;
 
 	if (bb->shift > 0) {
 		/* When clearing we round the start up and the end down.
@@ -8339,10 +8338,8 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
 
 			if (a < s) {
 				/* we need to split this range */
-				if (bb->count >= MD_MAX_BADBLOCKS) {
-					rv = 0;
+				if (bb->count >= MD_MAX_BADBLOCKS)
 					goto out;
-				}
 				memmove(p+lo+1, p+lo, (bb->count - lo) * 8);
 				bb->count++;
 				p[lo] = BB_MAKE(a, s-a, ack);
@@ -8378,7 +8375,7 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
 	bb->changed = 1;
 out:
 	write_sequnlock_irq(&bb->lock);
-	return rv;
+	return 0;
 }
 
 int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,

Re: [PATCH 9/13] md: make return of 0 explicit

[NeilBrown]

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

On Mon, 19 May 2014 06:31:11 +0200 Julia Lawall <Julia.Lawall@lip6.fr> wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> Delete unnecessary local variable whose value is always 0 and that hides
> the fact that the result is always 0.
> 
> A simplified version of the semantic patch that fixes this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression ret;
> expression e;
> position p;
> @@
> 
> -ret = 0;
> ... when != ret = e
> return 
> - ret
> + 0
>   ;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> Alternatively, was an error code intended in the MD_MAX_BADBLOCKS case?

Yes, I did want an error code in that case  - thanks!
I'll make a patch.

NeilBrown


> 
>  drivers/md/md.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index f477e4c..23b7fee 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8297,7 +8297,6 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
>  	u64 *p;
>  	int lo, hi;
>  	sector_t target = s + sectors;
> -	int rv = 0;
>  
>  	if (bb->shift > 0) {
>  		/* When clearing we round the start up and the end down.
> @@ -8339,10 +8338,8 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
>  
>  			if (a < s) {
>  				/* we need to split this range */
> -				if (bb->count >= MD_MAX_BADBLOCKS) {
> -					rv = 0;
> +				if (bb->count >= MD_MAX_BADBLOCKS)
>  					goto out;
> -				}
>  				memmove(p+lo+1, p+lo, (bb->count - lo) * 8);
>  				bb->count++;
>  				p[lo] = BB_MAKE(a, s-a, ack);
> @@ -8378,7 +8375,7 @@ static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
>  	bb->changed = 1;
>  out:
>  	write_sequnlock_irq(&bb->lock);
> -	return rv;
> +	return 0;
>  }
>  
>  int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]