Re: [PATCH v2] staging: wlan-ng: Remove unnecessary parentheses

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Oct 18, 2017 at 10:46:09AM -0400, Frank A. Cancio Bello wrote:
> On Wed, Oct 18, 2017 at 04:17:04PM +0200, Greg KH wrote:
> > On Mon, Oct 16, 2017 at 09:48:21PM -0400, Frank A. Cancio Bello wrote:
> > > Remove unnecessary parentheses to comply with preferred coding style for
> > > the linux kernel and avoid the following checkpatch's message:
> > > 'CHECK: Unnecessary parentheses around'
> > > 
> > > Credits to checkpatch.
> > > 
> > > Signed-off-by: Frank A. Cancio Bello <frank@generalsoftwareinc.com>
> > > ---
> > > Changes in v2:
> > > 	* I rewrote the log message to improve the style taking in consideration Julia's suggestions.
> > > 	* I merged in this patch similars changes that initially were in theirs own patch. I will reply that other patch email thread, saying to discard it, to avoid confussion.
> > > 
> > >  drivers/staging/wlan-ng/cfg80211.c     | 10 +++++-----
> > >  drivers/staging/wlan-ng/hfa384x_usb.c  | 18 +++++++++---------
> > >  drivers/staging/wlan-ng/p80211conv.c   |  6 +++---
> > >  drivers/staging/wlan-ng/p80211netdev.c |  4 ++--
> > >  drivers/staging/wlan-ng/p80211req.c    |  2 +-
> > >  drivers/staging/wlan-ng/prism2fw.c     | 23 +++++++++++------------
> > >  drivers/staging/wlan-ng/prism2mgmt.c   | 29 ++++++++++++++---------------
> > >  drivers/staging/wlan-ng/prism2sta.c    |  4 ++--
> > >  8 files changed, 47 insertions(+), 49 deletions(-)
> > > 
> > > diff --git a/drivers/staging/wlan-ng/cfg80211.c b/drivers/staging/wlan-ng/cfg80211.c
> > > index 178f6f5..03279aa 100644
> > > --- a/drivers/staging/wlan-ng/cfg80211.c
> > > +++ b/drivers/staging/wlan-ng/cfg80211.c
> > > @@ -265,7 +265,7 @@ static int prism2_get_station(struct wiphy *wiphy, struct net_device *dev,
> > >  
> > >  	memset(sinfo, 0, sizeof(*sinfo));
> > >  
> > > -	if (!wlandev || (wlandev->msdstate != WLAN_MSD_RUNNING))
> > > +	if (!wlandev || wlandev->msdstate != WLAN_MSD_RUNNING)
> > 
> > That's not "unnecessary" as now I need to go look up or try to remember
> > the order of operations for != and || :(
> > 
> 
> Thanks for your help Greg!
> I agreed with you. I'm a newbie still learning the kernel code style.
> When I run:
>   perl scripts/checkpatch.pl -f drivers/staging/wlan-ng/cfg80211.c
> I get the following warning:
> 
> CHECK: Unnecessary parentheses around 'wlandev->msdstate != WLAN_MSD_RUNNING'
> #268: FILE: drivers/staging/wlan-ng/cfg80211.c:268:
> +       if (!wlandev || (wlandev->msdstate != WLAN_MSD_RUNNING))
> 
> and that is why I proposed the change.

checkpatch is a hint, it's not always correct :)

> > Please don't make things such that it is harder to read for a
> > programmer.
> > 
> 
> Is OK to say that in cases of newline after the || parentheses could be 
> removed without affect code clarity?
> 
> Would you accept a v3 of this patch that just removes parentheses in those
> cases? Or you prefer to drop this patch?

I've already dropped this, if you have other changes in this patch that
do not make things harder to understand, please fix up and resend.

thanks,

greg k-h