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 <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 69689fab2478c8fed40b63e9a7cefb43c13e7ae7..758410c11ac286adc77c6f992e51822def202a40 100644 (file)
--- 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)
+               return -1;
 
-       if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0) {
+       if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) {
+               res = xdl_do_patience_diff(mf1, mf2, xpp, xe);
+               goto out;
+       }
 
-               return -1;
+       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 80794748b0de6bb9176ce088c472d44c62b91e6d..01decffc332629dd9dcfd79c904187b7cc6d0943 100644 (file)
--- 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 c5d48e80aefb33eddb4dbe4e359f2c598a5483d7..e8de8d150cede1d1c5ad1627e9cc5bc7a490725d 100644 (file)
--- 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);
 }