Re: [PATCH] DSPBRIDGE: Fix memory leak in PROC_AutoStart()

[Omar Ramirez Luna <omar.ramirez@ti.com>]

Hi,

On 2/1/2010 11:31 AM, Ameya Palande wrote:
> Hi Omar,
>
> On Mon, 2010-01-25 at 20:21 +0100, ext Omar Ramirez Luna wrote:
>> Hi,
>>
>> On 1/21/2010 7:03 AM, Ameya Palande wrote:
>>> Signed-off-by: Ameya Palande<ameya.palande@nokia.com>
>>> ---
>>>    drivers/dsp/bridge/rmgr/proc.c |    4 ++++
>>>    1 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
>>> index a75b64a..91ab64f 100644
>>> --- a/drivers/dsp/bridge/rmgr/proc.c
>>> +++ b/drivers/dsp/bridge/rmgr/proc.c
>>> @@ -512,6 +512,10 @@ DSP_STATUS PROC_AutoStart(struct CFG_DEVNODE *hDevNode,
>>>    			 "No Exec file found \n");
>>>    	}
>>>    func_cont:
>>> +	if (hProcObject->g_pszLastCoff) {
>>> +		MEM_Free(hProcObject->g_pszLastCoff);
>>> +		hProcObject->g_pszLastCoff = NULL;
>>> +	}
>>
>> Wouldn't be better to keep this inside PROC_Load in case of error?
>
> PROC_Load allocates the memory to hProcObject->g_pszLastCoff which gets
> freed in PROC_Detach. But PROC_AutoStart is a special case. It creates a
> dummy ProcObject and after it is done with it, it frees it. And there it
> leaks the memory. So IMO this is the correct place.

Ok, agree.

>
>> Also MEM_Free checks for NULL.
>
> Agreed! I will send a V2 of the patch with without the check.
>
>>
>>>    	MEM_FreeObject(hProcObject);
>>>    func_end:
>>>    	GT_1trace(PROC_DebugMask, GT_ENTER,
>>
>> Regards,
>>
>> Omar
>
> Thanks for your review!
>
> Cheers,
> Ameya.
>

- omar