Messages in this thread | | | Date | Tue, 21 Feb 2017 23:26:39 -0600 | From | "Gustavo A. R. Silva" <> | Subject | Re: [PATCH] usb: misc: add a missing continue and refactor code |
| |
Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>:
> Hi Alan, > > Quoting Alan Stern <stern@rowland.harvard.edu>: > >> On Tue, 21 Feb 2017, Gustavo A. R. Silva wrote: >> >>> Code refactoring to make the flow easier to follow and add missing >>> 'continue' for case USB_ENDPOINT_XFER_INT. >>> >>> Addresses-Coverity-ID: 1248733 >>> Cc: Alan Stern <stern@rowland.harvard.edu> >>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> >>> --- >>> drivers/usb/misc/usbtest.c | 50 >>> +++++++++++++++++++++++++++------------------- >>> 1 file changed, 30 insertions(+), 20 deletions(-) >>> >>> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c >>> index 3525626..8723e33 100644 >>> --- a/drivers/usb/misc/usbtest.c >>> +++ b/drivers/usb/misc/usbtest.c >>> @@ -124,6 +124,32 @@ static struct usb_device >>> *testdev_to_usbdev(struct usbtest_dev *test) >>> >>> /*-------------------------------------------------------------------------*/ >>> >>> +static inline void try_intr(struct usb_host_endpoint *e, >>> + struct usb_host_endpoint *int_in, >>> + struct usb_host_endpoint *int_out) >>> +{ >>> + if (usb_endpoint_dir_in(&e->desc)) { >>> + if (!int_in) >>> + int_in = e; >>> + } else { >>> + if (!int_out) >>> + int_out = e; >>> + } >>> +} >>> + >>> +static inline void try_iso(struct usb_host_endpoint *e, >>> + struct usb_host_endpoint *iso_in, >>> + struct usb_host_endpoint *iso_out) >>> +{ >>> + if (usb_endpoint_dir_in(&e->desc)) { >>> + if (!iso_in) >>> + iso_in = e; >>> + } else { >>> + if (!iso_out) >>> + iso_out = e; >>> + } >>> +} >>> + >> >> This is not at all what I had in mind. First, it's incorrect (can you >> see why?). Second, by "inline" I meant moving the code to be actually >> in-line next to the conditional, not some place else in a separate >> subroutine (even if the subroutine is declared inline). >> > > Interesting... let me double check. > > I thought it would've been better to have separate inline > subroutines for those "goto". > >> Also, the code for the USB_ENDPOINT_XFER_BULK case should look like the >> other two. >> > > Do you mean a 'continue' instead of the 'break'? >
Oh I see, the following piece of code should be part of the USB_ENDPOINT_XFER_BULK case:
if (usb_endpoint_dir_in(&e->desc)) { if (!in) in = e; } else { if (!out) out = e; } continue;
-- Gustavo A. R. Silva
| |