Hi Alan,
Quoting Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
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.
I'll code the "goto" parts into in-line functions and send a patch.
Thanks for clarifying.
--
Gustavo A. R. Silva
--
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
