Re: [PATCH v5 4/7] usb: gadget: f_midi: fix leak on failed to enqueue out requests

[Felipe Ferreri Tonello <eu@felipetonello.com>]

[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]

Hi Robert,

On 16/11/15 11:43, Robert Baldyga wrote:
> On 11/16/2015 12:08 PM, Felipe Ferreri Tonello wrote:
>> Hi Robert,
>>
>> On 13/11/15 08:31, Robert Baldyga wrote:
>>> Hi Felipe,
>>>
>>> On 11/10/2015 06:52 PM, Felipe F. Tonello wrote:
>>>> This patch fixes a memory leak that occurs when an endpoint fails to enqueue
>>>> the request. If that happens the complete function will never be called, thus
>>>> never freeing the request.
>>>>
>>>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>>>> ---
>>>>  drivers/usb/gadget/function/f_midi.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
>>>> index f36db2d..76ea53c 100644
>>>> --- a/drivers/usb/gadget/function/f_midi.c
>>>> +++ b/drivers/usb/gadget/function/f_midi.c
>>>> @@ -345,6 +345,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
>>>>  		if (err) {
>>>>  			ERROR(midi, "%s queue req: %d\n",
>>>>  				    midi->out_ep->name, err);
>>>> +			free_ep_req(midi->out_ep, req);
>>>>  		}
>>>>  	}
>>>>  
>>>>
>>>
>>> There is one more thing I haven't noticed before. We can have situation
>>> when all requests were allocated successfully, but their allocation
>>> failed. What we get then is set_alt() returning 0, while no request is
>>> allocated, hence the function is, in fact, inactive.
>>
>> Right. So in this case should we return some error? We can restrict the
>> function to work iff allocates the 'qlen' number of allocations,
>> otherwise returns an error and frees all other requests (IN and OUT).
>>
> 
> Yes, IMO it's a proper solution. When user sets qlen to given value he
> expects that exact number of requests to be allocated and enqueued, and
> if we cannot do that we should consider this as an error.
> 

Ok. I will do that in a new patch on v6 since this patch is correct
anyway and Blabi already applied to his test branch.

Felipe

[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 4830 bytes --]