Date: Wed, 10 Jan 2018 09:44:14 +0100
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Joe Perches <joe@perches.com>
Cc: Valentin Vidic <Valentin.Vidic@CARNet.hr>,
        David Miller <davem@davemloft.net>, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org,
        Marcin Ciupak <marcin.s.ciupak@gmail.com>,
        Marcus Wolf <linux@wolf-entwicklungen.de>,
        Simon =?iso-8859-1?Q?Sandstr=F6m?= <simon@nikanor.nu>
Subject: Re: [PATCH] staging: pi433: remove unnecessary parentheses
Message-ID: <20180110084414.GA10805@kroah.com>
References: <20180108173855.19366-1-Valentin.Vidic@CARNet.hr>
 <20180109143148.GB608@kroah.com>
 <1515525697.9619.100.camel@perches.com>
 <20180109192844.GB20280@kroah.com>
 <1515526936.9619.121.camel@perches.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1515526936.9619.121.camel@perches.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org

On Tue, Jan 09, 2018 at 11:42:16AM -0800, Joe Perches wrote:
> On Tue, 2018-01-09 at 20:28 +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 09, 2018 at 11:21:37AM -0800, Joe Perches wrote:
> > > On Tue, 2018-01-09 at 15:31 +0100, Greg Kroah-Hartman wrote:
> > > > On Mon, Jan 08, 2018 at 06:38:55PM +0100, Valentin Vidic wrote:
> > > > > Fixes checkpatch warnings:
> > > > >   CHECK: Unnecessary parentheses around 'mantisse != mantisse16'
> > > > >   CHECK: Unnecessary parentheses around 'mantisse != mantisse20'
> > > > >   CHECK: Unnecessary parentheses around 'mantisse != mantisse24'
> > > 
> > > []
> > > > > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> > > 
> > > []
> > > > > @@ -391,9 +391,9 @@ static int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg,
> > > > >  		return -EINVAL;
> > > > >  	}
> > > > >  
> > > > > -	if ((mantisse != mantisse16) &&
> > > > > -	    (mantisse != mantisse20) &&
> > > > > -	    (mantisse != mantisse24)) {
> > > > > +	if (mantisse != mantisse16 &&
> > > > > +	    mantisse != mantisse20 &&
> > > > > +	    mantisse != mantisse24) {
> > > > 
> > > > I'm getting really tired of seeing this checkpatch warning, when it's a
> > > > major pain.
> > > 
> > > Your idea of major pain and mine differ a bit.
> > 
> > I don't like taking patches that cause future problems.
> 
> What future problems might this particular case present
> that isn't generic in all patches.
> 
> > > > Joe, can you please turn these off.  Patches like this will force people
> > > > to have to remember that != is higher precidence than &&.
> > > 
> > > As it's not just 1 precedence level but 4 and 5, it
> > > really shouldn't be that hard to remember.
> > 
> > I can't remember any of them, and I should not have to.
> 
> That depends on how well you know your C.

I have used C for almost ever single day for the past 20+ years, and I
sure don't remember the order of these things.  But maybe I really don't
know my C :)

> >   That's the
> > point, you should not assume anyone knows the levels, code is written
> > for developers to understand first, and the compiler second.
> 
> And someone that knows C knows those levels and the parentheses
> can just be visual noise requiring extra thought.
> 
> Sometimes it's useful, sometimes it's not.
> 
> 	if (a == b && c == d)
> 
> is pretty trivial.

But again, don't do that.

> and I believe
> 
> 	if ((a == b))
> 
> emits clang warnings

Then remove the extra () there.

thanks,

greg k-h