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

[Simon Glass]

On Tue, 21 May 2019 at 10:43, Simon Glass <sjg at chromium.org> wrote:
>
> 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>

Applied to u-boot-dm/next, thanks!