On Mon, 20 Feb 2017, Gustavo A. R. Silva wrote:
> Hello everybody,
>
> I ran into the following piece of code at
> drivers/usb/misc/usbtest.c:149 (linux-next)
>
> 149 /* take the first altsetting with in-bulk + out-bulk;
> 150 * ignore other endpoints and altsettings.
> 151 */
> 152 for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
> 153 struct usb_host_endpoint *e;
> 154
> 155 e = alt->endpoint + ep;
> 156 switch (usb_endpoint_type(&e->desc)) {
> 157 case USB_ENDPOINT_XFER_BULK:
> 158 break;
> 159 case USB_ENDPOINT_XFER_INT:
> 160 if (dev->info->intr)
> 161 goto try_intr;
> 162 case USB_ENDPOINT_XFER_ISOC:
> 163 if (dev->info->iso)
> 164 goto try_iso;
> 165 /* FALLTHROUGH */
> 166 default:
> 167 continue;
> 168 }
>
> The thing is that the case for USB_ENDPOINT_XFER_INT is not terminated
> by a break statement, and it falls through to the next case
> USB_ENDPOINT_XFER_ISOC, in case "if (dev->info->intr)" turns to be
> false.
>
> My question here is if this code is intentional?
As far as I can tell, it is not.
> Similar to the case for USB_ENDPOINT_XFER_ISOC, which falls through to
> the default case. But in that case there is a code comment that
> confirms such behavior.
>
> In case it is not intentional, I will write a patch to fix this.
> In case it is indeed intentional I think it would be good to add a
> code comment (/* fall through */) before "case USB_ENDPOINT_XFER_ISOC:"
>
> It would be great to hear any comment about this.
The USB_ENDPOINT_XFER_INT case should end with "continue;".
Although, in fact that whole loop could be structured better. The
"goto" parts really should be all in-line. Then the flow would be a
lot easier to follow.
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
