[U-Boot] [PATCH 1/2] fdtdec: test: Fix memory leak

[Thierry Reding]

On Mon, May 20, 2019 at 02:51:08PM -0600, Simon Glass wrote:
> Hi Thierry,
> 
> On Mon, 20 May 2019 at 10:05, Thierry Reding <thierry.reding at gmail.com> wrote:
> >
> > From: Thierry Reding <treding at nvidia.com>
> >
> > Free the memory allocated to store the test FDT upon test completion to
> > avoid leaking the memory. We don't bother cleaning up on test failure
> > since the code is broken in that case and should be fixed, in which case
> > the leak would also go away.
> >
> > Reported-by: Tom Rini <tom.rini at gmail.com>
> > Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Signed-off-by: Thierry Reding <treding at nvidia.com>
> > ---
> >  lib/fdtdec_test.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/fdtdec_test.c b/lib/fdtdec_test.c
> > index f6defe16c5a6..54efcc3d46ac 100644
> > --- a/lib/fdtdec_test.c
> > +++ b/lib/fdtdec_test.c
> > @@ -138,6 +138,7 @@ static int run_test(const char *aliases, const char *nodes, const char *expect)
> >         }
> >
> >         printf("pass\n");
> > +       free(blob);
> 
> Strictly speaking, CHECKVAL() can cause a function return in the case
> of an error.
> 
> So a better solution might be to put the code after the malloc() into
> a separate function.

When Heinrich suggested this fix he brought up the same issue, but
concluded, and I agree with him, that it wasn't worth addressing the
CHECKVAL case because if CHECKVAL failed, our code was buggy and would
need fixing, at which point the leak would go away along with the bug.

Do you feel strongly about reworking this so it doesn't leak in the
error case either?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190521/5fa2473b/attachment.sig>