Re: [PATCH 4/9] staging: nvec: ps2: add some more debug functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


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


[Video for Linux]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Photos]     [Yosemite Photos]     [Free Singles Community]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Devices]     [Yosemite Backpacking]

Add to Google Powered by Linux