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: 1437 bytes --]

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).

-- 
Felipe

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