Skip to contentSkip to conversation

Assign outside if

PATCH In review Srishti Sharma 3 participants 5 messages Sep 15, 2017

Subsystems (3)
Networking Network drivers Staging
lore
Share & tools
lore

Srishti Sharma

author
#0 lore raw

cover Assign outside if

This patch series intends to remove the assignment statements inside the if statement, and eliminates the cases of parentheses around the right hand side of assignment generated as a result of the same.

Srishti Sharma (2):
Staging: irda: Don't use assignment inside if statement
Staging: irda: Remove parentheses on the right of assignment

 drivers/staging/irda/drivers/irda-usb.c | 4 ++--
 drivers/staging/irda/drivers/mcs7780.c  | 9 ++++++---
 drivers/staging/irda/net/irqueue.c      | 3 ++-
 3 files changed, 10 insertions(+), 6 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/2 Staging: irda: Don't use assignment inside if statement

Write assignment statement outside of the if statement. Done using the following semantic patch by coccinelle.

@@
identifier E;
expression F;
statement S;
@@

-if((E = F))
+E = F;
+if(E)
S

Signed-off-by Srishti Sharma <[email protected]>
---
 drivers/staging/irda/drivers/irda-usb.c | 4 ++--
 drivers/staging/irda/drivers/mcs7780.c  | 9 ++++++---
 drivers/staging/irda/net/irqueue.c      | 3 ++-
 3 files changed, 10 insertions(+), 6 deletions(-)
drivers/staging/irda/drivers/irda-usb.c +2 -2
index 723e49b..82bfc05 100644
@@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
 	urb->transfer_flags = 0;
 
 	/* Irq disabled -> GFP_ATOMIC */
-	if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret)
 		net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
-	}
 }
 
 /*------------------------------------------------------------------*/
drivers/staging/irda/drivers/mcs7780.c +6 -3
index c3f0b25..2b674d5 100644
@@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	if (mcs->new_speed <= 115200) {
 		rval &= ~MCS_FIR;
 
-		if ((rst = (mcs->speed > 115200)))
+		rst = (mcs->speed > 115200);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
 
 	} else if (mcs->new_speed <= 1152000) {
 		rval &= ~MCS_FIR;
 
-		if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
+		rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
 
 	} else {
 		rval |= MCS_FIR;
 
-		if ((rst = (mcs->speed != 4000000)))
+		rst = (mcs->speed != 4000000);
+		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
 
 	}
drivers/staging/irda/net/irqueue.c +2 -1
index 160dc89..5aab072 100644
@@ -217,7 +217,8 @@ static __u32 hash( const char* name)
 
 	while(*name) {
 		h = (h<<4) + *name++;
-		if ((g = (h & 0xf0000000)))
+		g = (h & 0xf0000000);
+		if (g)
 			h ^=g>>24;
 		h &=~g;
 	}
Signature · 1 line
2.7.4
author
#2 lore raw

PATCH 2/2 Staging: irda: Remove parentheses on the right of assignment

In reply to Srishti Sharma

Parentheses are not needed on the right hand side of assignment statement in most cases. Done using the following semantic patch by coccinelle.

@@
identifier E,F,G,f;
expression e,r;
@@

(
E = (G == F);
|
E = (e == r);
|
E =
-(
...
-)
;
)

Signed-off-by Srishti Sharma <[email protected]>
---
 drivers/staging/irda/drivers/mcs7780.c | 4 ++--
 drivers/staging/irda/net/irqueue.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/staging/irda/drivers/mcs7780.c +2 -2
index 2b674d5..d52e9f4 100644
@@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	if (mcs->new_speed <= 115200) {
 		rval &= ~MCS_FIR;
 
-		rst = (mcs->speed > 115200);
+		rst = mcs->speed > 115200;
 		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
 

@@ -619,7 +619,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	} else {
 		rval |= MCS_FIR;
 
-		rst = (mcs->speed != 4000000);
+		rst = mcs->speed != 4000000;
 		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
drivers/staging/irda/net/irqueue.c +1 -1
index 5aab072..14291cb 100644
@@ -217,7 +217,7 @@ static __u32 hash( const char* name)
 
 	while(*name) {
 		h = (h<<4) + *name++;
-		g = (h & 0xf0000000);
+		g = h & 0xf0000000;
 		if (g)
 			h ^=g>>24;
 		h &=~g;
Signature · 1 line
2.7.4
#3 lore raw

Re: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment

Quoted 3 lines · Srishti Sharma

Parentheses are not needed on the right hand side of assignment statement in most cases. Done using the following semantic patch by coccinelle.

[]

Quoted 16 lines

@@
identifier E,F,G,f;
expression e,r;
@@

(
E = (G == F);

Quoted 1 line

E = (e == r);

Quoted 1 line

E =

(quoted diff) +0 -2
-(
...
-)
Quoted 2 lines

;
)

[]

drivers/staging/irda/drivers/mcs7780.c

Quoted 1 line
drivers/staging/irda/drivers/mcs7780.c +0 -0

[]

drivers/staging/irda/drivers/mcs7780.c static int mcs_speed_change(struct mcs_cb *mcs) · line 610

Quoted 8 lines
drivers/staging/irda/drivers/mcs7780.c +1 -1
@@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	if (mcs->new_speed <= 115200) {
 		rval &= ~MCS_FIR;
 
-		rst = (mcs->speed > 115200);
+		rst = mcs->speed > 115200;
 		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);

Coccinelle is a good tool, but its output is limited to the correctness
and completeness of its input script.

Please look at the suggested modifications of the script and examine the code for other similar uses.

The else if block immediately below this is:

	} else if (mcs->new_speed <= 1152000) {
		rval &= ~MCS_FIR;

		if ((rst = !(mcs->speed == 576000 || mcs->speed == 11520
00)))
			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);

which should also be corrected by this patch.

#4 lore raw

Re: [Outreachy kernel] Re: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment

Quoted 44 lines · Joe Perches
Quoted 3 lines · Srishti Sharma

Parentheses are not needed on the right hand side of assignment statement in most cases. Done using the following semantic patch by coccinelle.

[]

Quoted 16 lines

@@
identifier E,F,G,f;
expression e,r;
@@

(
E = (G == F);

Quoted 1 line

E = (e == r);

Quoted 1 line

E =

(quoted diff) +0 -2
-(
...
-)
Quoted 2 lines

;
)

[]

drivers/staging/irda/drivers/mcs7780.c

Quoted 1 line
drivers/staging/irda/drivers/mcs7780.c +0 -0

[]

drivers/staging/irda/drivers/mcs7780.c static int mcs_speed_change(struct mcs_cb *mcs) · line 610

Quoted 8 lines
drivers/staging/irda/drivers/mcs7780.c +1 -1
@@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	if (mcs->new_speed <= 115200) {
 		rval &= ~MCS_FIR;
 
-		rst = (mcs->speed > 115200);
+		rst = mcs->speed > 115200;
 		if (rst)
 			mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);

Coccinelle is a good tool, but its output is limited to the correctness
and completeness of its input script.

Please look at the suggested modifications of the script and examine the code for other similar uses.

The else if block immediately below this is:

Quoted 8 lines
	} else if (mcs->new_speed <= 1152000) {
		rval &= ~MCS_FIR;

		if ((rst = !(mcs->speed == 576000 || mcs->speed == 11520
00)))
			mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);

which should also be corrected by this patch.

You're concerned about the assignment in the if header? Because that was in 1/2. One could also push the ! under the ||, but I'm not sure that would be much of an improvement.

julia

Quoted 8 lines

--
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 [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505511108.27581.16.camel%40perches.com.
For more options, visit https://groups.google.com/d/optout.