linux-kernel - Re: [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else

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]

Date:   Mon, 8 Jan 2018 22:26:46 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Luis Gerhorst <linux-kernel@...sgerhorst.de>,
        devel@...verdev.osuosl.org, linux-kernel@...cs.fau.de,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org,
        Jonny Schäfer <schaefer.jonny@...il.com>
Subject: Re: [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else

On Mon, Jan 08, 2018 at 10:27:01AM -0800, Joe Perches wrote:
> On Mon, 2018-01-08 at 18:03 +0300, Dan Carpenter wrote:
> > On Mon, Jan 08, 2018 at 11:06:37AM +0100, Luis Gerhorst wrote:
> > > The Linux kernel coding style states that braces should only be used
> > > when necessary.
> > > 
> > > This fixes the checkpatch warning
> > > 
> > > WARNING: line over 80 characters
> > > +	} else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
> > > 
> > > introduced by patch #1.
> > > 
> > 
> > Don't introduce warnings and then fix them in later patches.
> > 
> > Anyway there is another unwritten rule that multi-line indents get curly
> > braces.  Probably it should be:
> 
> Nope.  That'd be your own preferred style.
> 

I copied it from Greg so it's the subsystem style.


> If you want it to be followed by others,
> please try and get it added to CodingStyle
> and use examples to show why it's better
> than allowing maintainer preference.
> 

I'm not going to NACK patches which don't follow that rule, so it's not
something which is helpful to put in checkpatch.pl.  The reason I'm
complaining about this patch is that it fixes a patch the author
introduces earlier in the patchset.


> > 	} else if (display->regwidth == 8 && display->buswidth == 9 &&
> > 		   par->spi) {
> 
> My own preferred style here would be to align
> the display->regwidth and display->buswidth so
> I can differentiate the similarity in naming a
> bit more easily.

Yeah.  I thought about that as well but then it takes up 3 lines...
Either way is fine.

> 
> 	} else if (display->regwidth == 8 &&
> 		   display->buswidth == 9 && par->spi) {
> 
> I'm not sure at all there is a single best
> style for this and it can easily become
> situation dependent.
> 
> And so I suggest not adding anything about this
> style nit to CodingStyle.

Agreed.

regards,
dan carpenter