From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754413AbdBVBlF (ORCPT ); Tue, 21 Feb 2017 20:41:05 -0500 Received: from netrider.rowland.org ([192.131.102.5]:59707 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753937AbdBVBk4 (ORCPT ); Tue, 21 Feb 2017 20:40:56 -0500 Date: Tue, 21 Feb 2017 20:40:53 -0500 (EST) From: Alan Stern X-X-Sender: stern@netrider.rowland.org To: "Gustavo A. R. Silva" cc: Greg Kroah-Hartman , Felipe Balbi , Peter Chen , Chunfeng Yun , Mathias Nyman , , , Peter Senna Tschudin Subject: Re: [PATCH] usb: misc: add a missing continue and refactor code In-Reply-To: <20170221231708.GA8073@embeddedgus> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Signed-off-by: Gustavo A. R. Silva > --- > 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). Also, the code for the USB_ENDPOINT_XFER_BULK case should look like the other two. Alan Stern