On Fri, 25 Jan 2013, Greg KH wrote:
> > +static int has_required_endpoints(const struct usb_host_interface *interface)
> > +{
> > + __u8 i;
> > + int has_bulk_in = 0;
> > + int has_bulk_out = 0;
> > +
> > + for (i = 0; i < interface->desc.bNumEndpoints; ++i) {
> > + if (usb_endpoint_is_bulk_in(&interface->endpoint[i].desc))
> > + has_bulk_in = 1;
> > + else if (usb_endpoint_is_bulk_out(&interface->endpoint[i].desc))
> > + has_bulk_out = 1;
> > + }
> > +
> > + return has_bulk_in && has_bulk_out;
> > +}
>
> Well, only one _very_ tiny issue. And it's nothing to stop me from
> taking this patch, but only for you to know about.
>
> Variable types that start with "__" are there as they cross the
> user/kernel boundry. On the kernel side, they can always be referenced
> by using the non-"__" type, as it is the same. So you almost never see
> local variables with the "__" type.
>
> So you really should just make the first line of this function be:
> u8 i;
In this case I'd say to make it a plain old
int i;
There's no reason for restricting i to be 8 bits. It isn't a field in
a structure or a member of an array, so saving space isn't an issue.
Neither is sign extension. Why force the compiler to go to extra
effort when using a natural-sized integer will work just as well?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
