@matheusbsbs / matheus-bagliano / commits / 763a59e71c
Fork 0

merge-recursive: remove unnecessary oid_eq function

Back when merge-recursive was first introduced in commit 6d297f8137 (Status update on merge-recursive in C, 2006-07-08), it created a sha_eq() function. This function pre-dated the introduction of hashcmp() to cache.h by about a month, but was switched over to using hashcmp() as part of commit 9047ebbc22 (Split out merge_recursive() to merge-recursive.c, 2008-08-12). In commit b4da9d62f9 (merge-recursive: convert leaf functions to use struct object_id, 2016-06-24), sha_eq() was renamed to oid_eq() and its hashcmp() call was switched to oideq(). oid_eq() is basically just a wrapper around oideq() that has some extra checks to protect against NULL arguments or to allow short-circuiting if one of the arguments is NULL. I don't know if any caller ever tried to call with NULL arguments, but certainly none do now which means the extra checks serve no purpose. (Also, if these checks were genuinely useful, then they probably should be added to the main oideq() so all callers could benefit from them.) Reduce the cognitive overhead of having both oid_eq() and oideq(), by getting rid of merge-recursive's special oid_eq() wrapper. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Jan 1, 2020 at 05:20 UTC 763a59e71cee1542665e640f63141a0bf89e6381
1 file changed +11 -22
merge-recursive.c
+11 -22
@@ -224,17 +224,6 @@ static struct commit *make_virtual_commit(struct repository *repo,
224 return commit;
225 }
226
227 -/*
228 - * Since we use get_tree_entry(), which does not put the read object into
229 - * the object pool, we cannot rely on a == b.
230 - */
231 -static int oid_eq(const struct object_id *a, const struct object_id *b)
232 -{
233 - if (!a && !b)
234 - return 2;
235 - return a && b && oideq(a, b);
236 -}
237 -
227 enum rename_type {
228 RENAME_NORMAL = 0,
229 RENAME_VIA_DIR,
@@ -805,7 +794,7 @@ static int was_tracked_and_matches(struct merge_options *opt, const char *path,
794
795 /* See if the file we were tracking before matches */
796 ce = opt->priv->orig_index.cache[pos];
808 - return (oid_eq(&ce->oid, &blob->oid) && ce->ce_mode == blob->mode);
797 + return (oideq(&ce->oid, &blob->oid) && ce->ce_mode == blob->mode);
798 }
799
800 /*
@@ -1317,7 +1306,7 @@ static int merge_mode_and_contents(struct merge_options *opt,
1306 oidcpy(&result->blob.oid, &b->oid);
1307 }
1308 } else {
1320 - if (!oid_eq(&a->oid, &o->oid) && !oid_eq(&b->oid, &o->oid))
1309 + if (!oideq(&a->oid, &o->oid) && !oideq(&b->oid, &o->oid))
1310 result->merge = 1;
1311
1312 /*
@@ -1333,9 +1322,9 @@ static int merge_mode_and_contents(struct merge_options *opt,
1322 }
1323 }
1324
1336 - if (oid_eq(&a->oid, &b->oid) || oid_eq(&a->oid, &o->oid))
1325 + if (oideq(&a->oid, &b->oid) || oideq(&a->oid, &o->oid))
1326 oidcpy(&result->blob.oid, &b->oid);
1338 - else if (oid_eq(&b->oid, &o->oid))
1327 + else if (oideq(&b->oid, &o->oid))
1328 oidcpy(&result->blob.oid, &a->oid);
1329 else if (S_ISREG(a->mode)) {
1330 mmbuffer_t result_buf;
@@ -1368,7 +1357,7 @@ static int merge_mode_and_contents(struct merge_options *opt,
1357 switch (opt->recursive_variant) {
1358 case MERGE_VARIANT_NORMAL:
1359 oidcpy(&result->blob.oid, &a->oid);
1371 - if (!oid_eq(&a->oid, &b->oid))
1360 + if (!oideq(&a->oid, &b->oid))
1361 result->clean = 0;
1362 break;
1363 case MERGE_VARIANT_OURS:
@@ -2778,15 +2767,15 @@ static int process_renames(struct merge_options *opt,
2767 dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
2768 try_merge = 0;
2769
2781 - if (oid_eq(&src_other.oid, &null_oid) &&
2770 + if (oideq(&src_other.oid, &null_oid) &&
2771 ren1->dir_rename_original_type == 'A') {
2772 setup_rename_conflict_info(RENAME_VIA_DIR,
2773 opt, ren1, NULL);
2785 - } else if (oid_eq(&src_other.oid, &null_oid)) {
2774 + } else if (oideq(&src_other.oid, &null_oid)) {
2775 setup_rename_conflict_info(RENAME_DELETE,
2776 opt, ren1, NULL);
2777 } else if ((dst_other.mode == ren1->pair->two->mode) &&
2789 - oid_eq(&dst_other.oid, &ren1->pair->two->oid)) {
2778 + oideq(&dst_other.oid, &ren1->pair->two->oid)) {
2779 /*
2780 * Added file on the other side identical to
2781 * the file being renamed: clean merge.
@@ -2801,7 +2790,7 @@ static int process_renames(struct merge_options *opt,
2790 1, /* update_cache */
2791 0 /* update_wd */))
2792 clean_merge = -1;
2804 - } else if (!oid_eq(&dst_other.oid, &null_oid)) {
2793 + } else if (!oideq(&dst_other.oid, &null_oid)) {
2794 /*
2795 * Probably not a clean merge, but it's
2796 * premature to set clean_merge to 0 here,
@@ -2979,7 +2968,7 @@ static int blob_unchanged(struct merge_options *opt,
2968
2969 if (a->mode != o->mode)
2970 return 0;
2982 - if (oid_eq(&o->oid, &a->oid))
2971 + if (oideq(&o->oid, &a->oid))
2972 return 1;
2973 if (!renormalize)
2974 return 0;
@@ -3420,7 +3409,7 @@ static int merge_trees_internal(struct merge_options *opt,
3409 opt->subtree_shift);
3410 }
3411
3423 - if (oid_eq(&merge_base->object.oid, &merge->object.oid)) {
3412 + if (oideq(&merge_base->object.oid, &merge->object.oid)) {
3413 output(opt, 0, _("Already up to date!"));
3414 *result = head;
3415 return 1;