[OPW,kernel,v2,2/7] Staging: lustre: Fix no use of assignment in if condition
diff mbox

Message ID 02d3790632abf2967b783d656646db09659e8bd2.1382784520.git.rashika.kheria@gmail.com
State Accepted
Headers show

Commit Message

Rashika Oct. 26, 2013, 10:50 a.m. UTC
This patch fixes the following checkpatch.pl warning in lustre/ldlm/interval_tree.c-
ERROR: do not use assignment in if condition

Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
---

This revision fixes the following issues of the previous revision-
Incorrect file name, Remove extra braces

 drivers/staging/lustre/lustre/ldlm/interval_tree.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Waskiewicz Jr, Peter P Oct. 26, 2013, 8:23 p.m. UTC | #1
On Sat, 2013-10-26 at 16:20 +0530, Rashika Kheria wrote:
> This patch fixes the following checkpatch.pl warning in lustre/ldlm/interval_tree.c-
> ERROR: do not use assignment in if condition
> 
> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>

Reviewed-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>

> ---
> 
> This revision fixes the following issues of the previous revision-
> Incorrect file name, Remove extra braces
> 
>  drivers/staging/lustre/lustre/ldlm/interval_tree.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/interval_tree.c b/drivers/staging/lustre/lustre/ldlm/interval_tree.c
> index ced3355..7c02800 100644
> --- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c
> +++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c
> @@ -427,7 +427,7 @@ static void interval_erase_color(struct interval_node *node,
>  			} else {
>  				if (node_is_black_or_0(tmp->in_right)) {
>  					struct interval_node *o_left;
> -					if ((o_left = tmp->in_left))
> +					if (o_left == tmp->in_left)
>  						o_left->in_color = INTERVAL_BLACK;
>  					tmp->in_color = INTERVAL_RED;
>  					__rotate_right(tmp, root);
> @@ -457,7 +457,7 @@ static void interval_erase_color(struct interval_node *node,
>  			} else {
>  				if (node_is_black_or_0(tmp->in_left)) {
>  					struct interval_node *o_right;
> -					if ((o_right = tmp->in_right))
> +					if (o_right == tmp->in_right)
>  						o_right->in_color = INTERVAL_BLACK;
>  					tmp->in_color = INTERVAL_RED;
>  					__rotate_left(tmp, root);
> -- 
> 1.7.9.5
>
Greg Kroah-Hartman Oct. 27, 2013, 3:39 a.m. UTC | #2
On Sat, Oct 26, 2013 at 04:20:41PM +0530, Rashika Kheria wrote:
> This patch fixes the following checkpatch.pl warning in lustre/ldlm/interval_tree.c-
> ERROR: do not use assignment in if condition
> 
> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
> Reviewed-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> ---
> 
> This revision fixes the following issues of the previous revision-
> Incorrect file name, Remove extra braces
> 
>  drivers/staging/lustre/lustre/ldlm/interval_tree.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/interval_tree.c b/drivers/staging/lustre/lustre/ldlm/interval_tree.c
> index ced3355..7c02800 100644
> --- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c
> +++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c
> @@ -427,7 +427,7 @@ static void interval_erase_color(struct interval_node *node,
>  			} else {
>  				if (node_is_black_or_0(tmp->in_right)) {
>  					struct interval_node *o_left;
> -					if ((o_left = tmp->in_left))
> +					if (o_left == tmp->in_left)

Are you sure about this?  You just changed the logic of the code in a
large way.

>  						o_left->in_color = INTERVAL_BLACK;
>  					tmp->in_color = INTERVAL_RED;
>  					__rotate_right(tmp, root);
> @@ -457,7 +457,7 @@ static void interval_erase_color(struct interval_node *node,
>  			} else {
>  				if (node_is_black_or_0(tmp->in_left)) {
>  					struct interval_node *o_right;
> -					if ((o_right = tmp->in_right))
> +					if (o_right == tmp->in_right)

Same here, this isn't a code cleanup, you just changed the logic, did
you verify it is still correct?

PJ, watch out for this when you review stuff :)

thanks,

greg k-h

Patch
diff mbox

diff --git a/drivers/staging/lustre/lustre/ldlm/interval_tree.c b/drivers/staging/lustre/lustre/ldlm/interval_tree.c
index ced3355..7c02800 100644
--- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c
+++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c
@@ -427,7 +427,7 @@  static void interval_erase_color(struct interval_node *node,
 			} else {
 				if (node_is_black_or_0(tmp->in_right)) {
 					struct interval_node *o_left;
-					if ((o_left = tmp->in_left))
+					if (o_left == tmp->in_left)
 						o_left->in_color = INTERVAL_BLACK;
 					tmp->in_color = INTERVAL_RED;
 					__rotate_right(tmp, root);
@@ -457,7 +457,7 @@  static void interval_erase_color(struct interval_node *node,
 			} else {
 				if (node_is_black_or_0(tmp->in_left)) {
 					struct interval_node *o_right;
-					if ((o_right = tmp->in_right))
+					if (o_right == tmp->in_right)
 						o_right->in_color = INTERVAL_BLACK;
 					tmp->in_color = INTERVAL_RED;
 					__rotate_left(tmp, root);