On Tue, 2012-01-10 at 15:14 +0300, Dan Carpenter wrote:
> On Tue, Jan 10, 2012 at 12:48:35PM +0100, Julian Andres Klode wrote:
> > On Tue, Jan 10, 2012 at 02:35:52PM +0300, Dan Carpenter wrote:
> > > On Mon, Dec 26, 2011 at 05:57:35PM +0100, Julian Andres Klode wrote:
> > > > case NVEC_PS2:
> > > > - if (msg[2] == 1)
> > > > + if (msg[2] == 1) {
> > > > for (i = 0; i < (msg[1] - 2); i++)
> > > > serio_interrupt(ps2_dev.ser_dev, msg[i + 4], 0);
> > > > - else if (msg[1] != 2) { /* !ack */
> > > > - print_hex_dump(KERN_WARNING, "unhandled mouse event: ",
> > > > - DUMP_PREFIX_NONE, 16, 1,
> > > > - msg, msg[1] + 2, true);
> > > > + NVEC_PHD("ps/2 mouse reply: ", &msg[4], msg[1] - 2);
> > > > }
> > > >
> > > > + else if (msg[1] != 2) /* !ack */
> > > > + NVEC_PHD("unhandled mouse event: ", msg, msg[1] + 2);
> > >
> > > Kernel style is that the else goes on the same line as the close
> > > brace and if one side of the if else statement gets braces then they
> > > both do.
> >
> > Thanks, seems I overlooked that part of the patch, and checkpatch did
> > not detect it either (should it?).
>
> Hm... It seems like checkpatch.pl doesn't care if the second arm of
> the if else statement gets the braces or not. But I'm definitely
> right on this, it's not just a random preference I have of my own
> which I tormet submitters with. ;)
>
> It's there in CodingStyle at the very end of the "Placing Braces and
> Spaces" section.
>
> Btw, generally for review comments introducing a bug is absolutely
> forbidden but if it's a small style thing or it's an existing bug
> the reviewer just noticed it's OK to send a follow on patch to clean
> that up. It's bad to ignore reviewers though because we're more
> rare and thus more special than code submitters. ;)
Andy,
Here's a snippet where checkpatch does not emit
a warning where it reasonably could:
int main(int argc, char **argv)
{
int a;
if (a) {
int foo;
int bar;
} else
int foo;
}
Maybe you could look at it?
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
