From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933222AbdBHNK5 (ORCPT ); Wed, 8 Feb 2017 08:10:57 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:52616 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932998AbdBHNKw (ORCPT ); Wed, 8 Feb 2017 08:10:52 -0500 Date: Wed, 8 Feb 2017 14:03:57 +0100 From: Greg KH To: Felipe Balbi Cc: "Gustavo A. R. Silva" , bhumirks@gmail.com, mina86@mina86.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] drivers: usb: gadget: udc: add missing break in switch Message-ID: <20170208130357.GC28327@kroah.com> References: <20170208072212.GA8209@embeddedgus> <87mvdxxchc.fsf@linux.intel.com> <20170208040243.Horde.88jNxPiVAcL_hzrlS8ZVsGy@gator4166.hostgator.com> <87inokyjjk.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87inokyjjk.fsf@linux.intel.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 08, 2017 at 02:05:35PM +0200, Felipe Balbi wrote: > > Hi, > > "Gustavo A. R. Silva" writes: > >> "Gustavo A. R. Silva" writes: > >>> Add missing break in switch. > >>> > >>> Addresses-Coverity-ID: 201385 > >>> Signed-off-by: Gustavo A. R. Silva > >>> --- > >>> 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