[PATCH] staging: lustre: libcfs: use swap() in cfs_hash_bd_order()

[PATCH] staging: lustre: libcfs: use swap() in cfs_hash_bd_order()

[Amitoj Kaur Chawla]

Use swap() function instead of using a temporary variable for swapping
two variables.

The Coccinelle semantic patch used to make this change is as follows:
//<smpl>
@@
type T;
T a,b,c;
@@
- a = b;
- b = c;
- c = a;
+ swap(b, c);
//<smpl>

Additionally cleaned up the if-else block.

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
 drivers/staging/lustre/lustre/libcfs/hash.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
index 5e2afe4..a645f50 100644
--- a/drivers/staging/lustre/lustre/libcfs/hash.c
+++ b/drivers/staging/lustre/lustre/libcfs/hash.c
@@ -800,16 +800,10 @@ cfs_hash_bd_order(struct cfs_hash_bd *bd1, struct cfs_hash_bd *bd2)
 	}
 
 	rc = cfs_hash_bd_compare(bd1, bd2);
-	if (rc == 0) {
+	if (!rc)
 		bd2->bd_bucket = NULL;
-
-	} else if (rc > 0) { /* swab bd1 and bd2 */
-		struct cfs_hash_bd tmp;
-
-		tmp = *bd2;
-		*bd2 = *bd1;
-		*bd1 = tmp;
-	}
+	else if (rc > 0)
+		swap(*bd1, *bd2); /* swab bd1 and bd2 */
 }
 
 void
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] staging: lustre: libcfs: use swap() in cfs_hash_bd_order()

[Julia Lawall]

On Tue, 23 Feb 2016, Amitoj Kaur Chawla wrote:

> Use swap() function instead of using a temporary variable for swapping
> two variables.
> 
> The Coccinelle semantic patch used to make this change is as follows:
> //<smpl>
> @@
> type T;
> T a,b,c;
> @@
> - a = b;
> - b = c;
> - c = a;
> + swap(b, c);
> //<smpl>
> 
> Additionally cleaned up the if-else block.
> 
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> ---
>  drivers/staging/lustre/lustre/libcfs/hash.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
> index 5e2afe4..a645f50 100644
> --- a/drivers/staging/lustre/lustre/libcfs/hash.c
> +++ b/drivers/staging/lustre/lustre/libcfs/hash.c
> @@ -800,16 +800,10 @@ cfs_hash_bd_order(struct cfs_hash_bd *bd1, struct cfs_hash_bd *bd2)
>  	}
>  
>  	rc = cfs_hash_bd_compare(bd1, bd2);
> -	if (rc == 0) {
> +	if (!rc)

This seems like a completely different issue than the use of swap.

julia

>  		bd2->bd_bucket = NULL;
> -
> -	} else if (rc > 0) { /* swab bd1 and bd2 */
> -		struct cfs_hash_bd tmp;
> -
> -		tmp = *bd2;
> -		*bd2 = *bd1;
> -		*bd1 = tmp;
> -	}
> +	else if (rc > 0)
> +		swap(*bd1, *bd2); /* swab bd1 and bd2 */
>  }
>  
>  void
> -- 
> 1.9.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160223133302.GA3944%40amitoj-Inspiron-3542.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH] staging: lustre: libcfs: use swap() in cfs_hash_bd_order()

[Amitoj Kaur Chawla]

On Tue, Feb 23, 2016 at 7:06 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>>
>>       rc = cfs_hash_bd_compare(bd1, bd2);
>> -     if (rc == 0) {
>> +     if (!rc)
>
> This seems like a completely different issue than the use of swap.
>
> julia
>
I thought it would be okay to clean up the entire if-else block in the
same patch and not just the else condition.

If you prefer me sticking to only the swap cleanup, I'll do that.

Amitoj

Re: [Outreachy kernel] [PATCH] staging: lustre: libcfs: use swap() in cfs_hash_bd_order()

[Julia Lawall]

On Tue, 23 Feb 2016, Amitoj Kaur Chawla wrote:

> On Tue, Feb 23, 2016 at 7:06 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >>
> >>       rc = cfs_hash_bd_compare(bd1, bd2);
> >> -     if (rc == 0) {
> >> +     if (!rc)
> >
> > This seems like a completely different issue than the use of swap.
> >
> > julia
> >
> I thought it would be okay to clean up the entire if-else block in the
> same patch and not just the else condition.
> 
> If you prefer me sticking to only the swap cleanup, I'll do that.

Yes, it would be better.  The swap has really nothing to do with the if.  
You can also make a series with the other change, to get it done.

julia

> 
> Amitoj
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CA%2B5yK5Ey6zJzef7jzbgsUk2TYYZ_%2BAdJ8Suk7L6PKYGepzwHRw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH] staging: lustre: libcfs: use swap() in cfs_hash_bd_order()

[Amitoj Kaur Chawla]

On Tue, Feb 23, 2016 at 7:24 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
> Yes, it would be better.  The swap has really nothing to do with the if.
> You can also make a series with the other change, to get it done.
>
> julia
>
Okay, will send a series.

Amitoj