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>
Phillip Wood committed
Feb 16, 2022 at 10:15 UTC
4a37b80e88f8f26a2f56d77c5ad6caa41d572ea8
1 file changed
+16
-19
xdiff/xmerge.c
+16
-19
@@ -684,35 +684,30 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
684
int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
685
xmparam_t const *xmp, mmbuffer_t *result)
686
{
687
- xdchange_t *xscr1, *xscr2;
687
+ xdchange_t *xscr1 = NULL, *xscr2 = NULL;
688
xdfenv_t xe1, xe2;
689
- int status;
689
+ int status = -1;
690
xpparam_t const *xpp = &xmp->xpp;
691
692
result->ptr = NULL;
693
result->size = 0;
694
695
- if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0) {
695
+ if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0)
696
return -1;
697
- }
698
- if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0) {
699
- xdl_free_env(&xe1);
700
- return -1;
701
- }
697
+
698
+ if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0)
699
+ goto free_xe1; /* avoid double free of xe2 */
700
+
701
if (xdl_change_compact(&xe1.xdf1, &xe1.xdf2, xpp->flags) < 0 ||
702
xdl_change_compact(&xe1.xdf2, &xe1.xdf1, xpp->flags) < 0 ||
704
- xdl_build_script(&xe1, &xscr1) < 0) {
705
- xdl_free_env(&xe1);
706
- return -1;
707
- }
703
+ xdl_build_script(&xe1, &xscr1) < 0)
704
+ goto out;
705
+
706
if (xdl_change_compact(&xe2.xdf1, &xe2.xdf2, xpp->flags) < 0 ||
707
xdl_change_compact(&xe2.xdf2, &xe2.xdf1, xpp->flags) < 0 ||
710
- xdl_build_script(&xe2, &xscr2) < 0) {
711
- xdl_free_script(xscr1);
712
- xdl_free_env(&xe1);
713
- xdl_free_env(&xe2);
714
- return -1;
715
- }
708
+ xdl_build_script(&xe2, &xscr2) < 0)
709
+ goto out;
710
+
711
status = 0;
712
if (!xscr1) {
713
result->ptr = xdl_malloc(mf2->size);
@@ -727,11 +722,13 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
722
&xe2, xscr2,
723
xmp, result);
724
}
725
+ out:
726
xdl_free_script(xscr1);
727
xdl_free_script(xscr2);
728
733
- xdl_free_env(&xe1);
729
xdl_free_env(&xe2);
730
+ free_xe1:
731
+ xdl_free_env(&xe1);
732
733
return status;
734
}