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

[Ameya Palande <ameya.palande@nokia.com>]

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.

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