From 4a37b80e88f8f26a2f56d77c5ad6caa41d572ea8 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Wed, 16 Feb 2022 10:15:08 +0000 Subject: [PATCH] xdiff: refactor a function Use the standard "goto out" pattern rather than repeating very similar code after checking for each error. This will simplify the next commit that starts handling allocation failures that are currently ignored. On error xdl_do_diff() frees the environment so we need to take care to avoid a double free in that case. xdl_build_script() does not assign a result unless it is successful so there is no possibility of a double free if it fails. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- xdiff/xmerge.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c index fff0b594f9..d43abe05b9 100644 --- a/xdiff/xmerge.c +++ b/xdiff/xmerge.c @@ -684,35 +684,30 @@ 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, *xscr2; + xdchange_t *xscr1 = NULL, *xscr2 = NULL; xdfenv_t xe1, xe2; - int status; + int status = -1; xpparam_t const *xpp = &xmp->xpp; result->ptr = NULL; result->size = 0; - if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0) { + if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0) return -1; - } - if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0) { - xdl_free_env(&xe1); - return -1; - } + + if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0) + goto free_xe1; /* avoid double free of xe2 */ + 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) { - xdl_free_env(&xe1); - return -1; - } + xdl_build_script(&xe1, &xscr1) < 0) + goto out; + 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) { - xdl_free_script(xscr1); - xdl_free_env(&xe1); - xdl_free_env(&xe2); - return -1; - } + xdl_build_script(&xe2, &xscr2) < 0) + goto out; + status = 0; if (!xscr1) { result->ptr = xdl_malloc(mf2->size); @@ -727,11 +722,13 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2, &xe2, xscr2, xmp, result); } + out: xdl_free_script(xscr1); xdl_free_script(xscr2); - xdl_free_env(&xe1); xdl_free_env(&xe2); + free_xe1: + xdl_free_env(&xe1); return status; }