Re: [PATCH] staging: pi433: remove unnecessary parentheses

[Greg Kroah-Hartman]

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.

> > 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's the
point, you should not assume anyone knows the levels, code is written
for developers to understand first, and the compiler second.  By
removing these, it doesn't do anything for the compiler, but makes the
developer think longer about them.

> > The original code here was just fine.
> 
> And I don't really disagree with you.
> 
> David Miller has a preference for the parenthesis free
> style. I believe he mentioned it sometime in August 2017
> but I can't find it right now.
> 
> Anyway, perhaps
> ---
>  scripts/checkpatch.pl | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index d2464058ab5d..3a7499de2c2d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -4526,7 +4526,9 @@ sub process {
>  		}
>  
>  # check for unnecessary parentheses around comparisons in if uses
> -		if ($^V && $^V ge 5.10.0 && defined($stat) &&
> +# when !drivers/staging or the command-line uses --strict
> +		if (($realfile !~ m@^(?:drivers/staging/)@ || $check_orig) &&
> +		    $^V && $^V ge 5.10.0 && defined($stat) &&

How about only if in the networking area?  I don't want to take patches
for this for other parts of the kernel either, it's really useless.

thanks,

greg k-h