Change the invocations of xdl_free_env() so that only the function
that allocated the "xe" variable frees it, rather than passing it to a
function that might use "&xe", and on failure having that function
free it.
This simplifies the allocation management, since due to the new "{ 0
}" initialization we can pass "&xe" to xdl_free_env() without
accounting for the "&xe" not being initialized yet.
This change was originally suggested as an amendment of the series
that since got merged in 47be28e51e6 (Merge branch
'pw/xdiff-alloc-fail', 2022-03-09) in [1]. The "avoid double free of
xe2" in that series is one of the pattern we can simplify here.
1. https://lore.kernel.org/git/220216.86tuczt7js.gmgdl@xxxxxxxxxxxxxxxxxxx/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
xdiff/xdiffi.c | 40 ++++++++++++++++------------------------
xdiff/xmerge.c | 47 ++++++++++++++++++++++++++++-------------------
xdiff/xutils.c | 12 ++++++++----
3 files changed, 52 insertions(+), 47 deletions(-)
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 53e803e6bcb..6fded43e87d 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -320,15 +320,11 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
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_PATIENCE_DIFF)
+ return xdl_do_patience_diff(mf1, mf2, xpp, xe);
- if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) {
- res = xdl_do_histogram_diff(mf1, mf2, xpp, xe);
- goto out;
- }
+ if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF)
+ return xdl_do_histogram_diff(mf1, mf2, xpp, xe);
/*
* Allocate and setup K vectors to be used by the differential
@@ -337,11 +333,8 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
* One is to store the forward path and one to store the backward path.
*/
ndiags = xe->xdf1.nreff + xe->xdf2.nreff + 3;
- if (!XDL_ALLOC_ARRAY(kvd, 2 * ndiags + 2)) {
-
- xdl_free_env(xe);
+ if (!XDL_ALLOC_ARRAY(kvd, 2 * ndiags + 2))
return -1;
- }
kvdf = kvd;
kvdb = kvdf + ndiags;
kvdf += xe->xdf2.nreff + 1;
@@ -366,9 +359,6 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
kvdf, kvdb, (xpp->flags & XDF_NEED_MINIMAL) != 0,
&xenv);
xdl_free(kvd);
- out:
- if (res < 0)
- xdl_free_env(xe);
return res;
}
@@ -1054,19 +1044,19 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe,
int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
xdemitconf_t const *xecfg, xdemitcb_t *ecb) {
xdchange_t *xscr;
- xdfenv_t xe;
+ xdfenv_t xe = { 0 };
emit_func_t ef = xecfg->hunk_func ? xdl_call_hunk_func : xdl_emit_diff;
+ int status = 0;
if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0) {
-
- return -1;
+ status = -1;
+ goto cleanup;
}
if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
xdl_change_compact(&xe.xdf2, &xe.xdf1, xpp->flags) < 0 ||
xdl_build_script(&xe, &xscr) < 0) {
-
- xdl_free_env(&xe);
- return -1;
+ status = -1;
+ goto cleanup;
}
if (xscr) {
if (xpp->flags & XDF_IGNORE_BLANK_LINES)
@@ -1078,12 +1068,14 @@ int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
if (ef(&xe, xscr, ecb, xecfg) < 0) {
xdl_free_script(xscr);
- xdl_free_env(&xe);
- return -1;
+ status = -1;
+ goto cleanup;
}
xdl_free_script(xscr);
}
+
+cleanup:
xdl_free_env(&xe);
- return 0;
+ return status;
}
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index af40c88a5b3..ac0cf52f3be 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -365,7 +365,7 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,
{
for (; m; m = m->next) {
mmfile_t t1, t2;
- xdfenv_t xe;
+ xdfenv_t xe = { 0 };
xdchange_t *xscr, *x;
int i1 = m->i1, i2 = m->i2;
@@ -387,8 +387,10 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,
t2.ptr = (char *)xe2->xdf2.recs[m->i2]->ptr;
t2.size = xe2->xdf2.recs[m->i2 + m->chg2 - 1]->ptr
+ xe2->xdf2.recs[m->i2 + m->chg2 - 1]->size - t2.ptr;
- if (xdl_do_diff(&t1, &t2, xpp, &xe) < 0)
+ if (xdl_do_diff(&t1, &t2, xpp, &xe) < 0) {
+ xdl_free_env(&xe);
return -1;
+ }
if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
xdl_change_compact(&xe.xdf2, &xe.xdf1, xpp->flags) < 0 ||
xdl_build_script(&xe, &xscr) < 0) {
@@ -684,30 +686,37 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
xmparam_t const *xmp, mmbuffer_t *result)
{
- xdchange_t *xscr1 = NULL, *xscr2 = NULL;
- xdfenv_t xe1, xe2;
- int status = -1;
+ xdchange_t *xscr1, *xscr2;
+ xdfenv_t xe1 = { 0 };
+ xdfenv_t xe2 = { 0 };
+ int status;
xpparam_t const *xpp = &xmp->xpp;
result->ptr = NULL;
result->size = 0;
- if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0)
- return -1;
-
- if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0)
- goto free_xe1; /* avoid double free of xe2 */
-
+ if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0) {
+ status = -1;
+ goto cleanup;
+ }
+ if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0) {
+ status = -1;
+ goto cleanup;
+ }
if (xdl_change_compact(&xe1.xdf1, &xe1.xdf2, xpp->flags) < 0 ||
xdl_change_compact(&xe1.xdf2, &xe1.xdf1, xpp->flags) < 0 ||
- xdl_build_script(&xe1, &xscr1) < 0)
- goto out;
-
+ xdl_build_script(&xe1, &xscr1) < 0) {
+ status = -1;
+ goto cleanup;
+ }
if (xdl_change_compact(&xe2.xdf1, &xe2.xdf2, xpp->flags) < 0 ||
xdl_change_compact(&xe2.xdf2, &xe2.xdf1, xpp->flags) < 0 ||
- xdl_build_script(&xe2, &xscr2) < 0)
- goto out;
-
+ xdl_build_script(&xe2, &xscr2) < 0) {
+ xdl_free_script(xscr1);
+ status = -1;
+ goto cleanup;
+ }
+ status = 0;
if (!xscr1) {
result->ptr = xdl_malloc(mf2->size);
if (!result->ptr)
@@ -731,9 +740,9 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
xdl_free_script(xscr1);
xdl_free_script(xscr2);
- xdl_free_env(&xe2);
- free_xe1:
+cleanup:
xdl_free_env(&xe1);
+ xdl_free_env(&xe2);
return status;
}
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 9e36f24875d..a6f10353cff 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -414,7 +414,8 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp,
* ranges of lines instead of the whole files.
*/
mmfile_t subfile1, subfile2;
- xdfenv_t env;
+ xdfenv_t env = { 0 };
+ int status = 0;
subfile1.ptr = (char *)diff_env->xdf1.recs[line1 - 1]->ptr;
subfile1.size = diff_env->xdf1.recs[line1 + count1 - 2]->ptr +
@@ -422,15 +423,18 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp,
subfile2.ptr = (char *)diff_env->xdf2.recs[line2 - 1]->ptr;
subfile2.size = diff_env->xdf2.recs[line2 + count2 - 2]->ptr +
diff_env->xdf2.recs[line2 + count2 - 2]->size - subfile2.ptr;
- if (xdl_do_diff(&subfile1, &subfile2, xpp, &env) < 0)
- return -1;
+ if (xdl_do_diff(&subfile1, &subfile2, xpp, &env) < 0) {
+ status = -1;
+ goto cleanup;
+ }
memcpy(diff_env->xdf1.rchg + line1 - 1, env.xdf1.rchg, count1);
memcpy(diff_env->xdf2.rchg + line2 - 1, env.xdf2.rchg, count2);
+cleanup:
xdl_free_env(&env);
- return 0;
+ return status;
}
void* xdl_alloc_grow_helper(void *p, long nr, long *alloc, size_t size)
--
2.37.0.913.g189dca38629
