[lkml]   [2017]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: Inconsistency in usb_add_gadget_udc_release() interface
On 16.08.2017 18:24, Alan Stern wrote:
> On Wed, 16 Aug 2017, Alexey Khoroshilov wrote:
>> Hello,
>> usb_add_gadget_udc_release() gets release() argument that allows to
>> release user resources.
>> As far as I can see, the release() is called on error paths
>> of usb_add_gadget_udc_release() as a result of
>> put_device(&gadget->dev);
>> except for the only path going via err1.
>> As a result a caller of the usb_add_gadget_udc_release() have no chance
>> to know if the release() was invoked or not.
>> It may lead to memory leaks (drivers/usb/gadget/udc/snps_udc_core.c)
>> or to double free (drivers/usb/gadget/udc/fsl_udc_core.c).
>> Is my reading correct? If so, should we always call release() on error paths?
> How about this (untested)?

It looks reasonable. I would only suggest also to make contract
description more explicit, e.g.

* usb_add_gadget_udc_release - adds a new gadget to the udc class
driver list
* @parent: the parent device to this udc. Usually the controller driver's
* device.
* @gadget: the gadget to be added to the list.
* @release: a gadget release function.
* Returns zero on success, negative errno otherwise.
+* Calls the gadget release function in the latter case.

Alexey Khoroshilov
Linux Verification Center, ISPRAS

 \ /
  Last update: 2017-08-16 23:16    [W:0.071 / U:1.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site