Hi, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes: > 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. heh :-) > 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. works for me -- balbi
Attachment:
signature.asc
Description: PGP signature
