Hi Alan,

> Just a couple of minor comments:
> 
> It's not really necessary to restrict yourself to printable Latin
> characters (although it is safer).  After all, properly-formed string
> descriptors may contain arbitrary UCS-16 characters.  You could just
> terminate the scanning when you find \u0000.

I considered this to be too dangerous.  Another possibility is to check
that all the characters have the same high byte.  Does that actually
indicate anything meaningful though?

> Stylistically, I disapprove of
> 
> > +                   usb_try_string_workarounds(buf, &rc);
> 
> when you could just as easily write
> 
> > +                   rc = usb_try_string_workarounds(buf, rc);

This was a deliberate choice.  usb_try_string_workarounds modifies
both rc and the contents of buf.  To my mind, the function form
implicitly suggests that the arguments are not modified, which is
not true for buf.  Also, it suggests an asymmetry between buf and
rc which does not exist.  The procedural form seemed more natural,
since it more clearly suggests what happens: rc and buf go in, and
are both modified.

> But on the whole I approve of the patch.

Great, thanks.

All the best,

Duncan.


-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to