[PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else

[PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else

[Luis Gerhorst]

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.

Signed-off-by: Luis Gerhorst <linux-kernel@luisgerhorst.de>
Acked-by: Jonny Schaefer <schaefer.jonny@gmail.com>
Acked-by: Alexander Wuerstlein <arw@cs.fau.de>
---
 drivers/staging/fbtft/fbtft-core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 66b46b2..34b1c81 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1367,19 +1367,18 @@ int fbtft_probe_common(struct fbtft_display *display,
 	}
 
 	/* write register functions */
-	if (display->regwidth == 8 && display->buswidth == 8) {
+	if (display->regwidth == 8 && display->buswidth == 8)
 		par->fbtftops.write_register = fbtft_write_reg8_bus8;
-	} else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
+	else if (display->regwidth == 8 && display->buswidth == 9 && par->spi)
 		par->fbtftops.write_register = fbtft_write_reg8_bus9;
-	} else if (display->regwidth == 16 && display->buswidth == 8) {
+	else if (display->regwidth == 16 && display->buswidth == 8)
 		par->fbtftops.write_register = fbtft_write_reg16_bus8;
-	} else if (display->regwidth == 16 && display->buswidth == 16) {
+	else if (display->regwidth == 16 && display->buswidth == 16)
 		par->fbtftops.write_register = fbtft_write_reg16_bus16;
-	} else {
+	else
 		dev_warn(dev,
 			"no default functions for regwidth=%d and buswidth=%d\n",
 			display->regwidth, display->buswidth);
-	}
 
 	/* write_vmem() functions */
 	if (display->buswidth == 8)
-- 
2.7.4

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

[Dan Carpenter]

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:


	} else if (display->regwidth == 8 && display->buswidth == 9 &&
		   par->spi) {

regards,
dan carpenter

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

[Joe Perches]

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.

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.

> 	} 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.

	} 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.

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

[Dan Carpenter]

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

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

[Joe Perches]

On Mon, 2018-01-08 at 22:26 +0300, Dan Carpenter wrote:
> 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.

Hey Dan.

btw: I completely agree with this

> > > 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.

I don't fine any examples that match your
suggestion above in drivers/staging/fbtft.

Perhaps because there aren't many multi line
if statements.

All of the blocks that use braces are required
because one or more of the if/else blocks
contain multiple statements.

There are counterexamples in the subsystem.

cheers, Joe

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

[Dan Carpenter]

> There are counterexamples in the subsystem.

Staging is essentially one giant counter example.

regards,
dan carpenter