merge-recursive: handle return values indicating errors
We are about to libify the recursive merge machinery, where we only die() in case of a bug or memory contention. To that end, we must heed negative return values as indicating errors. This requires our functions to be careful to pass through error conditions in call chains, and for quite a few functions this means that they have to return values to begin with. The next step will be to convert the places where we currently die() to return negative values (read: -1) instead. Note that we ignore errors reported by make_room_for_path(), consistent with the previous behavior (update_file_flags() used the return value of make_room_for_path() only to indicate an early return, but not a fatal error): if the error is really a fatal error, we will notice later; If not, it was not that serious a problem to begin with. (Witnesses in favor of this reasoning are t4151-am-abort and t7610-mergetool, which would start failing if we stopped on errors reported by make_room_for_path()). Also note: while this patch makes the code slightly less readable in update_file_flags() (we introduce a new "goto free_buf;" instead of an explicit "free(buf); return;"), it is a preparatory change for the next patch where we will convert all of the die() calls in the same function to go through the free_buf return path instead. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Jul 26, 2016 at 18:06 UTC
75456f96d4bd201f815484e9d2ff2ae429305ab5
1 file changed
+150
-102
merge-recursive.c
+150
-102
@@ -742,12 +742,12 @@ static int make_room_for_path(struct merge_options *o, const char *path)
742
return error(msg, path, _(": perhaps a D/F conflict?"));
743
}
744
745
-static void update_file_flags(struct merge_options *o,
746
- const struct object_id *oid,
747
- unsigned mode,
748
- const char *path,
749
- int update_cache,
750
- int update_wd)
745
+static int update_file_flags(struct merge_options *o,
746
+ const struct object_id *oid,
747
+ unsigned mode,
748
+ const char *path,
749
+ int update_cache,
750
+ int update_wd)
751
{
752
if (o->call_depth)
753
update_wd = 0;
@@ -783,8 +783,7 @@ static void update_file_flags(struct merge_options *o,
783
784
if (make_room_for_path(o, path) < 0) {
785
update_wd = 0;
786
- free(buf);
787
- goto update_index;
786
+ goto free_buf;
787
}
788
if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
789
int fd;
@@ -807,20 +806,22 @@ static void update_file_flags(struct merge_options *o,
806
} else
807
die(_("do not know what to do with %06o %s '%s'"),
808
mode, oid_to_hex(oid), path);
809
+ free_buf:
810
free(buf);
811
}
812
update_index:
813
if (update_cache)
814
add_cacheinfo(mode, oid, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
815
+ return 0;
816
}
817
817
-static void update_file(struct merge_options *o,
818
- int clean,
819
- const struct object_id *oid,
820
- unsigned mode,
821
- const char *path)
818
+static int update_file(struct merge_options *o,
819
+ int clean,
820
+ const struct object_id *oid,
821
+ unsigned mode,
822
+ const char *path)
823
{
823
- update_file_flags(o, oid, mode, path, o->call_depth || clean, !o->call_depth);
824
+ return update_file_flags(o, oid, mode, path, o->call_depth || clean, !o->call_depth);
825
}
826
827
/* Low level file merging, update and removal */
@@ -1019,7 +1020,7 @@ static int merge_file_one(struct merge_options *o,
1020
return merge_file_1(o, &one, &a, &b, branch1, branch2, mfi);
1021
}
1022
1022
-static void handle_change_delete(struct merge_options *o,
1023
+static int handle_change_delete(struct merge_options *o,
1024
const char *path,
1025
const struct object_id *o_oid, int o_mode,
1026
const struct object_id *a_oid, int a_mode,
@@ -1027,6 +1028,7 @@ static void handle_change_delete(struct merge_options *o,
1028
const char *change, const char *change_past)
1029
{
1030
char *renamed = NULL;
1031
+ int ret = 0;
1032
if (dir_in_way(path, !o->call_depth)) {
1033
renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
1034
}
@@ -1037,21 +1039,23 @@ static void handle_change_delete(struct merge_options *o,
1039
* correct; since there is no true "middle point" between
1040
* them, simply reuse the base version for virtual merge base.
1041
*/
1040
- remove_file_from_cache(path);
1041
- update_file(o, 0, o_oid, o_mode, renamed ? renamed : path);
1042
+ ret = remove_file_from_cache(path);
1043
+ if (!ret)
1044
+ ret = update_file(o, 0, o_oid, o_mode,
1045
+ renamed ? renamed : path);
1046
} else if (!a_oid) {
1047
if (!renamed) {
1048
output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1049
"and %s in %s. Version %s of %s left in tree."),
1050
change, path, o->branch1, change_past,
1051
o->branch2, o->branch2, path);
1048
- update_file(o, 0, b_oid, b_mode, path);
1052
+ ret = update_file(o, 0, b_oid, b_mode, path);
1053
} else {
1054
output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
1055
"and %s in %s. Version %s of %s left in tree at %s."),
1056
change, path, o->branch1, change_past,
1057
o->branch2, o->branch2, path, renamed);
1054
- update_file(o, 0, b_oid, b_mode, renamed);
1058
+ ret = update_file(o, 0, b_oid, b_mode, renamed);
1059
}
1060
} else {
1061
if (!renamed) {
@@ -1064,7 +1068,7 @@ static void handle_change_delete(struct merge_options *o,
1068
"and %s in %s. Version %s of %s left in tree at %s."),
1069
change, path, o->branch2, change_past,
1070
o->branch1, o->branch1, path, renamed);
1067
- update_file(o, 0, a_oid, a_mode, renamed);
1071
+ ret = update_file(o, 0, a_oid, a_mode, renamed);
1072
}
1073
/*
1074
* No need to call update_file() on path when !renamed, since
@@ -1074,9 +1078,11 @@ static void handle_change_delete(struct merge_options *o,
1078
*/
1079
}
1080
free(renamed);
1081
+
1082
+ return ret;
1083
}
1084
1079
-static void conflict_rename_delete(struct merge_options *o,
1085
+static int conflict_rename_delete(struct merge_options *o,
1086
struct diff_filepair *pair,
1087
const char *rename_branch,
1088
const char *other_branch)
@@ -1096,21 +1102,20 @@ static void conflict_rename_delete(struct merge_options *o,
1102
b_mode = dest->mode;
1103
}
1104
1099
- handle_change_delete(o,
1100
- o->call_depth ? orig->path : dest->path,
1101
- &orig->oid, orig->mode,
1102
- a_oid, a_mode,
1103
- b_oid, b_mode,
1104
- _("rename"), _("renamed"));
1105
-
1106
- if (o->call_depth) {
1107
- remove_file_from_cache(dest->path);
1108
- } else {
1109
- update_stages(dest->path, NULL,
1110
- rename_branch == o->branch1 ? dest : NULL,
1111
- rename_branch == o->branch1 ? NULL : dest);
1112
- }
1105
+ if (handle_change_delete(o,
1106
+ o->call_depth ? orig->path : dest->path,
1107
+ &orig->oid, orig->mode,
1108
+ a_oid, a_mode,
1109
+ b_oid, b_mode,
1110
+ _("rename"), _("renamed")))
1111
+ return -1;
1112
1113
+ if (o->call_depth)
1114
+ return remove_file_from_cache(dest->path);
1115
+ else
1116
+ return update_stages(dest->path, NULL,
1117
+ rename_branch == o->branch1 ? dest : NULL,
1118
+ rename_branch == o->branch1 ? NULL : dest);
1119
}
1120
1121
static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
@@ -1126,7 +1131,7 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
1131
return target;
1132
}
1133
1129
-static void handle_file(struct merge_options *o,
1134
+static int handle_file(struct merge_options *o,
1135
struct diff_filespec *rename,
1136
int stage,
1137
struct rename_conflict_info *ci)
@@ -1136,6 +1141,7 @@ static void handle_file(struct merge_options *o,
1141
const char *cur_branch, *other_branch;
1142
struct diff_filespec other;
1143
struct diff_filespec *add;
1144
+ int ret;
1145
1146
if (stage == 2) {
1147
dst_entry = ci->dst_entry1;
@@ -1150,7 +1156,8 @@ static void handle_file(struct merge_options *o,
1156
add = filespec_from_entry(&other, dst_entry, stage ^ 1);
1157
if (add) {
1158
char *add_name = unique_path(o, rename->path, other_branch);
1153
- update_file(o, 0, &add->oid, add->mode, add_name);
1159
+ if (update_file(o, 0, &add->oid, add->mode, add_name))
1160
+ return -1;
1161
1162
remove_file(o, 0, rename->path, 0);
1163
dst_name = unique_path(o, rename->path, cur_branch);
@@ -1161,17 +1168,20 @@ static void handle_file(struct merge_options *o,
1168
rename->path, other_branch, dst_name);
1169
}
1170
}
1164
- update_file(o, 0, &rename->oid, rename->mode, dst_name);
1165
- if (stage == 2)
1166
- update_stages(rename->path, NULL, rename, add);
1171
+ if ((ret = update_file(o, 0, &rename->oid, rename->mode, dst_name)))
1172
+ ; /* fall through, do allow dst_name to be released */
1173
+ else if (stage == 2)
1174
+ ret = update_stages(rename->path, NULL, rename, add);
1175
else
1168
- update_stages(rename->path, NULL, add, rename);
1176
+ ret = update_stages(rename->path, NULL, add, rename);
1177
1178
if (dst_name != rename->path)
1179
free(dst_name);
1180
+
1181
+ return ret;
1182
}
1183
1174
-static void conflict_rename_rename_1to2(struct merge_options *o,
1184
+static int conflict_rename_rename_1to2(struct merge_options *o,
1185
struct rename_conflict_info *ci)
1186
{
1187
/* One file was renamed in both branches, but to different names. */
@@ -1194,14 +1204,16 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
1204
&a->oid, a->mode,
1205
&b->oid, b->mode,
1206
ci->branch1, ci->branch2, &mfi))
1197
- return;
1207
+ return -1;
1208
+
1209
/*
1210
* FIXME: For rename/add-source conflicts (if we could detect
1211
* such), this is wrong. We should instead find a unique
1212
* pathname and then either rename the add-source file to that
1213
* unique path, or use that unique path instead of src here.
1214
*/
1204
- update_file(o, 0, &mfi.oid, mfi.mode, one->path);
1215
+ if (update_file(o, 0, &mfi.oid, mfi.mode, one->path))
1216
+ return -1;
1217
1218
/*
1219
* Above, we put the merged content at the merge-base's
@@ -1212,22 +1224,26 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
1224
* resolving the conflict at that path in its favor.
1225
*/
1226
add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1);
1215
- if (add)
1216
- update_file(o, 0, &add->oid, add->mode, a->path);
1227
+ if (add) {
1228
+ if (update_file(o, 0, &add->oid, add->mode, a->path))
1229
+ return -1;
1230
+ }
1231
else
1232
remove_file_from_cache(a->path);
1233
add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1);
1220
- if (add)
1221
- update_file(o, 0, &add->oid, add->mode, b->path);
1234
+ if (add) {
1235
+ if (update_file(o, 0, &add->oid, add->mode, b->path))
1236
+ return -1;
1237
+ }
1238
else
1239
remove_file_from_cache(b->path);
1224
- } else {
1225
- handle_file(o, a, 2, ci);
1226
- handle_file(o, b, 3, ci);
1227
- }
1240
+ } else if (handle_file(o, a, 2, ci) || handle_file(o, b, 3, ci))
1241
+ return -1;
1242
+
1243
+ return 0;
1244
}
1245
1230
-static void conflict_rename_rename_2to1(struct merge_options *o,
1246
+static int conflict_rename_rename_2to1(struct merge_options *o,
1247
struct rename_conflict_info *ci)
1248
{
1249
/* Two files, a & b, were renamed to the same thing, c. */
@@ -1238,6 +1254,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
1254
char *path = c1->path; /* == c2->path */
1255
struct merge_file_info mfi_c1;
1256
struct merge_file_info mfi_c2;
1257
+ int ret;
1258
1259
output(o, 1, _("CONFLICT (rename/rename): "
1260
"Rename %s->%s in %s. "
@@ -1254,7 +1271,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
1271
merge_file_special_markers(o, b, &ci->ren2_other, c2,
1272
o->branch1, ci->ren2_other.path,
1273
o->branch2, c2->path, &mfi_c2))
1257
- return;
1274
+ return -1;
1275
1276
if (o->call_depth) {
1277
/*
@@ -1265,19 +1282,25 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
1282
* again later for the non-recursive merge.
1283
*/
1284
remove_file(o, 0, path, 0);
1268
- update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, a->path);
1269
- update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, b->path);
1285
+ ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, a->path);
1286
+ if (!ret)
1287
+ ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode,
1288
+ b->path);
1289
} else {
1290
char *new_path1 = unique_path(o, path, ci->branch1);
1291
char *new_path2 = unique_path(o, path, ci->branch2);
1292
output(o, 1, _("Renaming %s to %s and %s to %s instead"),
1293
a->path, new_path1, b->path, new_path2);
1294
remove_file(o, 0, path, 0);
1276
- update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, new_path1);
1277
- update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, new_path2);
1295
+ ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, new_path1);
1296
+ if (!ret)
1297
+ ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode,
1298
+ new_path2);
1299
free(new_path2);
1300
free(new_path1);
1301
}
1302
+
1303
+ return ret;
1304
}
1305
1306
static int process_renames(struct merge_options *o,
@@ -1462,12 +1485,13 @@ static int process_renames(struct merge_options *o,
1485
* update_file_flags() instead of
1486
* update_file().
1487
*/
1465
- update_file_flags(o,
1466
- &ren1->pair->two->oid,
1467
- ren1->pair->two->mode,
1468
- ren1_dst,
1469
- 1, /* update_cache */
1470
- 0 /* update_wd */);
1488
+ if (update_file_flags(o,
1489
+ &ren1->pair->two->oid,
1490
+ ren1->pair->two->mode,
1491
+ ren1_dst,
1492
+ 1, /* update_cache */
1493
+ 0 /* update_wd */))
1494
+ clean_merge = -1;
1495
} else if (!oid_eq(&dst_other.oid, &null_oid)) {
1496
clean_merge = 0;
1497
try_merge = 1;
@@ -1482,22 +1506,28 @@ static int process_renames(struct merge_options *o,
1506
ren1->pair->two->mode,
1507
&dst_other.oid,
1508
dst_other.mode,
1485
- branch1, branch2, &mfi))
1486
- return -1;
1509
+ branch1, branch2, &mfi)) {
1510
+ clean_merge = -1;
1511
+ goto cleanup_and_return;
1512
+ }
1513
output(o, 1, _("Adding merged %s"), ren1_dst);
1488
- update_file(o, 0, &mfi.oid,
1489
- mfi.mode, ren1_dst);
1514
+ if (update_file(o, 0, &mfi.oid,
1515
+ mfi.mode, ren1_dst))
1516
+ clean_merge = -1;
1517
try_merge = 0;
1518
} else {
1519
char *new_path = unique_path(o, ren1_dst, branch2);
1520
output(o, 1, _("Adding as %s instead"), new_path);
1494
- update_file(o, 0, &dst_other.oid,
1495
- dst_other.mode, new_path);
1521
+ if (update_file(o, 0, &dst_other.oid,
1522
+ dst_other.mode, new_path))
1523
+ clean_merge = -1;
1524
free(new_path);
1525
}
1526
} else
1527
try_merge = 1;
1528
1529
+ if (clean_merge < 0)
1530
+ goto cleanup_and_return;
1531
if (try_merge) {
1532
struct diff_filespec *one, *a, *b;
1533
src_other.path = (char *)ren1_src;
@@ -1524,6 +1554,7 @@ static int process_renames(struct merge_options *o,
1554
}
1555
}
1556
}
1557
+cleanup_and_return:
1558
string_list_clear(&a_by_dst, 0);
1559
string_list_clear(&b_by_dst, 0);
1560
@@ -1586,18 +1617,18 @@ error_return:
1617
return ret;
1618
}
1619
1589
-static void handle_modify_delete(struct merge_options *o,
1620
+static int handle_modify_delete(struct merge_options *o,
1621
const char *path,
1622
struct object_id *o_oid, int o_mode,
1623
struct object_id *a_oid, int a_mode,
1624
struct object_id *b_oid, int b_mode)
1625
{
1595
- handle_change_delete(o,
1596
- path,
1597
- o_oid, o_mode,
1598
- a_oid, a_mode,
1599
- b_oid, b_mode,
1600
- _("modify"), _("modified"));
1626
+ return handle_change_delete(o,
1627
+ path,
1628
+ o_oid, o_mode,
1629
+ a_oid, a_mode,
1630
+ b_oid, b_mode,
1631
+ _("modify"), _("modified"));
1632
}
1633
1634
static int merge_content(struct merge_options *o,
@@ -1671,7 +1702,8 @@ static int merge_content(struct merge_options *o,
1702
output(o, 1, _("CONFLICT (%s): Merge conflict in %s"),
1703
reason, path);
1704
if (rename_conflict_info && !df_conflict_remains)
1674
- update_stages(path, &one, &a, &b);
1705
+ if (update_stages(path, &one, &a, &b))
1706
+ return -1;
1707
}
1708
1709
if (df_conflict_remains) {
@@ -1679,30 +1711,33 @@ static int merge_content(struct merge_options *o,
1711
if (o->call_depth) {
1712
remove_file_from_cache(path);
1713
} else {
1682
- if (!mfi.clean)
1683
- update_stages(path, &one, &a, &b);
1684
- else {
1714
+ if (!mfi.clean) {
1715
+ if (update_stages(path, &one, &a, &b))
1716
+ return -1;
1717
+ } else {
1718
int file_from_stage2 = was_tracked(path);
1719
struct diff_filespec merged;
1720
oidcpy(&merged.oid, &mfi.oid);
1721
merged.mode = mfi.mode;
1722
1690
- update_stages(path, NULL,
1691
- file_from_stage2 ? &merged : NULL,
1692
- file_from_stage2 ? NULL : &merged);
1723
+ if (update_stages(path, NULL,
1724
+ file_from_stage2 ? &merged : NULL,
1725
+ file_from_stage2 ? NULL : &merged))
1726
+ return -1;
1727
}
1728
1729
}
1730
new_path = unique_path(o, path, rename_conflict_info->branch1);
1731
output(o, 1, _("Adding as %s instead"), new_path);
1698
- update_file(o, 0, &mfi.oid, mfi.mode, new_path);
1732
+ if (update_file(o, 0, &mfi.oid, mfi.mode, new_path)) {
1733
+ free(new_path);
1734
+ return -1;
1735
+ }
1736
free(new_path);
1737
mfi.clean = 0;
1701
- } else {
1702
- update_file(o, mfi.clean, &mfi.oid, mfi.mode, path);
1703
- }
1738
+ } else if (update_file(o, mfi.clean, &mfi.oid, mfi.mode, path))
1739
+ return -1;
1740
return mfi.clean;
1705
-
1741
}
1742
1743
/* Per entry merge function */
@@ -1730,17 +1765,21 @@ static int process_entry(struct merge_options *o,
1765
break;
1766
case RENAME_DELETE:
1767
clean_merge = 0;
1733
- conflict_rename_delete(o, conflict_info->pair1,
1734
- conflict_info->branch1,
1735
- conflict_info->branch2);
1768
+ if (conflict_rename_delete(o,
1769
+ conflict_info->pair1,
1770
+ conflict_info->branch1,
1771
+ conflict_info->branch2))
1772
+ clean_merge = -1;
1773
break;
1774
case RENAME_ONE_FILE_TO_TWO:
1775
clean_merge = 0;
1739
- conflict_rename_rename_1to2(o, conflict_info);
1776
+ if (conflict_rename_rename_1to2(o, conflict_info))
1777
+ clean_merge = -1;
1778
break;
1779
case RENAME_TWO_FILES_TO_ONE:
1780
clean_merge = 0;
1743
- conflict_rename_rename_2to1(o, conflict_info);
1781
+ if (conflict_rename_rename_2to1(o, conflict_info))
1782
+ clean_merge = -1;
1783
break;
1784
default:
1785
entry->processed = 0;
@@ -1760,8 +1799,9 @@ static int process_entry(struct merge_options *o,
1799
} else {
1800
/* Modify/delete; deleted side may have put a directory in the way */
1801
clean_merge = 0;
1763
- handle_modify_delete(o, path, o_oid, o_mode,
1764
- a_oid, a_mode, b_oid, b_mode);
1802
+ if (handle_modify_delete(o, path, o_oid, o_mode,
1803
+ a_oid, a_mode, b_oid, b_mode))
1804
+ clean_merge = -1;
1805
}
1806
} else if ((!o_oid && a_oid && !b_oid) ||
1807
(!o_oid && !a_oid && b_oid)) {
@@ -1793,14 +1833,16 @@ static int process_entry(struct merge_options *o,
1833
output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
1834
"Adding %s as %s"),
1835
conf, path, other_branch, path, new_path);
1796
- update_file(o, 0, oid, mode, new_path);
1797
- if (o->call_depth)
1836
+ if (update_file(o, 0, oid, mode, new_path))
1837
+ clean_merge = -1;
1838
+ else if (o->call_depth)
1839
remove_file_from_cache(path);
1840
free(new_path);
1841
} else {
1842
output(o, 2, _("Adding %s"), path);
1843
/* do not overwrite file if already present */
1803
- update_file_flags(o, oid, mode, path, 1, !a_oid);
1844
+ if (update_file_flags(o, oid, mode, path, 1, !a_oid))
1845
+ clean_merge = -1;
1846
}
1847
} else if (a_oid && b_oid) {
1848
/* Case C: Added in both (check for same permissions) and */
@@ -1863,12 +1905,18 @@ int merge_trees(struct merge_options *o,
1905
re_head = get_renames(o, head, common, head, merge, entries);
1906
re_merge = get_renames(o, merge, common, head, merge, entries);
1907
clean = process_renames(o, re_head, re_merge);
1908
+ if (clean < 0)
1909
+ return clean;
1910
for (i = entries->nr-1; 0 <= i; i--) {
1911
const char *path = entries->items[i].string;
1912
struct stage_data *e = entries->items[i].util;
1869
- if (!e->processed
1870
- && !process_entry(o, path, e))
1871
- clean = 0;
1913
+ if (!e->processed) {
1914
+ int ret = process_entry(o, path, e);
1915
+ if (!ret)
1916
+ clean = 0;
1917
+ else if (ret < 0)
1918
+ return ret;
1919
+ }
1920
}
1921
for (i = 0; i < entries->nr; i++) {
1922
struct stage_data *e = entries->items[i].util;