On Wed, Feb 08, 2017 at 02:05:35PM +0200, Felipe Balbi wrote: > > Hi, > > "Gustavo A. R. Silva" <garsilva@xxxxxxxxxxxxxx> writes: > >> "Gustavo A. R. Silva" <garsilva@xxxxxxxxxxxxxx> writes: > >>> Add missing break in switch. > >>> > >>> Addresses-Coverity-ID: 201385 > >>> Signed-off-by: Gustavo A. R. Silva <garsilva@xxxxxxxxxxxxxx> > >>> --- > >>> drivers/usb/gadget/udc/mv_udc_core.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/drivers/usb/gadget/udc/mv_udc_core.c > >>> b/drivers/usb/gadget/udc/mv_udc_core.c > >>> index 27ebb0d..56b3574 100644 > >>> --- a/drivers/usb/gadget/udc/mv_udc_core.c > >>> +++ b/drivers/usb/gadget/udc/mv_udc_core.c > >>> @@ -489,6 +489,7 @@ static int mv_ep_enable(struct usb_ep *_ep, > >>> break; > >>> case USB_ENDPOINT_XFER_CONTROL: > >>> ios = 1; > >>> + break; > >> > >> are you SURE this is supposed to have this break statement? What if we > >> want to initialize mult to 0 *also* for control endpoints? How did you > >> test this? Do you have access to Marvel's documentation for this > >> controller? > >> > > > > Certainly I wasn't sure, but I also think this is kind of obscure > > code. If that is the case that we also want to initialize mult to 0, > > wouldn't it be clearer (for maintenance purposes) to add mult = 0 and > > the break statement after ios = 1? > > > > What do you think if I modify that piece of code as follows: > > I think you need to test it, or get someone to test it for you :-) For crap code like this where it's "obvious" that something is wrong? That's really hard. How about a nice comment instead: /* Code path falls through, is it correct or not, who knows??? */ which will make the static code checkers stop complaining about it, and if someone actually has the hardware, then they can test it. thanks, greg k-h -- 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
