Hi Thierry,
On Tue, 21 May 2019 at 04:21, Thierry Reding thierry.reding@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@gmail.com wrote:
From: Thierry Reding treding@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@gmail.com Suggested-by: Heinrich Schuchardt xypron.glpk@gmx.de Signed-off-by: Thierry Reding treding@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@chromium.org
Regards, Simon