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@xxxxxxxxxxxxxxxxx>
>>>> ---
>>>> 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:
0x92698E6A.asc
Description: application/pgp-keys
