[U-Boot] [PATCH] drivers/elbc:Memory leak fix

[U-Boot] [PATCH] drivers/elbc:Memory leak fix

[Raghav Dogra]

Freeing allocated memory to priv before returning
from the function

Signed-off-by: Raghav Dogra <raghav@freescale.com>
---
 drivers/mtd/nand/fsl_elbc_nand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index e85832d..bbf1f39 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -679,6 +679,7 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr)
 	if (priv->bank >= MAX_BANKS) {
 		printf("fsl_elbc_nand: address did not match any "
 		       "chip selects\n");
+		kfree((void *)priv);
 		return -ENODEV;
 	}
 
-- 
1.9.1

[U-Boot] drivers/elbc:Memory leak fix

[Scott Wood]

On Wed, May 20, 2015 at 02:54:58PM +0530, Raghav Dogra wrote:
> Freeing allocated memory to priv before returning
> from the function
> 
> Signed-off-by: Raghav Dogra <raghav@freescale.com>
> ---
>  drivers/mtd/nand/fsl_elbc_nand.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index e85832d..bbf1f39 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -679,6 +679,7 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr)
>  	if (priv->bank >= MAX_BANKS) {
>  		printf("fsl_elbc_nand: address did not match any "
>  		       "chip selects\n");
> +		kfree((void *)priv);
>  		return -ENODEV;
>  	}

Cast is unnecessary; I'll remove when applying.

-Scott

[U-Boot] drivers/elbc:Memory leak fix

[Kushwaha Prabhakar]

> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Scott
> Wood
> Sent: Saturday, May 23, 2015 1:57 AM
> To: Dogra Raghav-B46184
> Cc: u-boot at lists.denx.de; Sun York-R58495
> Subject: Re: [U-Boot] drivers/elbc:Memory leak fix
> 
> On Wed, May 20, 2015 at 02:54:58PM +0530, Raghav Dogra wrote:
> > Freeing allocated memory to priv before returning from the function
> >
> > Signed-off-by: Raghav Dogra <raghav@freescale.com>
> > ---
> >  drivers/mtd/nand/fsl_elbc_nand.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/mtd/nand/fsl_elbc_nand.c
> > b/drivers/mtd/nand/fsl_elbc_nand.c
> > index e85832d..bbf1f39 100644
> > --- a/drivers/mtd/nand/fsl_elbc_nand.c
> > +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> > @@ -679,6 +679,7 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr)
> >  	if (priv->bank >= MAX_BANKS) {
> >  		printf("fsl_elbc_nand: address did not match any "
> >  		       "chip selects\n");
> > +		kfree((void *)priv);
> >  		return -ENODEV;
> >  	}
> 
> Cast is unnecessary; I'll remove when applying.
> 

It is required as kfree declaration need void pointer. 
 It is not getting catch during GCC build. But during static analysis using Klockwork, this is shown as error. 

I will suggest to have this patch as it is. It is good practice to pass pointer in the desired format to called function. 

--prabhakar

[U-Boot] drivers/elbc:Memory leak fix

[Scott Wood]

On Fri, 2015-05-22 at 21:55 -0500, Kushwaha Prabhakar-B32579 wrote:
> > -----Original Message-----
> > From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Scott
> > Wood
> > Sent: Saturday, May 23, 2015 1:57 AM
> > To: Dogra Raghav-B46184
> > Cc: u-boot at lists.denx.de; Sun York-R58495
> > Subject: Re: [U-Boot] drivers/elbc:Memory leak fix
> > 
> > On Wed, May 20, 2015 at 02:54:58PM +0530, Raghav Dogra wrote:
> > > Freeing allocated memory to priv before returning from the function
> > >
> > > Signed-off-by: Raghav Dogra <raghav@freescale.com>
> > > ---
> > >  drivers/mtd/nand/fsl_elbc_nand.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/mtd/nand/fsl_elbc_nand.c
> > > b/drivers/mtd/nand/fsl_elbc_nand.c
> > > index e85832d..bbf1f39 100644
> > > --- a/drivers/mtd/nand/fsl_elbc_nand.c
> > > +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> > > @@ -679,6 +679,7 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr)
> > >  	if (priv->bank >= MAX_BANKS) {
> > >  		printf("fsl_elbc_nand: address did not match any "
> > >  		       "chip selects\n");
> > > +		kfree((void *)priv);
> > >  		return -ENODEV;
> > >  	}
> > 
> > Cast is unnecessary; I'll remove when applying.
> > 
> 
> It is required as kfree declaration need void pointer. 

It is *not* required.  Any pointer can be implicitly converted to a void
pointer.

>  It is not getting catch during GCC build. But during static analysis using Klockwork, this is shown as error. 

I don't care what Klockwork considers an error.  Klockwork does not
dictate the project's style guidelines.

> I will suggest to have this patch as it is. It is good practice to pass pointer in the desired format to called function. 

It is not good practice to cast unnecessarily.  Casts bypass the type
system and thus should be rare.

-Scott

[U-Boot] drivers/elbc:Memory leak fix

[Timur Tabi]

On Fri, May 22, 2015 at 9:55 PM, Kushwaha Prabhakar
<prabhakar@freescale.com> wrote:
>
>  It is not getting catch during GCC build. But during static analysis using Klockwork, this is shown as error.

Then Klockwork is wrong.