Re: [PATCH] fdtget.c: Fix memory leak

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2181 bytes --]

On Wed, Jul 13, 2016 at 08:05:16AM +0200, Jean-Christophe DUBOIS wrote:
> Le 13/07/2016 04:50, David Gibson a écrit :
> > On Wed, Jul 13, 2016 at 12:36:08AM +0200, Jean-Christophe Dubois wrote:
> > > CID 132823 (#1 of 1): Resource leak (RESOURCE_LEAK)
> > > 5. leaked_storage: Variable blob going out of scope leaks the storage it points to.
> > > 
> > > Signed-off-by: Jean-Christophe Dubois <jcd-WBS85hRCVJbxB9160cZjhg@public.gmane.org>
> > Since the program exits immediately after this free(), there's really
> > no point.
> This is usually a good practice to free explicitly the memory we are
> allocating rather than relying on some kind of (final) garbage collector
> (like in java).

Well... it's important to have a consistent model for memory
management and stick to it.  For little programs like this, never
freeing - essentially using the process lifetime as a really simple
pool allocator - is a pretty valid option.

That said, I guess there are existing free()s so it's not really using
that model consistently.  So, yeah, ok, we might as well shut the
checker up by applying these.

> 
> It seems cheap to fix but this is your call.
> > 
> > > ---
> > >   fdtget.c | 8 +++++++-
> > >   1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fdtget.c b/fdtget.c
> > > index 4377419..fb9d0e1 100644
> > > --- a/fdtget.c
> > > +++ b/fdtget.c
> > > @@ -266,14 +266,20 @@ static int do_fdtget(struct display_info *disp, const char *filename,
> > >   				continue;
> > >   			} else {
> > >   				report_error(arg[i], node);
> > > +				free(blob);
> > >   				return -1;
> > >   			}
> > >   		}
> > >   		prop = args_per_step == 1 ? NULL : arg[i + 1];
> > > -		if (show_data_for_item(blob, disp, node, prop))
> > > +		if (show_data_for_item(blob, disp, node, prop)) {
> > > +			free(blob);
> > >   			return -1;
> > > +		}
> > >   	}
> > > +
> > > +	free(blob);
> > > +
> > >   	return 0;
> > >   }
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]