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. ;)
regards,
dan carpenter
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
