linux-kernel - Re: [PATCH] staging: pi433: remove unnecessary parentheses

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180110084414.GA10805@kroah.com>
Date:   Wed, 10 Jan 2018 09:44:14 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Joe Perches <joe@...ches.com>
Cc:     Valentin Vidic <Valentin.Vidic@...Net.hr>,
        David Miller <davem@...emloft.net>, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org,
        Marcin Ciupak <marcin.s.ciupak@...il.com>,
        Marcus Wolf <linux@...f-entwicklungen.de>,
        Simon Sandström <simon@...anor.nu>
Subject: Re: [PATCH] staging: pi433: remove unnecessary parentheses

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