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

diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index fff0b594f9a851a8e1a6fca961692614ff0c822a..d43abe05b95c458a0df6ce35c8b878c454be7a1c 100644 (file)
--- 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;
 }