Netdev List
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@lip6.fr>
To: Joe Perches <joe@perches.com>
Cc: Srishti Sharma <srishtishar@gmail.com>,
	samuel@sortiz.org, gregkh@linuxfoundation.org,
	netdev@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org, outreachy-kernel@googlegroups.com
Subject: Re: [Outreachy kernel] Re: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment
Date: Fri, 15 Sep 2017 23:50:29 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.20.1709152348360.2533@hadrien> (raw)
In-Reply-To: <1505511108.27581.16.camel@perches.com>



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 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/1505511108.27581.16.camel%40perches.com.
> For more options, visit https://groups.google.com/d/optout.
>

      reply	other threads:[~2017-09-15 21:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-15 21:05 [PATCH 0/2] Assign outside if Srishti Sharma
2017-09-15 21:05 ` [PATCH 1/2] Staging: irda: Don't use assignment inside if statement Srishti Sharma
2017-09-15 21:06 ` [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment Srishti Sharma
2017-09-15 21:31   ` Joe Perches
2017-09-15 21:50     ` Julia Lawall [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1709152348360.2533@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=outreachy-kernel@googlegroups.com \
    --cc=samuel@sortiz.org \
    --cc=srishtishar@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox