2017-09-15 21:05:32

by Srishti Sharma

[permalink] [raw]
Subject: [PATCH 0/2] 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(-)

--
2.7.4


2017-09-15 21:05:55

by Srishti Sharma

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..82bfc05 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -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__);
- }
}

/*------------------------------------------------------------------*/
diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index c3f0b25..2b674d5 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -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);

}
diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 160dc89..5aab072 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -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;
}
--
2.7.4

2017-09-15 21:06:24

by Srishti Sharma

[permalink] [raw]
Subject: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment

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(-)

diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index 2b674d5..d52e9f4 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -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);

diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 5aab072..14291cb 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -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;
--
2.7.4

2017-09-15 21:31:55

by Joe Perches

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

On Sat, 2017-09-16 at 02:36 +0530, Srishti Sharma wrote:
> 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 =
> -(
> ...
> -)
> ;
> )
[]
> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
[]
> @@ -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.

2017-09-15 21:50:32

by Julia Lawall

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



On Fri, 15 Sep 2017, Joe Perches wrote:

> On Sat, 2017-09-16 at 02:36 +0530, Srishti Sharma wrote:
> > 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 =
> > -(
> > ...
> > -)
> > ;
> > )
> []
> > diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
> []
> > @@ -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.

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

>
> --
> 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.
>