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

[Simon Glass]

Hi Thierry,

On Tue, 21 May 2019 at 04:21, Thierry Reding <thierry.reding at gmail.com> wrote:
>
> 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?

No. Oddly enough I haven't seen a Coverity report on this.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon