linux-kernel - Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 15 Sep 2015 11:30:48 -0500
From:	Felipe Balbi <balbi@...com>
To:	Krzysztof Opasiak <k.opasiak@...sung.com>
CC:	<balbi@...com>, Robert Baldyga <r.baldyga@...sung.com>,
	<gregkh@...uxfoundation.org>, <linux-usb@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <b.zolnierkie@...sung.com>,
	<m.szyprowski@...sung.com>, <andrzej.p@...sung.com>
Subject: Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct
 usb_ep

Hi,

On Tue, Sep 15, 2015 at 06:15:25PM +0200, Krzysztof Opasiak wrote:
> >>>+	}
> >>>+
> >>>+	return ret;
> >>>  }
> >>>
> >>
> >>Personally I don't like this convention. In my opinion usb_ep_disable() &
> >>usb_ep_enable() should fail if ep is already disabled/enabled. Then in
> >>function code we should check if endpoint is enabled (maybe even we should
> >>have usb_ep_is_enabled()) and call disable only when it is really enabled.
> >
> >usb_ep_is_enabled() should be a good addition but I don't see an issue
> >ignoring usb_ep_enabled() for something that's already enabled.
> >
> >Imagine if you got an error when you tried to push the light switch to
> >the 'on' position while the light was already on :-p
> >
> 
> Hmmm not sure right now, didn't test this recently :D as usually I check if
> light isn't already "on" before I touch the switch to turn it on:P

not my best analogy :)

> Just joking. Personally I just prefer to don't touch things which are
> already in desired condition. Let's take close() as example which
> could be a little bit equivalent of our usb_ep_disable(). It is not
> legal to call it twice on some fd and second call ends up with error.

I understand that, it's just the difference between adding
usb_ep_is_enabled() to every single gadget/function driver or adding it
to usb_ep_enable() itself to keep gadget/function drivers cleaner.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)