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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to