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>
Phillip Wood committed
Feb 16, 2022 at 10:15 UTC
9df0fc3d578acfdeb7fa1914fcf0507adb021fa5
3 files changed
+17
-23
xdiff/xdiffi.c
+17
-16
@@ -315,16 +315,19 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
315
long *kvd, *kvdf, *kvdb;
316
xdalgoenv_t xenv;
317
diffdata_t dd1, dd2;
318
+ int res;
319
319
- if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF)
320
- return xdl_do_patience_diff(mf1, mf2, xpp, xe);
321
-
322
- if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF)
323
- return xdl_do_histogram_diff(mf1, mf2, xpp, xe);
320
+ if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0)
321
+ return -1;
322
325
- if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0) {
323
+ if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) {
324
+ res = xdl_do_patience_diff(mf1, mf2, xpp, xe);
325
+ goto out;
326
+ }
327
327
- return -1;
328
+ if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) {
329
+ res = xdl_do_histogram_diff(mf1, mf2, xpp, xe);
330
+ goto out;
331
}
332
333
/*
@@ -359,17 +362,15 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
362
dd2.rchg = xe->xdf2.rchg;
363
dd2.rindex = xe->xdf2.rindex;
364
362
- if (xdl_recs_cmp(&dd1, 0, dd1.nrec, &dd2, 0, dd2.nrec,
363
- kvdf, kvdb, (xpp->flags & XDF_NEED_MINIMAL) != 0, &xenv) < 0) {
364
-
365
- xdl_free(kvd);
366
- xdl_free_env(xe);
367
- return -1;
368
- }
369
-
365
+ res = xdl_recs_cmp(&dd1, 0, dd1.nrec, &dd2, 0, dd2.nrec,
366
+ kvdf, kvdb, (xpp->flags & XDF_NEED_MINIMAL) != 0,
367
+ &xenv);
368
xdl_free(kvd);
369
+ out:
370
+ if (res < 0)
371
+ xdl_free_env(xe);
372
372
- return 0;
373
+ return res;
374
}
375
376
xdiff/xhistogram.c
-3
@@ -372,9 +372,6 @@ out:
372
int xdl_do_histogram_diff(mmfile_t *file1, mmfile_t *file2,
373
xpparam_t const *xpp, xdfenv_t *env)
374
{
375
- if (xdl_prepare_env(file1, file2, xpp, env) < 0)
376
- return -1;
377
-
375
return histogram_diff(xpp, env,
376
env->xdf1.dstart + 1, env->xdf1.dend - env->xdf1.dstart + 1,
377
env->xdf2.dstart + 1, env->xdf2.dend - env->xdf2.dstart + 1);
xdiff/xpatience.c
-4
@@ -373,10 +373,6 @@ static int patience_diff(mmfile_t *file1, mmfile_t *file2,
373
int xdl_do_patience_diff(mmfile_t *file1, mmfile_t *file2,
374
xpparam_t const *xpp, xdfenv_t *env)
375
{
376
- if (xdl_prepare_env(file1, file2, xpp, env) < 0)
377
- return -1;
378
-
379
- /* environment is cleaned up in xdl_diff() */
376
return patience_diff(file1, file2, xpp, env,
377
1, env->xdf1.nrec, 1, env->xdf2.nrec);
378
}