Re: [PATCH 2.6.20 1/1] usb/input updatedaiptek tablet driver

On Tue, 2007-04-03 at 16:41 -0700, Pete Zaitcev wrote:
> On Tue, 03 Apr 2007 23:25:02 +0200, René van Paassen <[email protected]> wrote:
> 
> > @@ -635,29 +760,25 @@
> >  						 TOOL_BUTTON(aiptek->curSetting.toolMode),
> >  						 1);
> 
> I think that your mailer line-wrapped it after all. Most of it is ok, but
> look at the line above, please. That seems broken.
> 
> I think Greg takes MIME, so maybe just attach it as a text/plain file?
> 
> > -	} else if (strcmp(buf, "right") == 0) {
> > +	} else if (strncmp(buf, pRight, ARRAY_SIZE(pRight)) == 0) {
> 
> Is this the right thing to do? I do not understand what you are
> trying to accomplish here by replacing strcmp with strncmp, and
> then using as length which includes the nul character. This is not
> necesserily incorrect, but it's very odd and suspicious.
> 
> When I suggested to use sizeof(pRight)-1, I thought that you wanted
> this to compare with buf which are not nul terminated. But apparently
> not?

You are right. Don't remember how I got to use ARRAY_SIZE. Will correct
this.

> >  static DEVICE_ATTR(execute,
> > -		   S_IRUGO | S_IWUGO, show_tabletExecute, store_tabletExecute);
> > +		   S_IRUGO | S_IWUGO, show_tabletExecute,
> > +		   store_tabletExecute);
> 
> I'm not going to pick upon whitespace, but just FYI you seem to fold
> a bit aggressively. I saw a couple of places where 79 and 80 character
> lines were folded.
> 
> > +	for (i = 0; i < sizeof(macroKeyEvents) / sizeof(macroKeyEvents[0]);
> > +	     ++i) {
> 
> This is a bit inconsistent. You use ARRAY_SIZE everywhere (including
> places like strncmp, where it is unusual), but this one place is missed.
> Not a show-stopper, surely, but odd.


> Best wishes,
> -- Pete
-- 
M.M. (René) van Paassen    | ______o____/_|     [email protected]
Associate Professor       <[___\_\_-----<      t/f: +31 15 27 85370/86480
Aerospace Engineering      |  o'              http://www.cs.lr.tudelft.nl
Delft University of Technology           Kluijverweg 1, NL-2629 HS  Delft
###########################################

This message has been scanned by F-Secure Anti-Virus for Microsoft Exchange.
For more information, connect to http://www.f-secure.com/

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel