linux-kernel - Re: [PATCH 1/2] drivers: usb: gadget: udc: add missing break in switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170208040243.Horde.88jNxPiVAcL_hzrlS8ZVsGy@gator4166.hostgator.com>
Date:   Wed, 08 Feb 2017 04:02:43 -0600
From:   "Gustavo A. R. Silva" <garsilva@...eddedor.com>
To:     Felipe Balbi <balbi@...nel.org>
Cc:     gregkh@...uxfoundation.org, bhumirks@...il.com, mina86@...a86.com,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] drivers: usb: gadget: udc: add missing break in
 switch

Hello Felipe,

Quoting Felipe Balbi <balbi@...nel.org>:

> Hi,
>
> "Gustavo A. R. Silva" <garsilva@...eddedor.com> writes:
>> Add missing break in switch.
>>
>> Addresses-Coverity-ID: 201385
>> Signed-off-by: Gustavo A. R. Silva <garsilva@...eddedor.com>
>> ---
>>  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:

case USB_ENDPOINT_XFER_CONTROL:
  	ios = 1;
         mult = 0;
	break;


Thank you
--
Gustavo A. R. Silva