From 9df0fc3d578acfdeb7fa1914fcf0507adb021fa5 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Wed, 16 Feb 2022 10:15:06 +0000 Subject: [PATCH] xdiff: fix a memory leak Although the patience and histogram algorithms initialize the environment they do not free it if there is an error. In contrast for the Myers algorithm the environment is initalized in xdl_do_diff() and it is freed if there is an error. Fix this by always initializing the environment in xdl_do_diff() and freeing it there if there is an error. Remove the comment in do_patience_diff() about the environment being freed by xdl_diff() as it is not accurate because (a) xdl_diff() does not do that if there is an error and (b) xdl_diff() is not the only caller. Reported-by: Junio C Hamano Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 35 ++++++++++++++++++----------------- xdiff/xhistogram.c | 3 --- xdiff/xpatience.c | 4 ---- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 69689fab24..758410c11a 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -315,16 +315,19 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, long *kvd, *kvdf, *kvdb; xdalgoenv_t xenv; diffdata_t dd1, dd2; + int res; - if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) - return xdl_do_patience_diff(mf1, mf2, xpp, xe); - - if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) - return xdl_do_histogram_diff(mf1, mf2, xpp, xe); - - if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0) { - + if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0) return -1; + + if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) { + res = xdl_do_patience_diff(mf1, mf2, xpp, xe); + goto out; + } + + if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) { + res = xdl_do_histogram_diff(mf1, mf2, xpp, xe); + goto out; } /* @@ -359,17 +362,15 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, dd2.rchg = xe->xdf2.rchg; dd2.rindex = xe->xdf2.rindex; - if (xdl_recs_cmp(&dd1, 0, dd1.nrec, &dd2, 0, dd2.nrec, - kvdf, kvdb, (xpp->flags & XDF_NEED_MINIMAL) != 0, &xenv) < 0) { - - xdl_free(kvd); - xdl_free_env(xe); - return -1; - } - + res = xdl_recs_cmp(&dd1, 0, dd1.nrec, &dd2, 0, dd2.nrec, + kvdf, kvdb, (xpp->flags & XDF_NEED_MINIMAL) != 0, + &xenv); xdl_free(kvd); + out: + if (res < 0) + xdl_free_env(xe); - return 0; + return res; } diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index 80794748b0..01decffc33 100644 --- a/xdiff/xhistogram.c +++ b/xdiff/xhistogram.c @@ -372,9 +372,6 @@ out: int xdl_do_histogram_diff(mmfile_t *file1, mmfile_t *file2, xpparam_t const *xpp, xdfenv_t *env) { - if (xdl_prepare_env(file1, file2, xpp, env) < 0) - return -1; - return histogram_diff(xpp, env, env->xdf1.dstart + 1, env->xdf1.dend - env->xdf1.dstart + 1, env->xdf2.dstart + 1, env->xdf2.dend - env->xdf2.dstart + 1); diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index c5d48e80ae..e8de8d150c 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -373,10 +373,6 @@ static int patience_diff(mmfile_t *file1, mmfile_t *file2, int xdl_do_patience_diff(mmfile_t *file1, mmfile_t *file2, xpparam_t const *xpp, xdfenv_t *env) { - if (xdl_prepare_env(file1, file2, xpp, env) < 0) - return -1; - - /* environment is cleaned up in xdl_diff() */ return patience_diff(file1, file2, xpp, env, 1, env->xdf1.nrec, 1, env->xdf2.nrec); }