From 4d168e742a39b5e647ab8d2408cd5c0062ac35f4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:32 -0400 Subject: coccinelle: use <...> for function exclusion Sometimes we want to suppress a coccinelle transformation inside a particular function. For example, in finding conversions of hashcmp() to oidcmp(), we should not convert the call in oidcmp() itself, since that would cause infinite recursion. We write that like this: @@ identifier f != oidcmp; expression E1, E2; @@ f(...) {... - hashcmp(E1->hash, E2->hash) + oidcmp(E1, E2) ...} to match the interior of any function _except_ oidcmp(). Unfortunately, this doesn't catch all cases (e.g., the one in sequencer.c that this patch fixes). The problem, as explained by one of the Coccinelle developers in [1], is: For transformation, A ... B requires that B occur on every execution path starting with A, unless that execution path ends up in error handling code. (eg, if (...) { ... return; }). Here your A is the start of the function. So you need a call to hashcmp on every path through the function, which fails when you add ifs. [...] Another issue with A ... B is that by default A and B should not appear in the matched region. So your original rule matches only the case where every execution path contains exactly one call to hashcmp, not more than one. One way to solve this is to put the pattern inside an angle-bracket pattern like "<... P ...>", which allows zero or more matches of P. That works (and is what this patch does), but it has one drawback: it matches more than we care about, and Coccinelle uses extra CPU. Here are timings for "make coccicheck" before and after this patch: [before] real 1m27.122s user 7m34.451s sys 0m37.330s [after] real 2m18.040s user 10m58.310s sys 0m41.549s That's not ideal, but it's more important for this to be correct than to be fast. And coccicheck is already fairly slow (and people don't run it for every single patch). So it's an acceptable tradeoff. There _is_ a better way to do it, which is to record the position at which we find hashcmp(), and then check it against the forbidden function list. Like: @@ position p : script:python() { p[0].current_element != "oidcmp" }; expression E1,E2; @@ - hashcmp@p(E1->hash, E2->hash) + oidcmp(E1, E2) This is only a little slower than the current code, and does the right thing in all cases. Unfortunately, not all builds of Coccinelle include python support (including the ones in Debian). Requiring it may mean that fewer people can easily run the tool, which is worse than it simply being a little slower. We may want to revisit this decision in the future if: - builds with python become more common - we find more uses for python support that tip the cost-benefit analysis But for now this patch sticks with the angle-bracket solution, and converts all existing cocci patches. This fixes only one missed case in the current code, though it makes a much better difference for some new rules I'm adding (converting "!hashcmp()" to "hasheq()" misses over half the possible conversions using the old form). [1] https://public-inbox.org/git/alpine.DEB.2.21.1808240652370.2344@hadrien/ Helped-by: Julia Lawall Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/contrib/coccinelle/commit.cocci b/contrib/coccinelle/commit.cocci index aec3345..c49aa55 100644 --- a/contrib/coccinelle/commit.cocci +++ b/contrib/coccinelle/commit.cocci @@ -15,10 +15,10 @@ expression c; identifier f !~ "^(get_commit_tree|get_commit_tree_in_graph_one|load_tree_for_commit)$"; expression c; @@ - f(...) {... + f(...) {<... - c->maybe_tree + get_commit_tree(c) - ...} + ...>} @@ expression c; diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci index 09afdbf..5869979 100644 --- a/contrib/coccinelle/object_id.cocci +++ b/contrib/coccinelle/object_id.cocci @@ -20,10 +20,10 @@ expression E1; identifier f != oid_to_hex; expression E1; @@ - f(...) {... + f(...) {<... - sha1_to_hex(E1->hash) + oid_to_hex(E1) - ...} + ...>} @@ expression E1, E2; @@ -35,10 +35,10 @@ expression E1, E2; identifier f != oid_to_hex_r; expression E1, E2; @@ - f(...) {... + f(...) {<... - sha1_to_hex_r(E1, E2->hash) + oid_to_hex_r(E1, E2) - ...} + ...>} @@ expression E1; @@ -50,10 +50,10 @@ expression E1; identifier f != oidclr; expression E1; @@ - f(...) {... + f(...) {<... - hashclr(E1->hash) + oidclr(E1) - ...} + ...>} @@ expression E1, E2; @@ -65,10 +65,10 @@ expression E1, E2; identifier f != oidcmp; expression E1, E2; @@ - f(...) {... + f(...) {<... - hashcmp(E1->hash, E2->hash) + oidcmp(E1, E2) - ...} + ...>} @@ expression E1, E2; @@ -92,10 +92,10 @@ expression E1, E2; identifier f != oidcpy; expression E1, E2; @@ - f(...) {... + f(...) {<... - hashcpy(E1->hash, E2->hash) + oidcpy(E1, E2) - ...} + ...>} @@ expression E1, E2; diff --git a/sequencer.c b/sequencer.c index 84bf598..305de74 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4545,7 +4545,7 @@ int skip_unnecessary_picks(void) if (item->commit->parents->next) break; /* merge commit */ parent_oid = &item->commit->parents->item->object.oid; - if (hashcmp(parent_oid->hash, oid->hash)) + if (oidcmp(parent_oid, oid)) break; oid = &item->commit->object.oid; } -- cgit v0.10.2-6-g49f6 From 14438c4497c3ab3988cf50ebd504acef3735953c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:35 -0400 Subject: introduce hasheq() and oideq() The main comparison functions we provide for comparing object ids are hashcmp() and oidcmp(). These are more flexible than a strict equality check, since they also express ordering. That makes them useful for sorting and binary searching. However, it also makes them potentially slower than a strict equality check. Consider this C code, which is traditionally what our hashcmp has looked like: #include int hashcmp(const unsigned char *a, const unsigned char *b) { return memcmp(a, b, 20); } Compiling with "gcc -O2 -S -fverbose-asm", the generated assembly shows that we actually call memcmp(). But if we change this to a strict equality check: return !memcmp(a, b, 20); we get a faster inline version: movq (%rdi), %rax # MEM[(void *)a_4(D)], MEM[(void *)a_4(D)] movq 8(%rdi), %rdx # MEM[(void *)a_4(D)], tmp101 xorq (%rsi), %rax # MEM[(void *)b_5(D)], tmp94 xorq 8(%rsi), %rdx # MEM[(void *)b_5(D)], tmp93 orq %rax, %rdx # tmp94, tmp93 jne .L2 #, movl 16(%rsi), %eax # MEM[(void *)b_5(D)], tmp104 cmpl %eax, 16(%rdi) # tmp104, MEM[(void *)a_4(D)] je .L5 #, Obviously our hashcmp() doesn't include the "!". But because it's an inline function, optimizing compilers are able to see "!hashcmp(a,b)" in calling code and take advantage of this case. So there has been no value thus far in introducing a more restricted interface for doing strict equality checks. But as Git learns about more values for the_hash_algo, our hashcmp() will grow more complicated and may even delegate at runtime to functions optimized specifically for that hash size. That breaks the inline connection we have, and the compiler will have to assume that the caller really cares about the sign and magnitude of the memcmp() result, even though the vast majority don't. We can solve that by introducing a hasheq() function (and matching oideq() wrapper), which callers can use to make it clear that they only care about equality. For now, the implementation will literally be "!hashcmp()", but it frees us up later to introduce code optimized specifically for the equality check. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 4d01454..f6d227f 100644 --- a/cache.h +++ b/cache.h @@ -1041,6 +1041,16 @@ static inline int oidcmp(const struct object_id *oid1, const struct object_id *o return hashcmp(oid1->hash, oid2->hash); } +static inline int hasheq(const unsigned char *sha1, const unsigned char *sha2) +{ + return !hashcmp(sha1, sha2); +} + +static inline int oideq(const struct object_id *oid1, const struct object_id *oid2) +{ + return hasheq(oid1->hash, oid2->hash); +} + static inline int is_null_sha1(const unsigned char *sha1) { return !hashcmp(sha1, null_sha1); -- cgit v0.10.2-6-g49f6 From 4a7e27e95797c0a094f8ee300a260777ddd7eec9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:40 -0400 Subject: convert "oidcmp() == 0" to oideq() Using the more restrictive oideq() should, in the long run, give the compiler more opportunities to optimize these callsites. For now, this conversion should be a complete noop with respect to the generated code. The result is also perhaps a little more readable, as it avoids the "zero is equal" idiom. Since it's so prevalent in C, I think seasoned programmers tend not to even notice it anymore, but it can sometimes make for awkward double negations (e.g., we can drop a few !!oidcmp() instances here). This patch was generated almost entirely by the included coccinelle patch. This mechanical conversion should be completely safe, because we check explicitly for cases where oidcmp() is compared to 0, which is what oideq() is doing under the hood. Note that we don't have to catch "!oidcmp()" separately; coccinelle's standard isomorphisms make sure the two are treated equivalently. I say "almost" because I did hand-edit the coccinelle output to fix up a few style violations (it mostly keeps the original formatting, but sometimes unwraps long lines). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/bisect.c b/bisect.c index e1275ba..41c56a6 100644 --- a/bisect.c +++ b/bisect.c @@ -807,7 +807,7 @@ static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout) for (; result; result = result->next) { const struct object_id *mb = &result->item->object.oid; - if (!oidcmp(mb, current_bad_oid)) { + if (oideq(mb, current_bad_oid)) { handle_bad_merge_base(); } else if (0 <= oid_array_lookup(&good_revs, mb)) { continue; @@ -988,7 +988,7 @@ int bisect_next_all(const char *prefix, int no_checkout) bisect_rev = &revs.commits->item->object.oid; - if (!oidcmp(bisect_rev, current_bad_oid)) { + if (oideq(bisect_rev, current_bad_oid)) { exit_if_skipped_commits(tried, current_bad_oid); printf("%s is the first %s commit\n", oid_to_hex(bisect_rev), term_bad); diff --git a/blame.c b/blame.c index aca06f4..c47d705 100644 --- a/blame.c +++ b/blame.c @@ -1457,14 +1457,14 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin, porigin = find(p, origin); if (!porigin) continue; - if (!oidcmp(&porigin->blob_oid, &origin->blob_oid)) { + if (oideq(&porigin->blob_oid, &origin->blob_oid)) { pass_whole_blame(sb, origin, porigin); blame_origin_decref(porigin); goto finish; } for (j = same = 0; j < i; j++) if (sg_origin[j] && - !oidcmp(&sg_origin[j]->blob_oid, &porigin->blob_oid)) { + oideq(&sg_origin[j]->blob_oid, &porigin->blob_oid)) { same = 1; break; } diff --git a/builtin/am.c b/builtin/am.c index 9f7ecf6..e54110d 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2077,7 +2077,7 @@ static int safe_to_abort(const struct am_state *state) if (get_oid("HEAD", &head)) oidclr(&head); - if (!oidcmp(&head, &abort_safety)) + if (oideq(&head, &abort_safety)) return 1; warning(_("You seem to have moved HEAD since the last 'am' failure.\n" diff --git a/builtin/checkout.c b/builtin/checkout.c index 29ef500..082e3a9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -96,7 +96,7 @@ static int update_some(const struct object_id *oid, struct strbuf *base, if (pos >= 0) { struct cache_entry *old = active_cache[pos]; if (ce->ce_mode == old->ce_mode && - !oidcmp(&ce->oid, &old->oid)) { + oideq(&ce->oid, &old->oid)) { old->ce_flags |= CE_UPDATE; discard_cache_entry(ce); return 0; diff --git a/builtin/describe.c b/builtin/describe.c index 41606c8..1e7c75b 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -190,7 +190,7 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi /* Is it annotated? */ if (!peel_ref(path, &peeled)) { - is_annotated = !!oidcmp(oid, &peeled); + is_annotated = !oideq(oid, &peeled); } else { oidcpy(&peeled, oid); is_annotated = 0; @@ -469,7 +469,7 @@ static void process_object(struct object *obj, const char *path, void *data) { struct process_commit_data *pcd = data; - if (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) { + if (oideq(&pcd->looking_for, &obj->oid) && !pcd->dst->len) { reset_revision_walk(); describe_commit(&pcd->current_commit, pcd->dst); strbuf_addf(pcd->dst, ":%s", path); diff --git a/builtin/diff.c b/builtin/diff.c index 361a3c3..b3a8ba4 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -41,7 +41,7 @@ static void stuff_change(struct diff_options *opt, struct diff_filespec *one, *two; if (!is_null_oid(old_oid) && !is_null_oid(new_oid) && - !oidcmp(old_oid, new_oid) && (old_mode == new_mode)) + oideq(old_oid, new_oid) && (old_mode == new_mode)) return; if (opt->flags.reverse_diff) { diff --git a/builtin/difftool.c b/builtin/difftool.c index cdd585c..b41a919 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -116,7 +116,7 @@ static int use_wt_file(const char *workdir, const char *name, if (is_null_oid(oid)) { oidcpy(oid, &wt_oid); use = 1; - } else if (!oidcmp(oid, &wt_oid)) + } else if (oideq(oid, &wt_oid)) use = 1; } } @@ -438,7 +438,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, strbuf_reset(&buf); strbuf_addf(&buf, "Subproject commit %s", oid_to_hex(&roid)); - if (!oidcmp(&loid, &roid)) + if (oideq(&loid, &roid)) strbuf_addstr(&buf, "-dirty"); add_left_or_right(&submodules, dst_path, buf.buf, 1); continue; diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 9bd8a14..74f3bf5 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -384,7 +384,7 @@ static void show_filemodify(struct diff_queue_struct *q, string_list_insert(changed, spec->path); putchar('\n'); - if (!oidcmp(&ospec->oid, &spec->oid) && + if (oideq(&ospec->oid, &spec->oid) && ospec->mode == spec->mode) break; } diff --git a/builtin/fetch.c b/builtin/fetch.c index 61bec5d..32b1d5d 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -507,7 +507,7 @@ static void adjust_refcol_width(const struct ref *ref) int max, rlen, llen, len; /* uptodate lines are only shown on high verbosity level */ - if (!verbosity && !oidcmp(&ref->peer_ref->old_oid, &ref->old_oid)) + if (!verbosity && oideq(&ref->peer_ref->old_oid, &ref->old_oid)) return; max = term_columns(); @@ -644,7 +644,7 @@ static int update_local_ref(struct ref *ref, if (type < 0) die(_("object %s not found"), oid_to_hex(&ref->new_oid)); - if (!oidcmp(&ref->old_oid, &ref->new_oid)) { + if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) format_display(display, '=', _("[up to date]"), NULL, remote, pretty_ref, summary_width); diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index f35ff16..4c82c23 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -582,7 +582,7 @@ static void find_merge_parents(struct merge_parents *result, while (parents) { struct commit *cmit = pop_commit(&parents); for (i = 0; i < result->nr; i++) - if (!oidcmp(&result->item[i].commit, &cmit->object.oid)) + if (oideq(&result->item[i].commit, &cmit->object.oid)) result->item[i].used = 1; } diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 9582ead..edcb0a3 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -719,9 +719,9 @@ static void find_ref_delta_children(const struct object_id *oid, *last_index = -1; return; } - while (first > 0 && !oidcmp(&ref_deltas[first - 1].oid, oid)) + while (first > 0 && oideq(&ref_deltas[first - 1].oid, oid)) --first; - while (last < end && !oidcmp(&ref_deltas[last + 1].oid, oid)) + while (last < end && oideq(&ref_deltas[last + 1].oid, oid)) ++last; *first_index = first; *last_index = last; diff --git a/builtin/log.c b/builtin/log.c index e094560..98d668b 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -992,7 +992,7 @@ static char *find_branch_name(struct rev_info *rev) tip_oid = &rev->cmdline.rev[positive].item->oid; if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref) && skip_prefix(full_ref, "refs/heads/", &v) && - !oidcmp(tip_oid, &branch_oid)) + oideq(tip_oid, &branch_oid)) branch = xstrdup(v); free(full_ref); return branch; @@ -1703,7 +1703,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) /* Don't say anything if head and upstream are the same. */ if (rev.pending.nr == 2) { struct object_array_entry *o = rev.pending.objects; - if (oidcmp(&o[0].item->oid, &o[1].item->oid) == 0) + if (oideq(&o[0].item->oid, &o[1].item->oid)) return 0; } get_patch_ids(&rev, &ids); @@ -1949,7 +1949,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix) /* Don't say anything if head and upstream are the same. */ if (revs.pending.nr == 2) { struct object_array_entry *o = revs.pending.objects; - if (oidcmp(&o[0].item->oid, &o[1].item->oid) == 0) + if (oideq(&o[0].item->oid, &o[1].item->oid)) return 0; } diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index f8023ba..8cea8a7 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -155,7 +155,7 @@ static int same_entry(struct name_entry *a, struct name_entry *b) { return a->oid && b->oid && - !oidcmp(a->oid, b->oid) && + oideq(a->oid, b->oid) && a->mode == b->mode; } diff --git a/builtin/merge.c b/builtin/merge.c index 8f4a506..57abff9 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1189,7 +1189,7 @@ static int merging_a_throwaway_tag(struct commit *commit) tag_ref = xstrfmt("refs/tags/%s", ((struct tag *)merge_remote_util(commit)->obj)->tag); if (!read_ref(tag_ref, &oid) && - !oidcmp(&oid, &merge_remote_util(commit)->obj->oid)) + oideq(&oid, &merge_remote_util(commit)->obj->oid)) is_throwaway_tag = 0; else is_throwaway_tag = 1; @@ -1448,7 +1448,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) goto done; } else if (fast_forward != FF_NO && !remoteheads->next && !common->next && - !oidcmp(&common->item->object.oid, &head_commit->object.oid)) { + oideq(&common->item->object.oid, &head_commit->object.oid)) { /* Again the most common case of merging one remote. */ struct strbuf msg = STRBUF_INIT; struct commit *commit; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index d1144a8..64156f6 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1202,7 +1202,7 @@ static struct pbase_tree_cache *pbase_tree_get(const struct object_id *oid) */ for (neigh = 0; neigh < 8; neigh++) { ent = pbase_tree_cache[my_ix]; - if (ent && !oidcmp(&ent->oid, oid)) { + if (ent && oideq(&ent->oid, oid)) { ent->ref++; return ent; } @@ -1384,7 +1384,7 @@ static void add_preferred_base(struct object_id *oid) return; for (it = pbase_tree; it; it = it->next) { - if (!oidcmp(&it->pcache.oid, &tree_oid)) { + if (oideq(&it->pcache.oid, &tree_oid)) { free(data); return; } diff --git a/builtin/pull.c b/builtin/pull.c index 681c127..a720f58 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -799,7 +799,7 @@ static int run_rebase(const struct object_id *curr_head, struct argv_array args = ARGV_ARRAY_INIT; if (!get_octopus_merge_base(&oct_merge_base, curr_head, merge_head, fork_point)) - if (!is_null_oid(fork_point) && !oidcmp(&oct_merge_base, fork_point)) + if (!is_null_oid(fork_point) && oideq(&oct_merge_base, fork_point)) fork_point = NULL; argv_array_push(&args, "rebase"); diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c17ce94..5bb163d 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1222,8 +1222,8 @@ static void check_aliased_update(struct command *cmd, struct string_list *list) dst_cmd = (struct command *) item->util; - if (!oidcmp(&cmd->old_oid, &dst_cmd->old_oid) && - !oidcmp(&cmd->new_oid, &dst_cmd->new_oid)) + if (oideq(&cmd->old_oid, &dst_cmd->old_oid) && + oideq(&cmd->new_oid, &dst_cmd->new_oid)) return; dst_cmd->skip_update = 1; diff --git a/builtin/remote.c b/builtin/remote.c index 7876db1..c171323 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -412,7 +412,7 @@ static int get_push_ref_states(const struct ref *remote_refs, if (is_null_oid(&ref->new_oid)) { info->status = PUSH_STATUS_DELETE; - } else if (!oidcmp(&ref->old_oid, &ref->new_oid)) + } else if (oideq(&ref->old_oid, &ref->new_oid)) info->status = PUSH_STATUS_UPTODATE; else if (is_null_oid(&ref->old_oid)) info->status = PUSH_STATUS_CREATE; diff --git a/builtin/replace.c b/builtin/replace.c index 4f05791..8e67e09 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -343,7 +343,7 @@ static int edit_and_replace(const char *object_ref, int force, int raw) } free(tmpfile); - if (!oidcmp(&old_oid, &new_oid)) + if (oideq(&old_oid, &new_oid)) return error(_("new object is the same as the old one: '%s'"), oid_to_hex(&old_oid)); return replace_object_oid(object_ref, &old_oid, "replacement", &new_oid, force); @@ -414,7 +414,7 @@ static int check_one_mergetag(struct commit *commit, if (get_oid(mergetag_data->argv[i], &oid) < 0) return error(_("not a valid object name: '%s'"), mergetag_data->argv[i]); - if (!oidcmp(&tag->tagged->oid, &oid)) + if (oideq(&tag->tagged->oid, &oid)) return 0; /* found */ } @@ -474,7 +474,7 @@ static int create_graft(int argc, const char **argv, int force, int gentle) strbuf_release(&buf); - if (!oidcmp(&old_oid, &new_oid)) { + if (oideq(&old_oid, &new_oid)) { if (gentle) { warning(_("graft for '%s' unnecessary"), oid_to_hex(&old_oid)); return 0; diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 30d9413..ad438f5 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -303,7 +303,7 @@ static void added_object(unsigned nr, enum object_type type, struct delta_info *info; while ((info = *p) != NULL) { - if (!oidcmp(&info->base_oid, &obj_list[nr].oid) || + if (oideq(&info->base_oid, &obj_list[nr].oid) || info->base_offset == obj_list[nr].offset) { *p = info->next; p = &delta_list; diff --git a/builtin/update-index.c b/builtin/update-index.c index fe84003..e7fab78 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -669,7 +669,7 @@ static int unresolve_one(const char *path) ret = -1; goto free_return; } - if (!oidcmp(&ce_2->oid, &ce_3->oid) && + if (oideq(&ce_2->oid, &ce_3->oid) && ce_2->ce_mode == ce_3->ce_mode) { fprintf(stderr, "%s: identical in both, skipping.\n", path); @@ -754,7 +754,7 @@ static int do_reupdate(int ac, const char **av, old = read_one_ent(NULL, &head_oid, ce->name, ce_namelen(ce), 0); if (old && ce->ce_mode == old->ce_mode && - !oidcmp(&ce->oid, &old->oid)) { + oideq(&ce->oid, &old->oid)) { discard_cache_entry(old); continue; /* unchanged */ } diff --git a/bulk-checkin.c b/bulk-checkin.c index 9f3b644..409ecb5 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c @@ -72,7 +72,7 @@ static int already_written(struct bulk_checkin_state *state, struct object_id *o /* Might want to keep the list sorted */ for (i = 0; i < state->nr_written; i++) - if (!oidcmp(&state->written[i]->oid, oid)) + if (oideq(&state->written[i]->oid, oid)) return 1; /* This is a new object we need to keep */ diff --git a/cache-tree.c b/cache-tree.c index 16ea022..b49bb5c 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -714,7 +714,7 @@ int cache_tree_matches_traversal(struct cache_tree *root, it = find_cache_tree_from_traversal(root, info); it = cache_tree_find(it, ent->path); - if (it && it->entry_count > 0 && !oidcmp(ent->oid, &it->oid)) + if (it && it->entry_count > 0 && oideq(ent->oid, &it->oid)) return it->entry_count; return 0; } diff --git a/cache.h b/cache.h index f6d227f..d090f71 100644 --- a/cache.h +++ b/cache.h @@ -1100,7 +1100,7 @@ static inline int is_empty_blob_sha1(const unsigned char *sha1) static inline int is_empty_blob_oid(const struct object_id *oid) { - return !oidcmp(oid, the_hash_algo->empty_blob); + return oideq(oid, the_hash_algo->empty_blob); } static inline int is_empty_tree_sha1(const unsigned char *sha1) @@ -1110,7 +1110,7 @@ static inline int is_empty_tree_sha1(const unsigned char *sha1) static inline int is_empty_tree_oid(const struct object_id *oid) { - return !oidcmp(oid, the_hash_algo->empty_tree); + return oideq(oid, the_hash_algo->empty_tree); } const char *empty_tree_oid_hex(void); diff --git a/combine-diff.c b/combine-diff.c index de7695e..0fed4ca 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1138,8 +1138,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, for (i = 0; i < num_parent; i++) { int j; for (j = 0; j < i; j++) { - if (!oidcmp(&elem->parent[i].oid, - &elem->parent[j].oid)) { + if (oideq(&elem->parent[i].oid, + &elem->parent[j].oid)) { reuse_combine_diff(sline, cnt, i, j); break; } diff --git a/commit-graph.c b/commit-graph.c index 8a1bec7..050c516 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -779,7 +779,7 @@ void write_commit_graph(const char *obj_dir, num_extra_edges = 0; for (i = 0; i < oids.nr; i++) { int num_parents = 0; - if (i > 0 && !oidcmp(&oids.list[i-1], &oids.list[i])) + if (i > 0 && oideq(&oids.list[i - 1], &oids.list[i])) continue; commits.list[commits.nr] = lookup_commit(the_repository, &oids.list[i]); diff --git a/connect.c b/connect.c index 94547e5..24281b6 100644 --- a/connect.c +++ b/connect.c @@ -224,7 +224,7 @@ static int process_dummy_ref(const char *line) return 0; name++; - return !oidcmp(&null_oid, &oid) && !strcmp(name, "capabilities^{}"); + return oideq(&null_oid, &oid) && !strcmp(name, "capabilities^{}"); } static void check_no_capabilities(const char *line, int len) diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci index 5869979..548c023 100644 --- a/contrib/coccinelle/object_id.cocci +++ b/contrib/coccinelle/object_id.cocci @@ -108,3 +108,9 @@ expression E1, E2; @@ - hashcpy(E1.hash, E2->hash) + oidcpy(&E1, E2) + +@@ +expression E1, E2; +@@ +- oidcmp(E1, E2) == 0 ++ oideq(E1, E2) diff --git a/diff-lib.c b/diff-lib.c index 88a98b1..c1f5a52 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -366,7 +366,7 @@ static int show_modified(struct rev_info *revs, } oldmode = old_entry->ce_mode; - if (mode == oldmode && !oidcmp(oid, &old_entry->oid) && !dirty_submodule && + if (mode == oldmode && oideq(oid, &old_entry->oid) && !dirty_submodule && !revs->diffopt.flags.find_copies_harder) return 0; diff --git a/diff.c b/diff.c index 145cfba..5cada68 100644 --- a/diff.c +++ b/diff.c @@ -3404,7 +3404,7 @@ static void builtin_diff(const char *name_a, if (!one->data && !two->data && S_ISREG(one->mode) && S_ISREG(two->mode) && !o->flags.binary) { - if (!oidcmp(&one->oid, &two->oid)) { + if (oideq(&one->oid, &two->oid)) { if (must_show_header) emit_diff_symbol(o, DIFF_SYMBOL_HEADER, header.buf, header.len, @@ -3569,7 +3569,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b, return; } - same_contents = !oidcmp(&one->oid, &two->oid); + same_contents = oideq(&one->oid, &two->oid); if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) { data->is_binary = 1; @@ -5323,7 +5323,7 @@ int diff_unmodified_pair(struct diff_filepair *p) * dealing with a change. */ if (one->oid_valid && two->oid_valid && - !oidcmp(&one->oid, &two->oid) && + oideq(&one->oid, &two->oid) && !one->dirty_submodule && !two->dirty_submodule) return 1; /* no change */ if (!one->oid_valid && !two->oid_valid) diff --git a/diffcore-break.c b/diffcore-break.c index c64359f..e11fcfd 100644 --- a/diffcore-break.c +++ b/diffcore-break.c @@ -58,7 +58,7 @@ static int should_break(struct diff_filespec *src, } if (src->oid_valid && dst->oid_valid && - !oidcmp(&src->oid, &dst->oid)) + oideq(&src->oid, &dst->oid)) return 0; /* they are the same */ if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0)) diff --git a/fast-import.c b/fast-import.c index 89bb0c9..a731088 100644 --- a/fast-import.c +++ b/fast-import.c @@ -572,7 +572,7 @@ static struct object_entry *find_object(struct object_id *oid) unsigned int h = oid->hash[0] << 8 | oid->hash[1]; struct object_entry *e; for (e = object_table[h]; e; e = e->next) - if (!oidcmp(oid, &e->idx.oid)) + if (oideq(oid, &e->idx.oid)) return e; return NULL; } @@ -583,7 +583,7 @@ static struct object_entry *insert_object(struct object_id *oid) struct object_entry *e = object_table[h]; while (e) { - if (!oidcmp(oid, &e->idx.oid)) + if (oideq(oid, &e->idx.oid)) return e; e = e->next; } @@ -1533,7 +1533,7 @@ static int tree_content_set( if (!*slash1) { if (!S_ISDIR(mode) && e->versions[1].mode == mode - && !oidcmp(&e->versions[1].oid, oid)) + && oideq(&e->versions[1].oid, oid)) return 0; e->versions[1].mode = mode; oidcpy(&e->versions[1].oid, oid); diff --git a/http-push.c b/http-push.c index 5eaf551..283495c 100644 --- a/http-push.c +++ b/http-push.c @@ -1859,7 +1859,7 @@ int cmd_main(int argc, const char **argv) continue; } - if (!oidcmp(&ref->old_oid, &ref->peer_ref->new_oid)) { + if (oideq(&ref->old_oid, &ref->peer_ref->new_oid)) { if (push_verbosely) fprintf(stderr, "'%s': up-to-date\n", ref->name); if (helper_status) diff --git a/log-tree.c b/log-tree.c index 7443e5f..2edff78 100644 --- a/log-tree.c +++ b/log-tree.c @@ -472,7 +472,7 @@ static int which_parent(const struct object_id *oid, const struct commit *commit const struct commit_list *parent; for (nth = 0, parent = commit->parents; parent; parent = parent->next) { - if (!oidcmp(&parent->item->object.oid, oid)) + if (oideq(&parent->item->object.oid, oid)) return nth; nth++; } @@ -506,8 +506,8 @@ static int show_one_mergetag(struct commit *commit, if (parse_tag_buffer(the_repository, tag, extra->value, extra->len)) strbuf_addstr(&verify_message, "malformed mergetag\n"); else if (is_common_merge(commit) && - !oidcmp(&tag->tagged->oid, - &commit->parents->next->item->object.oid)) + oideq(&tag->tagged->oid, + &commit->parents->next->item->object.oid)) strbuf_addf(&verify_message, "merged tag '%s'\n", tag->tag); else if ((nth = which_parent(&tag->tagged->oid, commit)) < 0) diff --git a/merge-recursive.c b/merge-recursive.c index dcdc930..2904cb8 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -156,7 +156,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two, shift_tree_by(&one->object.oid, &two->object.oid, &shifted, subtree_shift); } - if (!oidcmp(&two->object.oid, &shifted)) + if (oideq(&two->object.oid, &shifted)) return two; return lookup_tree(the_repository, &shifted); } @@ -179,7 +179,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b) { if (!a && !b) return 2; - return a && b && oidcmp(a, b) == 0; + return a && b && oideq(a, b); } enum rename_type { diff --git a/notes-merge.c b/notes-merge.c index 76ab19e..0a47e54 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -151,7 +151,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o, mp = find_notes_merge_pair_pos(changes, len, &obj, 1, &occupied); if (occupied) { /* We've found an addition/deletion pair */ - assert(!oidcmp(&mp->obj, &obj)); + assert(oideq(&mp->obj, &obj)); if (is_null_oid(&p->one->oid)) { /* addition */ assert(is_null_oid(&mp->remote)); oidcpy(&mp->remote, &p->two->oid); @@ -218,7 +218,7 @@ static void diff_tree_local(struct notes_merge_options *o, continue; } - assert(!oidcmp(&mp->obj, &obj)); + assert(oideq(&mp->obj, &obj)); if (is_null_oid(&p->two->oid)) { /* deletion */ /* * Either this is a true deletion (1), or it is part @@ -229,7 +229,7 @@ static void diff_tree_local(struct notes_merge_options *o, * (3) mp->local is uninitialized; set it to null_sha1 * (will be overwritten by following addition) */ - if (!oidcmp(&mp->local, &uninitialized)) + if (oideq(&mp->local, &uninitialized)) oidclr(&mp->local); } else if (is_null_oid(&p->one->oid)) { /* addition */ /* @@ -241,7 +241,7 @@ static void diff_tree_local(struct notes_merge_options *o, * (3) mp->local is null_sha1; set to p->two->sha1 */ assert(is_null_oid(&mp->local) || - !oidcmp(&mp->local, &uninitialized)); + oideq(&mp->local, &uninitialized)); oidcpy(&mp->local, &p->two->oid); } else { /* modification */ /* @@ -249,8 +249,8 @@ static void diff_tree_local(struct notes_merge_options *o, * match mp->base, and mp->local shall be uninitialized. * Set mp->local to p->two->sha1. */ - assert(!oidcmp(&p->one->oid, &mp->base)); - assert(!oidcmp(&mp->local, &uninitialized)); + assert(oideq(&p->one->oid, &mp->base)); + assert(oideq(&mp->local, &uninitialized)); oidcpy(&mp->local, &p->two->oid); } trace_printf("\t\tStored local change for %s: %.7s -> %.7s\n", @@ -480,14 +480,14 @@ static int merge_changes(struct notes_merge_options *o, oid_to_hex(&p->local), oid_to_hex(&p->remote)); - if (!oidcmp(&p->base, &p->remote)) { + if (oideq(&p->base, &p->remote)) { /* no remote change; nothing to do */ trace_printf("\t\t\tskipping (no remote change)\n"); - } else if (!oidcmp(&p->local, &p->remote)) { + } else if (oideq(&p->local, &p->remote)) { /* same change in local and remote; nothing to do */ trace_printf("\t\t\tskipping (local == remote)\n"); - } else if (!oidcmp(&p->local, &uninitialized) || - !oidcmp(&p->local, &p->base)) { + } else if (oideq(&p->local, &uninitialized) || + oideq(&p->local, &p->base)) { /* no local change; adopt remote change */ trace_printf("\t\t\tno local change, adopted remote\n"); if (add_note(t, &p->obj, &p->remote, @@ -621,14 +621,14 @@ int notes_merge(struct notes_merge_options *o, oid_to_hex(&local->object.oid), oid_to_hex(base_oid)); - if (!oidcmp(&remote->object.oid, base_oid)) { + if (oideq(&remote->object.oid, base_oid)) { /* Already merged; result == local commit */ if (o->verbosity >= 2) printf("Already up to date!\n"); oidcpy(result_oid, &local->object.oid); goto found_result; } - if (!oidcmp(&local->object.oid, base_oid)) { + if (oideq(&local->object.oid, base_oid)) { /* Fast-forward; result == remote commit */ if (o->verbosity >= 2) printf("Fast-forward\n"); diff --git a/notes.c b/notes.c index 32d3dbc..b3386d6 100644 --- a/notes.c +++ b/notes.c @@ -266,9 +266,9 @@ static int note_tree_insert(struct notes_tree *t, struct int_node *tree, case PTR_TYPE_NOTE: switch (type) { case PTR_TYPE_NOTE: - if (!oidcmp(&l->key_oid, &entry->key_oid)) { + if (oideq(&l->key_oid, &entry->key_oid)) { /* skip concatenation if l == entry */ - if (!oidcmp(&l->val_oid, &entry->val_oid)) + if (oideq(&l->val_oid, &entry->val_oid)) return 0; ret = combine_notes(&l->val_oid, diff --git a/pack-write.c b/pack-write.c index a9d46bc..7d14716 100644 --- a/pack-write.c +++ b/pack-write.c @@ -124,7 +124,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec } hashwrite(f, obj->oid.hash, the_hash_algo->rawsz); if ((opts->flags & WRITE_IDX_STRICT) && - (i && !oidcmp(&list[-2]->oid, &obj->oid))) + (i && oideq(&list[-2]->oid, &obj->oid))) die("The same object %s appears twice in the pack", oid_to_hex(&obj->oid)); } diff --git a/read-cache.c b/read-cache.c index 7b1354d..eb4919e 100644 --- a/read-cache.c +++ b/read-cache.c @@ -767,7 +767,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, /* It was suspected to be racily clean, but it turns out to be Ok */ was_same = (alias && !ce_stage(alias) && - !oidcmp(&alias->oid, &ce->oid) && + oideq(&alias->oid, &ce->oid) && ce->ce_mode == alias->ce_mode); if (pretend) diff --git a/ref-filter.c b/ref-filter.c index 0bccfce..ccca317 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1710,7 +1710,7 @@ struct contains_stack { static int in_commit_list(const struct commit_list *want, struct commit *c) { for (; want; want = want->next) - if (!oidcmp(&want->item->object.oid, &c->object.oid)) + if (oideq(&want->item->object.oid, &c->object.oid)) return 1; return 0; } diff --git a/refs/files-backend.c b/refs/files-backend.c index 1f1a98e..aa45f53 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2307,7 +2307,7 @@ static int check_old_oid(struct ref_update *update, struct object_id *oid, struct strbuf *err) { if (!(update->flags & REF_HAVE_OLD) || - !oidcmp(oid, &update->old_oid)) + oideq(oid, &update->old_oid)) return 0; if (is_null_oid(&update->old_oid)) @@ -2443,7 +2443,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, !(update->flags & REF_DELETING) && !(update->flags & REF_LOG_ONLY)) { if (!(update->type & REF_ISSYMREF) && - !oidcmp(&lock->old_oid, &update->new_oid)) { + oideq(&lock->old_oid, &update->new_oid)) { /* * The reference already has the desired * value, so we don't need to write it. diff --git a/remote.c b/remote.c index 7f6277a..6f1ee9d 100644 --- a/remote.c +++ b/remote.c @@ -1388,7 +1388,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, ref->deletion = is_null_oid(&ref->new_oid); if (!ref->deletion && - !oidcmp(&ref->old_oid, &ref->new_oid)) { + oideq(&ref->old_oid, &ref->new_oid)) { ref->status = REF_STATUS_UPTODATE; continue; } @@ -2049,7 +2049,7 @@ struct ref *guess_remote_head(const struct ref *head, /* If refs/heads/master could be right, it is. */ if (!all) { r = find_ref_by_name(refs, "refs/heads/master"); - if (r && !oidcmp(&r->old_oid, &head->old_oid)) + if (r && oideq(&r->old_oid, &head->old_oid)) return copy_ref(r); } @@ -2057,7 +2057,7 @@ struct ref *guess_remote_head(const struct ref *head, for (r = refs; r; r = r->next) { if (r != head && starts_with(r->name, "refs/heads/") && - !oidcmp(&r->old_oid, &head->old_oid)) { + oideq(&r->old_oid, &head->old_oid)) { *tail = copy_ref(r); tail = &((*tail)->next); if (!all) diff --git a/revision.c b/revision.c index de4dce6..a2a569b 100644 --- a/revision.c +++ b/revision.c @@ -3238,7 +3238,7 @@ static void track_linear(struct rev_info *revs, struct commit *commit) struct commit_list *p; for (p = revs->previous_parents; p; p = p->next) if (p->item == NULL || /* first commit */ - !oidcmp(&p->item->object.oid, &commit->object.oid)) + oideq(&p->item->object.oid, &commit->object.oid)) break; revs->linear = p != NULL; } diff --git a/sequencer.c b/sequencer.c index 305de74..5f823c9 100644 --- a/sequencer.c +++ b/sequencer.c @@ -610,7 +610,7 @@ static int is_index_unchanged(void) if (!(cache_tree_oid = get_cache_tree_oid())) return -1; - return !oidcmp(cache_tree_oid, get_commit_tree_oid(head_commit)); + return oideq(cache_tree_oid, get_commit_tree_oid(head_commit)); } static int write_author_script(const char *message) @@ -1258,9 +1258,9 @@ static int try_to_commit(struct strbuf *msg, const char *author, goto out; } - if (!(flags & ALLOW_EMPTY) && !oidcmp(current_head ? - get_commit_tree_oid(current_head) : - the_hash_algo->empty_tree, &tree)) { + if (!(flags & ALLOW_EMPTY) && oideq(current_head ? + get_commit_tree_oid(current_head) : + the_hash_algo->empty_tree, &tree)) { res = 1; /* run 'git commit' to display error message */ goto out; } @@ -1365,7 +1365,7 @@ static int is_original_commit_empty(struct commit *commit) ptree_oid = the_hash_algo->empty_tree; /* commit is root */ } - return !oidcmp(ptree_oid, get_commit_tree_oid(commit)); + return oideq(ptree_oid, get_commit_tree_oid(commit)); } /* @@ -1645,7 +1645,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, unborn = get_oid("HEAD", &head); /* Do we want to generate a root commit? */ if (is_pick_or_similar(command) && opts->have_squash_onto && - !oidcmp(&head, &opts->squash_onto)) { + oideq(&head, &opts->squash_onto)) { if (is_fixup(command)) return error(_("cannot fixup root commit")); flags |= CREATE_ROOT_COMMIT; @@ -1688,7 +1688,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, oid_to_hex(&commit->object.oid)); if (opts->allow_ff && !is_fixup(command) && - ((parent && !oidcmp(&parent->object.oid, &head)) || + ((parent && oideq(&parent->object.oid, &head)) || (!parent && unborn))) { if (is_rebase_i(opts)) write_author_script(msg.message); @@ -2393,7 +2393,7 @@ static int rollback_is_safe(void) if (get_oid("HEAD", &actual_head)) oidclr(&actual_head); - return !oidcmp(&actual_head, &expected_head); + return oideq(&actual_head, &expected_head); } static int reset_for_rollback(const struct object_id *oid) @@ -2954,7 +2954,7 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, } if (opts->have_squash_onto && - !oidcmp(&head_commit->object.oid, &opts->squash_onto)) { + oideq(&head_commit->object.oid, &opts->squash_onto)) { /* * When the user tells us to "merge" something into a * "[new root]", let's simply fast-forward to the merge head. @@ -3023,8 +3023,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, * commit, we cannot fast-forward. */ can_fast_forward = opts->allow_ff && commit && commit->parents && - !oidcmp(&commit->parents->item->object.oid, - &head_commit->object.oid); + oideq(&commit->parents->item->object.oid, + &head_commit->object.oid); /* * If any merge head is different from the original one, we cannot @@ -3102,8 +3102,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, write_message("no-ff", 5, git_path_merge_mode(the_repository), 0); bases = get_merge_bases(head_commit, merge_commit); - if (bases && !oidcmp(&merge_commit->object.oid, - &bases->item->object.oid)) { + if (bases && oideq(&merge_commit->object.oid, + &bases->item->object.oid)) { ret = 0; /* skip merging an ancestor of HEAD */ goto leave_merge; @@ -3349,9 +3349,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) */ if (item->command == TODO_REWORD && !get_oid("HEAD", &oid) && - (!oidcmp(&item->commit->object.oid, &oid) || + (oideq(&item->commit->object.oid, &oid) || (opts->have_squash_onto && - !oidcmp(&opts->squash_onto, &oid)))) + oideq(&opts->squash_onto, &oid)))) to_amend = 1; return res | error_with_patch(item->commit, @@ -3578,7 +3578,7 @@ static int commit_staged_changes(struct replay_opts *opts, * the commit message and if there was a squash, let the user * edit it. */ - if (is_clean && !oidcmp(&head, &to_amend) && + if (is_clean && oideq(&head, &to_amend) && opts->current_fixup_count > 0 && file_exists(rebase_path_stopped_sha())) { const char *p = opts->current_fixups.buf; diff --git a/sha1-array.c b/sha1-array.c index 265941f..b94e0ec 100644 --- a/sha1-array.c +++ b/sha1-array.c @@ -69,7 +69,7 @@ int oid_array_for_each_unique(struct oid_array *array, for (i = 0; i < array->nr; i++) { int ret; - if (i > 0 && !oidcmp(array->oid + i, array->oid + i - 1)) + if (i > 0 && oideq(array->oid + i, array->oid + i - 1)) continue; ret = fn(array->oid + i, data); if (ret) diff --git a/sha1-file.c b/sha1-file.c index 97b7423..71f49ee 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -149,10 +149,10 @@ static struct cached_object *find_cached_object(const struct object_id *oid) struct cached_object *co = cached_objects; for (i = 0; i < cached_object_nr; i++, co++) { - if (!oidcmp(&co->oid, oid)) + if (oideq(&co->oid, oid)) return co; } - if (!oidcmp(oid, the_hash_algo->empty_tree)) + if (oideq(oid, the_hash_algo->empty_tree)) return &empty_tree; return NULL; } diff --git a/sha1-name.c b/sha1-name.c index c9cc131..a0c8451 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -44,7 +44,7 @@ static void update_candidates(struct disambiguate_state *ds, const struct object oidcpy(&ds->candidate, current); ds->candidate_exists = 1; return; - } else if (!oidcmp(&ds->candidate, current)) { + } else if (oideq(&ds->candidate, current)) { /* the same as what we already have seen */ return; } diff --git a/submodule.c b/submodule.c index 50cbf5f..a2ce58e 100644 --- a/submodule.c +++ b/submodule.c @@ -536,7 +536,7 @@ static void show_submodule_header(struct diff_options *o, const char *path, fast_backward = 1; } - if (!oidcmp(one, two)) { + if (oideq(one, two)) { strbuf_release(&sb); return; } diff --git a/transport.c b/transport.c index 06ffea2..1c76d64 100644 --- a/transport.c +++ b/transport.c @@ -1228,7 +1228,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs) nr_refs++; if (rm->peer_ref && !is_null_oid(&rm->old_oid) && - !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid)) + oideq(&rm->peer_ref->old_oid, &rm->old_oid)) continue; ALLOC_GROW(heads, nr_heads + 1, nr_alloc); heads[nr_heads++] = rm; diff --git a/unpack-trees.c b/unpack-trees.c index f25089b..4056a92 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -630,7 +630,7 @@ static int switch_cache_bottom(struct traverse_info *info) static inline int are_same_oid(struct name_entry *name_j, struct name_entry *name_k) { - return name_j->oid && name_k->oid && !oidcmp(name_j->oid, name_k->oid); + return name_j->oid && name_k->oid && oideq(name_j->oid, name_k->oid); } static int traverse_trees_recursive(int n, unsigned long dirmask, @@ -1484,7 +1484,7 @@ static int same(const struct cache_entry *a, const struct cache_entry *b) if ((a->ce_flags | b->ce_flags) & CE_CONFLICTED) return 0; return a->ce_mode == b->ce_mode && - !oidcmp(&a->oid, &b->oid); + oideq(&a->oid, &b->oid); } @@ -1616,7 +1616,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce, * If we are not going to update the submodule, then * we don't care. */ - if (!sub_head && !oidcmp(&oid, &ce->oid)) + if (!sub_head && oideq(&oid, &ce->oid)) return 0; return verify_clean_submodule(sub_head ? NULL : oid_to_hex(&oid), ce, error_type, o); diff --git a/wt-status.c b/wt-status.c index 5ffab61..1c8746d 100644 --- a/wt-status.c +++ b/wt-status.c @@ -453,8 +453,8 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q, d->worktree_status = p->status; if (S_ISGITLINK(p->two->mode)) { d->dirty_submodule = p->two->dirty_submodule; - d->new_submodule_commits = !!oidcmp(&p->one->oid, - &p->two->oid); + d->new_submodule_commits = !oideq(&p->one->oid, + &p->two->oid); if (s->status_format == STATUS_FORMAT_SHORT) d->worktree_status = short_submodule_status(d); } @@ -1487,10 +1487,10 @@ static void wt_status_get_detached_from(struct wt_status_state *state) if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 && /* sha1 is a commit? match without further lookup */ - (!oidcmp(&cb.noid, &oid) || + (oideq(&cb.noid, &oid) || /* perhaps sha1 is a tag, try to dereference to a commit */ ((commit = lookup_commit_reference_gently(the_repository, &oid, 1)) != NULL && - !oidcmp(&cb.noid, &commit->object.oid)))) { + oideq(&cb.noid, &commit->object.oid)))) { const char *from = ref; if (!skip_prefix(from, "refs/tags/", &from)) skip_prefix(from, "refs/remotes/", &from); @@ -1500,7 +1500,7 @@ static void wt_status_get_detached_from(struct wt_status_state *state) xstrdup(find_unique_abbrev(&cb.noid, DEFAULT_ABBREV)); oidcpy(&state->detached_oid, &cb.noid); state->detached_at = !get_oid("HEAD", &oid) && - !oidcmp(&oid, &state->detached_oid); + oideq(&oid, &state->detached_oid); free(ref); strbuf_release(&cb.buf); diff --git a/xdiff-interface.c b/xdiff-interface.c index ec6e574..e7af95d 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -186,7 +186,7 @@ void read_mmblob(mmfile_t *ptr, const struct object_id *oid) unsigned long size; enum object_type type; - if (!oidcmp(oid, &null_oid)) { + if (oideq(oid, &null_oid)) { ptr->ptr = xstrdup(""); ptr->size = 0; return; -- cgit v0.10.2-6-g49f6 From e3ff0683e22070ce42c3c373f6e89e9dc38b756c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:44 -0400 Subject: convert "hashcmp() == 0" to hasheq() This is the partner patch to the previous one, but covering the "hash" variants instead of "oid". Note that our coccinelle rule is slightly more complex to avoid triggering the call in hasheq(). I didn't bother to add a new rule to convert: - hasheq(E1->hash, E2->hash) + oideq(E1, E2) Since these are new functions, there won't be any such existing callers. And since most of the code is already using oideq, we're not likely to introduce new ones. We might still see "!hashcmp(E1->hash, E2->hash)" from topics in flight. But because our new rule comes after the existing ones, that should first get converted to "!oidcmp(E1, E2)" and then to "oideq(E1, E2)". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/fetch.c b/builtin/fetch.c index 32b1d5d..84e0e80 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -238,7 +238,7 @@ static int will_fetch(struct ref **head, const unsigned char *sha1) { struct ref *rm = *head; while (rm) { - if (!hashcmp(rm->old_oid.hash, sha1)) + if (hasheq(rm->old_oid.hash, sha1)) return 1; rm = rm->next; } diff --git a/cache.h b/cache.h index d090f71..d97db26 100644 --- a/cache.h +++ b/cache.h @@ -1053,12 +1053,12 @@ static inline int oideq(const struct object_id *oid1, const struct object_id *oi static inline int is_null_sha1(const unsigned char *sha1) { - return !hashcmp(sha1, null_sha1); + return hasheq(sha1, null_sha1); } static inline int is_null_oid(const struct object_id *oid) { - return !hashcmp(oid->hash, null_sha1); + return hasheq(oid->hash, null_sha1); } static inline void hashcpy(unsigned char *sha_dst, const unsigned char *sha_src) @@ -1095,7 +1095,7 @@ static inline void oidread(struct object_id *oid, const unsigned char *hash) static inline int is_empty_blob_sha1(const unsigned char *sha1) { - return !hashcmp(sha1, the_hash_algo->empty_blob->hash); + return hasheq(sha1, the_hash_algo->empty_blob->hash); } static inline int is_empty_blob_oid(const struct object_id *oid) @@ -1105,7 +1105,7 @@ static inline int is_empty_blob_oid(const struct object_id *oid) static inline int is_empty_tree_sha1(const unsigned char *sha1) { - return !hashcmp(sha1, the_hash_algo->empty_tree->hash); + return hasheq(sha1, the_hash_algo->empty_tree->hash); } static inline int is_empty_tree_oid(const struct object_id *oid) diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci index 548c023..d90ba8a 100644 --- a/contrib/coccinelle/object_id.cocci +++ b/contrib/coccinelle/object_id.cocci @@ -114,3 +114,12 @@ expression E1, E2; @@ - oidcmp(E1, E2) == 0 + oideq(E1, E2) + +@@ +identifier f != hasheq; +expression E1, E2; +@@ + f(...) {<... +- hashcmp(E1, E2) == 0 ++ hasheq(E1, E2) + ...>} diff --git a/http-walker.c b/http-walker.c index 7cdfb2f..3a8edc7 100644 --- a/http-walker.c +++ b/http-walker.c @@ -483,7 +483,7 @@ static int fetch_object(struct walker *walker, unsigned char *sha1) list_for_each(pos, head) { obj_req = list_entry(pos, struct object_request, node); - if (!hashcmp(obj_req->oid.hash, sha1)) + if (hasheq(obj_req->oid.hash, sha1)) break; } if (obj_req == NULL) diff --git a/notes.c b/notes.c index b3386d6..33d16c1 100644 --- a/notes.c +++ b/notes.c @@ -147,7 +147,7 @@ static struct leaf_node *note_tree_find(struct notes_tree *t, void **p = note_tree_search(t, &tree, &n, key_sha1); if (GET_PTR_TYPE(*p) == PTR_TYPE_NOTE) { struct leaf_node *l = (struct leaf_node *) CLR_PTR_TYPE(*p); - if (!hashcmp(key_sha1, l->key_oid.hash)) + if (hasheq(key_sha1, l->key_oid.hash)) return l; } return NULL; diff --git a/object.c b/object.c index 51c4594..e541605 100644 --- a/object.c +++ b/object.c @@ -95,7 +95,7 @@ struct object *lookup_object(struct repository *r, const unsigned char *sha1) first = i = hash_obj(sha1, r->parsed_objects->obj_hash_size); while ((obj = r->parsed_objects->obj_hash[i]) != NULL) { - if (!hashcmp(sha1, obj->oid.hash)) + if (hasheq(sha1, obj->oid.hash)) break; i++; if (i == r->parsed_objects->obj_hash_size) diff --git a/pack-objects.c b/pack-objects.c index 6ef87e5..2bc7626 100644 --- a/pack-objects.c +++ b/pack-objects.c @@ -16,7 +16,7 @@ static uint32_t locate_object_entry_hash(struct packing_data *pdata, while (pdata->index[i] > 0) { uint32_t pos = pdata->index[i] - 1; - if (!hashcmp(sha1, pdata->objects[pos].idx.oid.hash)) { + if (hasheq(sha1, pdata->objects[pos].idx.oid.hash)) { *found = 1; return i; } diff --git a/packfile.c b/packfile.c index ebcb574..c2e9629 100644 --- a/packfile.c +++ b/packfile.c @@ -1015,7 +1015,7 @@ void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1) { unsigned i; for (i = 0; i < p->num_bad_objects; i++) - if (!hashcmp(sha1, p->bad_object_sha1 + GIT_SHA1_RAWSZ * i)) + if (hasheq(sha1, p->bad_object_sha1 + GIT_SHA1_RAWSZ * i)) return; p->bad_object_sha1 = xrealloc(p->bad_object_sha1, st_mult(GIT_MAX_RAWSZ, @@ -1031,8 +1031,8 @@ const struct packed_git *has_packed_and_bad(const unsigned char *sha1) for (p = the_repository->objects->packed_git; p; p = p->next) for (i = 0; i < p->num_bad_objects; i++) - if (!hashcmp(sha1, - p->bad_object_sha1 + the_hash_algo->rawsz * i)) + if (hasheq(sha1, + p->bad_object_sha1 + the_hash_algo->rawsz * i)) return p; return NULL; } @@ -1830,8 +1830,8 @@ static int fill_pack_entry(const struct object_id *oid, if (p->num_bad_objects) { unsigned i; for (i = 0; i < p->num_bad_objects; i++) - if (!hashcmp(oid->hash, - p->bad_object_sha1 + the_hash_algo->rawsz * i)) + if (hasheq(oid->hash, + p->bad_object_sha1 + the_hash_algo->rawsz * i)) return 0; } -- cgit v0.10.2-6-g49f6 From 9001dc2a7493f1366a183c3a9175f608769321d5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:48 -0400 Subject: convert "oidcmp() != 0" to "!oideq()" This is the flip side of the previous two patches: checking for a non-zero oidcmp() can be more strictly expressed as inequality. Like those patches, we write "!= 0" in the coccinelle transformation, which covers by isomorphism the more common: if (oidcmp(E1, E2)) As with the previous two patches, this patch can be achieved almost entirely by running "make coccicheck"; the only differences are manual line-wrap fixes to match the original code. There is one thing to note for anybody replicating this, though: coccinelle 1.0.4 seems to miss the case in builtin/tag.c, even though it's basically the same as all the others. Running with 1.0.7 does catch this, so presumably it's just a coccinelle bug that was fixed in the interim. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/bisect.c b/bisect.c index 41c56a6..7c1d8f1 100644 --- a/bisect.c +++ b/bisect.c @@ -595,7 +595,7 @@ static struct commit_list *skip_away(struct commit_list *list, int count) for (i = 0; cur; cur = cur->next, i++) { if (i == index) { - if (oidcmp(&cur->item->object.oid, current_bad_oid)) + if (!oideq(&cur->item->object.oid, current_bad_oid)) return cur; if (previous) return previous; diff --git a/blame.c b/blame.c index c47d705..d5f7b72 100644 --- a/blame.c +++ b/blame.c @@ -1832,7 +1832,7 @@ void setup_scoreboard(struct blame_scoreboard *sb, sb->revs->children.name = "children"; while (c->parents && - oidcmp(&c->object.oid, &sb->final->object.oid)) { + !oideq(&c->object.oid, &sb->final->object.oid)) { struct commit_list *l = xcalloc(1, sizeof(*l)); l->item = c; @@ -1842,7 +1842,7 @@ void setup_scoreboard(struct blame_scoreboard *sb, c = c->parents->item; } - if (oidcmp(&c->object.oid, &sb->final->object.oid)) + if (!oideq(&c->object.oid, &sb->final->object.oid)) die(_("--reverse --first-parent together require range along first-parent chain")); } diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 4c82c23..268f0c2 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -78,9 +78,9 @@ static struct merge_parent *find_merge_parent(struct merge_parents *table, { int i; for (i = 0; i < table->nr; i++) { - if (given && oidcmp(&table->item[i].given, given)) + if (given && !oideq(&table->item[i].given, given)) continue; - if (commit && oidcmp(&table->item[i].commit, commit)) + if (commit && !oideq(&table->item[i].commit, commit)) continue; return &table->item[i]; } diff --git a/builtin/merge.c b/builtin/merge.c index 57abff9..8d85d31 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1521,7 +1521,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) * HEAD^^" would be missed. */ common_one = get_merge_bases(head_commit, j->item); - if (oidcmp(&common_one->item->object.oid, &j->item->object.oid)) { + if (!oideq(&common_one->item->object.oid, &j->item->object.oid)) { up_to_date = 0; break; } diff --git a/builtin/pull.c b/builtin/pull.c index a720f58..09b0269 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -902,7 +902,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) oidclr(&curr_head); if (!is_null_oid(&orig_head) && !is_null_oid(&curr_head) && - oidcmp(&orig_head, &curr_head)) { + !oideq(&orig_head, &curr_head)) { /* * The fetch involved updating the current branch. * diff --git a/builtin/rm.c b/builtin/rm.c index 2cbe89e..17086d3 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -180,7 +180,7 @@ static int check_local_mod(struct object_id *head, int index_only) if (no_head || get_tree_entry(head, name, &oid, &mode) || ce->ce_mode != create_ce_mode(mode) - || oidcmp(&ce->oid, &oid)) + || !oideq(&ce->oid, &oid)) staged_changes = 1; /* diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 363cf85..5f94328 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -412,7 +412,7 @@ static int append_head_ref(const char *refname, const struct object_id *oid, /* If both heads/foo and tags/foo exists, get_sha1 would * get confused. */ - if (get_oid(refname + ofs, &tmp) || oidcmp(&tmp, oid)) + if (get_oid(refname + ofs, &tmp) || !oideq(&tmp, oid)) ofs = 5; return append_ref(refname + ofs, oid, 0); } @@ -427,7 +427,7 @@ static int append_remote_ref(const char *refname, const struct object_id *oid, /* If both heads/foo and tags/foo exists, get_sha1 would * get confused. */ - if (get_oid(refname + ofs, &tmp) || oidcmp(&tmp, oid)) + if (get_oid(refname + ofs, &tmp) || !oideq(&tmp, oid)) ofs = 5; return append_ref(refname + ofs, oid, 0); } diff --git a/builtin/tag.c b/builtin/tag.c index 9a19ffb..f623632 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -559,7 +559,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) ref_transaction_commit(transaction, &err)) die("%s", err.buf); ref_transaction_free(transaction); - if (force && !is_null_oid(&prev) && oidcmp(&prev, &object)) + if (force && !is_null_oid(&prev) && !oideq(&prev, &object)) printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(&prev, DEFAULT_ABBREV)); diff --git a/bundle.c b/bundle.c index 24cbe40..14f2cfc 100644 --- a/bundle.c +++ b/bundle.c @@ -369,7 +369,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs) * commit that is referenced by the tag, and not the tag * itself. */ - if (oidcmp(&oid, &e->item->oid)) { + if (!oideq(&oid, &e->item->oid)) { /* * Is this the positive end of a range expressed * in terms of a tag (e.g. v2.0 from the range diff --git a/commit-graph.c b/commit-graph.c index 050c516..7aed9f5 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -765,7 +765,7 @@ void write_commit_graph(const char *obj_dir, count_distinct = 1; for (i = 1; i < oids.nr; i++) { - if (oidcmp(&oids.list[i-1], &oids.list[i])) + if (!oideq(&oids.list[i - 1], &oids.list[i])) count_distinct++; } @@ -960,7 +960,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g) continue; } - if (oidcmp(&get_commit_tree_in_graph_one(g, graph_commit)->object.oid, + if (!oideq(&get_commit_tree_in_graph_one(g, graph_commit)->object.oid, get_commit_tree_oid(odb_commit))) graph_report("root tree OID for commit %s in commit-graph is %s != %s", oid_to_hex(&cur_oid), @@ -977,7 +977,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g) break; } - if (oidcmp(&graph_parents->item->object.oid, &odb_parents->item->object.oid)) + if (!oideq(&graph_parents->item->object.oid, &odb_parents->item->object.oid)) graph_report("commit-graph parent for %s is %s != %s", oid_to_hex(&cur_oid), oid_to_hex(&graph_parents->item->object.oid), diff --git a/commit.c b/commit.c index 1a6e632..1b94a8c 100644 --- a/commit.c +++ b/commit.c @@ -46,7 +46,7 @@ struct commit *lookup_commit_or_die(const struct object_id *oid, const char *ref struct commit *c = lookup_commit_reference(the_repository, oid); if (!c) die(_("could not parse %s"), ref_name); - if (oidcmp(oid, &c->object.oid)) { + if (!oideq(oid, &c->object.oid)) { warning(_("%s %s is not a commit!"), ref_name, oid_to_hex(oid)); } diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci index d90ba8a..4e1f1a7 100644 --- a/contrib/coccinelle/object_id.cocci +++ b/contrib/coccinelle/object_id.cocci @@ -123,3 +123,9 @@ expression E1, E2; - hashcmp(E1, E2) == 0 + hasheq(E1, E2) ...>} + +@@ +expression E1, E2; +@@ +- oidcmp(E1, E2) != 0 ++ !oideq(E1, E2) diff --git a/diff-lib.c b/diff-lib.c index c1f5a52..8866b1d 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -342,7 +342,7 @@ static int show_modified(struct rev_info *revs, } if (revs->combine_merges && !cached && - (oidcmp(oid, &old_entry->oid) || oidcmp(&old_entry->oid, &new_entry->oid))) { + (!oideq(oid, &old_entry->oid) || !oideq(&old_entry->oid, &new_entry->oid))) { struct combine_diff_path *p; int pathlen = ce_namelen(new_entry); diff --git a/diff.c b/diff.c index 5cada68..5d3219b 100644 --- a/diff.c +++ b/diff.c @@ -3765,7 +3765,7 @@ static int reuse_worktree_file(const char *name, const struct object_id *oid, in * This is not the sha1 we are looking for, or * unreusable because it is not a regular file. */ - if (oidcmp(oid, &ce->oid) || !S_ISREG(ce->ce_mode)) + if (!oideq(oid, &ce->oid) || !S_ISREG(ce->ce_mode)) return 0; /* @@ -4170,7 +4170,7 @@ static void fill_metainfo(struct strbuf *msg, default: *must_show_header = 0; } - if (one && two && oidcmp(&one->oid, &two->oid)) { + if (one && two && !oideq(&one->oid, &two->oid)) { const unsigned hexsz = the_hash_algo->hexsz; int abbrev = o->flags.full_index ? hexsz : DEFAULT_ABBREV; @@ -5457,7 +5457,7 @@ static void diff_resolve_rename_copy(void) else p->status = DIFF_STATUS_RENAMED; } - else if (oidcmp(&p->one->oid, &p->two->oid) || + else if (!oideq(&p->one->oid, &p->two->oid) || p->one->mode != p->two->mode || p->one->dirty_submodule || p->two->dirty_submodule || diff --git a/diffcore-rename.c b/diffcore-rename.c index d775183..daddd9b 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -286,7 +286,7 @@ static int find_identical_files(struct hashmap *srcs, struct diff_filespec *source = p->filespec; /* False hash collision? */ - if (oidcmp(&source->oid, &target->oid)) + if (!oideq(&source->oid, &target->oid)) continue; /* Non-regular files? If so, the modes must match! */ if (!S_ISREG(source->mode) || !S_ISREG(target->mode)) { diff --git a/dir.c b/dir.c index aceb0d4..8ee9fe8 100644 --- a/dir.c +++ b/dir.c @@ -1282,7 +1282,7 @@ static void prep_exclude(struct dir_struct *dir, * order, though, if you do that. */ if (untracked && - oidcmp(&oid_stat.oid, &untracked->exclude_oid)) { + !oideq(&oid_stat.oid, &untracked->exclude_oid)) { invalidate_gitignore(dir->untracked, untracked); oidcpy(&untracked->exclude_oid, &oid_stat.oid); } @@ -2248,12 +2248,12 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d /* Validate $GIT_DIR/info/exclude and core.excludesfile */ root = dir->untracked->root; - if (oidcmp(&dir->ss_info_exclude.oid, + if (!oideq(&dir->ss_info_exclude.oid, &dir->untracked->ss_info_exclude.oid)) { invalidate_gitignore(dir->untracked, root); dir->untracked->ss_info_exclude = dir->ss_info_exclude; } - if (oidcmp(&dir->ss_excludes_file.oid, + if (!oideq(&dir->ss_excludes_file.oid, &dir->untracked->ss_excludes_file.oid)) { invalidate_gitignore(dir->untracked, root); dir->untracked->ss_excludes_file = dir->ss_excludes_file; diff --git a/fast-import.c b/fast-import.c index a731088..67a53b7 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2649,7 +2649,7 @@ static int parse_from(struct branch *b) struct object_entry *oe = find_mark(idnum); if (oe->type != OBJ_COMMIT) die("Mark :%" PRIuMAX " not a commit", idnum); - if (oidcmp(&b->oid, &oe->idx.oid)) { + if (!oideq(&b->oid, &oe->idx.oid)) { oidcpy(&b->oid, &oe->idx.oid); if (oe->pack_id != MAX_PACK_ID) { unsigned long size; @@ -2667,7 +2667,7 @@ static int parse_from(struct branch *b) else die("Invalid ref name or SHA1 expression: %s", from); - if (b->branch_tree.tree && oidcmp(&oid, &b->branch_tree.versions[1].oid)) { + if (b->branch_tree.tree && !oideq(&oid, &b->branch_tree.versions[1].oid)) { release_tree_content_recursive(b->branch_tree.tree); b->branch_tree.tree = NULL; } diff --git a/fetch-pack.c b/fetch-pack.c index 88a078e..75047a4 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -599,7 +599,7 @@ static void filter_refs(struct fetch_pack_args *args, continue; if (parse_oid_hex(ref->name, &oid, &p) || *p != '\0' || - oidcmp(&oid, &ref->old_oid)) + !oideq(&oid, &ref->old_oid)) continue; if ((allow_unadvertised_object_request & diff --git a/match-trees.c b/match-trees.c index 3765330..2b6d31e 100644 --- a/match-trees.c +++ b/match-trees.c @@ -106,7 +106,7 @@ static int score_trees(const struct object_id *hash1, const struct object_id *ha update_tree_entry(&two); } else { /* path appears in both */ - if (oidcmp(one.entry.oid, two.entry.oid)) { + if (!oideq(one.entry.oid, two.entry.oid)) { /* they are different */ score += score_differs(one.entry.mode, two.entry.mode, diff --git a/notes.c b/notes.c index 33d16c1..25cdce2 100644 --- a/notes.c +++ b/notes.c @@ -206,7 +206,7 @@ static void note_tree_remove(struct notes_tree *t, if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE) return; /* type mismatch, nothing to remove */ l = (struct leaf_node *) CLR_PTR_TYPE(*p); - if (oidcmp(&l->key_oid, &entry->key_oid)) + if (!oideq(&l->key_oid, &entry->key_oid)) return; /* key mismatch, nothing to remove */ /* we have found a matching entry */ diff --git a/read-cache.c b/read-cache.c index eb4919e..88bb803 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2029,7 +2029,7 @@ int read_index_from(struct index_state *istate, const char *path, base_oid_hex = oid_to_hex(&split_index->base_oid); base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex); ret = do_read_index(split_index->base, base_path, 1); - if (oidcmp(&split_index->base_oid, &split_index->base->oid)) + if (!oideq(&split_index->base_oid, &split_index->base->oid)) die("broken index, expect %s in %s, got %s", base_oid_hex, base_path, oid_to_hex(&split_index->base->oid)); diff --git a/refs.c b/refs.c index de81c7b..a7a75b4 100644 --- a/refs.c +++ b/refs.c @@ -702,7 +702,7 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid, pseudoref); rollback_lock_file(&lock); goto done; - } else if (oidcmp(&actual_old_oid, old_oid)) { + } else if (!oideq(&actual_old_oid, old_oid)) { strbuf_addf(err, _("unexpected object ID when writing '%s'"), pseudoref); rollback_lock_file(&lock); @@ -744,7 +744,7 @@ static int delete_pseudoref(const char *pseudoref, const struct object_id *old_o } if (read_ref(pseudoref, &actual_old_oid)) die(_("could not read ref '%s'"), pseudoref); - if (oidcmp(&actual_old_oid, old_oid)) { + if (!oideq(&actual_old_oid, old_oid)) { error(_("unexpected object ID when deleting '%s'"), pseudoref); rollback_lock_file(&lock); @@ -875,13 +875,13 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid, */ if (!is_null_oid(&cb->ooid)) { oidcpy(cb->oid, noid); - if (oidcmp(&cb->ooid, noid)) + if (!oideq(&cb->ooid, noid)) warning(_("log for ref %s has gap after %s"), cb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822))); } else if (cb->date == cb->at_time) oidcpy(cb->oid, noid); - else if (oidcmp(noid, cb->oid)) + else if (!oideq(noid, cb->oid)) warning(_("log for ref %s unexpectedly ended on %s"), cb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822))); diff --git a/refs/files-backend.c b/refs/files-backend.c index aa45f53..16ef932 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -841,7 +841,7 @@ static int verify_lock(struct ref_store *ref_store, struct ref_lock *lock, return 0; } } - if (old_oid && oidcmp(&lock->old_oid, old_oid)) { + if (old_oid && !oideq(&lock->old_oid, old_oid)) { strbuf_addf(err, "ref '%s' is at %s but expected %s", lock->ref_name, oid_to_hex(&lock->old_oid), diff --git a/refs/packed-backend.c b/refs/packed-backend.c index d447a73..74e2996 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1160,7 +1160,7 @@ static int write_with_updates(struct packed_ref_store *refs, "reference already exists", update->refname); goto error; - } else if (oidcmp(&update->old_oid, iter->oid)) { + } else if (!oideq(&update->old_oid, iter->oid)) { strbuf_addf(err, "cannot update ref '%s': " "is at %s but expected %s", update->refname, diff --git a/refs/ref-cache.c b/refs/ref-cache.c index 9b110c8..b7052f7 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -272,7 +272,7 @@ static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2 /* This is impossible by construction */ die("Reference directory conflict: %s", ref1->name); - if (oidcmp(&ref1->u.value.oid, &ref2->u.value.oid)) + if (!oideq(&ref1->u.value.oid, &ref2->u.value.oid)) die("Duplicated ref, and SHA1s don't match: %s", ref1->name); warning("Duplicated ref: %s", ref1->name); diff --git a/remote.c b/remote.c index 6f1ee9d..e23b767 100644 --- a/remote.c +++ b/remote.c @@ -1403,7 +1403,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * branch. */ if (ref->expect_old_sha1) { - if (oidcmp(&ref->old_oid, &ref->old_oid_expect)) + if (!oideq(&ref->old_oid, &ref->old_oid_expect)) reject_reason = REF_STATUS_REJECT_STALE; else /* If the ref isn't stale then force the update. */ diff --git a/sequencer.c b/sequencer.c index 5f823c9..b14e5dc 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1188,7 +1188,7 @@ static int parse_head(struct commit **head) current_head = lookup_commit_reference(the_repository, &oid); if (!current_head) return error(_("could not parse HEAD")); - if (oidcmp(&oid, ¤t_head->object.oid)) { + if (!oideq(&oid, ¤t_head->object.oid)) { warning(_("HEAD %s is not a commit!"), oid_to_hex(&oid)); } @@ -3034,7 +3034,7 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, struct commit_list *p = commit->parents->next; for (j = to_merge; j && p; j = j->next, p = p->next) - if (oidcmp(&j->item->object.oid, + if (!oideq(&j->item->object.oid, &p->item->object.oid)) { can_fast_forward = 0; break; @@ -3566,7 +3566,7 @@ static int commit_staged_changes(struct replay_opts *opts, if (get_oid_hex(rev.buf, &to_amend)) return error(_("invalid contents: '%s'"), rebase_path_amend()); - if (!is_clean && oidcmp(&head, &to_amend)) + if (!is_clean && !oideq(&head, &to_amend)) return error(_("\nYou have uncommitted changes in your " "working tree. Please, commit them\n" "first and then run 'git rebase " @@ -4545,7 +4545,7 @@ int skip_unnecessary_picks(void) if (item->commit->parents->next) break; /* merge commit */ parent_oid = &item->commit->parents->item->object.oid; - if (oidcmp(parent_oid, oid)) + if (!oideq(parent_oid, oid)) break; oid = &item->commit->object.oid; } diff --git a/sha1-file.c b/sha1-file.c index 71f49ee..cc8a196 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -825,7 +825,7 @@ int check_object_signature(const struct object_id *oid, void *map, if (map) { hash_object_file(map, size, type, &real_oid); - return oidcmp(oid, &real_oid) ? -1 : 0; + return !oideq(oid, &real_oid) ? -1 : 0; } st = open_istream(oid, &obj_type, &size, NULL); @@ -852,7 +852,7 @@ int check_object_signature(const struct object_id *oid, void *map, } the_hash_algo->final_fn(real_oid.hash, &c); close_istream(st); - return oidcmp(oid, &real_oid) ? -1 : 0; + return !oideq(oid, &real_oid) ? -1 : 0; } int git_open_cloexec(const char *name, int flags) @@ -1671,7 +1671,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr, die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid), ret); the_hash_algo->final_fn(parano_oid.hash, &c); - if (oidcmp(oid, ¶no_oid) != 0) + if (!oideq(oid, ¶no_oid)) die(_("confused by unstable object source data for %s"), oid_to_hex(oid)); diff --git a/submodule-config.c b/submodule-config.c index fc2c41b..e04ba75 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -45,7 +45,7 @@ static int config_path_cmp(const void *unused_cmp_data, const struct submodule_entry *b = entry_or_key; return strcmp(a->config->path, b->config->path) || - oidcmp(&a->config->gitmodules_oid, &b->config->gitmodules_oid); + !oideq(&a->config->gitmodules_oid, &b->config->gitmodules_oid); } static int config_name_cmp(const void *unused_cmp_data, @@ -57,7 +57,7 @@ static int config_name_cmp(const void *unused_cmp_data, const struct submodule_entry *b = entry_or_key; return strcmp(a->config->name, b->config->name) || - oidcmp(&a->config->gitmodules_oid, &b->config->gitmodules_oid); + !oideq(&a->config->gitmodules_oid, &b->config->gitmodules_oid); } static struct submodule_cache *submodule_cache_alloc(void) diff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c index 98a4891..6a3f88f 100644 --- a/t/helper/test-dump-cache-tree.c +++ b/t/helper/test-dump-cache-tree.c @@ -33,7 +33,7 @@ static int dump_cache_tree(struct cache_tree *it, } else { dump_one(it, pfx, ""); - if (oidcmp(&it->oid, &ref->oid) || + if (!oideq(&it->oid, &ref->oid) || ref->entry_count != it->entry_count || ref->subtree_nr != it->subtree_nr) { /* claims to be valid but is lying */ diff --git a/tree-diff.c b/tree-diff.c index 553bc0e..425668e 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -491,7 +491,7 @@ static struct combine_diff_path *ll_diff_tree_paths( continue; /* diff(t,pi) != ΓΈ */ - if (oidcmp(t.entry.oid, tp[i].entry.oid) || + if (!oideq(t.entry.oid, tp[i].entry.oid) || (t.entry.mode != tp[i].entry.mode)) continue; -- cgit v0.10.2-6-g49f6 From 67947c34ae8f666e72b9406a38984fe8386f5e50 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:52 -0400 Subject: convert "hashcmp() != 0" to "!hasheq()" This rounds out the previous three patches, covering the inequality logic for the "hash" variant of the functions. As with the previous three, the accompanying code changes are the mechanical result of applying the coccinelle patch; see those patches for more discussion. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/index-pack.c b/builtin/index-pack.c index edcb0a3..2004e25 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1166,7 +1166,7 @@ static void parse_pack_objects(unsigned char *hash) /* Check pack integrity */ flush(); the_hash_algo->final_fn(hash, &input_ctx); - if (hashcmp(fill(the_hash_algo->rawsz), hash)) + if (!hasheq(fill(the_hash_algo->rawsz), hash)) die(_("pack is corrupted (SHA1 mismatch)")); use(the_hash_algo->rawsz); @@ -1280,7 +1280,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha fixup_pack_header_footer(output_fd, pack_hash, curr_pack, nr_objects, read_hash, consumed_bytes-the_hash_algo->rawsz); - if (hashcmp(read_hash, tail_hash) != 0) + if (!hasheq(read_hash, tail_hash)) die(_("Unexpected tail checksum for %s " "(disk corruption?)"), curr_pack); } diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 5f94328..65f4a4c 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -485,7 +485,7 @@ static void snarf_refs(int head, int remotes) static int rev_is_head(const char *head, const char *name, unsigned char *head_sha1, unsigned char *sha1) { - if (!head || (head_sha1 && sha1 && hashcmp(head_sha1, sha1))) + if (!head || (head_sha1 && sha1 && !hasheq(head_sha1, sha1))) return 0; skip_prefix(head, "refs/heads/", &head); if (!skip_prefix(name, "refs/heads/", &name)) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index ad438f5..8047880 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -579,7 +579,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix) if (fsck_finish(&fsck_options)) die(_("fsck error in pack objects")); } - if (hashcmp(fill(the_hash_algo->rawsz), oid.hash)) + if (!hasheq(fill(the_hash_algo->rawsz), oid.hash)) die("final sha1 did not match"); use(the_hash_algo->rawsz); diff --git a/commit-graph.c b/commit-graph.c index 7aed9f5..64ce794 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -900,7 +900,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g) f = hashfd(devnull, NULL); hashwrite(f, g->data, g->data_len - g->hash_len); finalize_hashfile(f, checksum.hash, CSUM_CLOSE); - if (hashcmp(checksum.hash, g->data + g->data_len - g->hash_len)) { + if (!hasheq(checksum.hash, g->data + g->data_len - g->hash_len)) { graph_report(_("the commit-graph file has incorrect checksum and is likely corrupt")); verify_commit_graph_error = VERIFY_COMMIT_GRAPH_ERROR_HASH; } diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci index 4e1f1a7..d8bdb48 100644 --- a/contrib/coccinelle/object_id.cocci +++ b/contrib/coccinelle/object_id.cocci @@ -129,3 +129,12 @@ expression E1, E2; @@ - oidcmp(E1, E2) != 0 + !oideq(E1, E2) + +@@ +identifier f != hasheq; +expression E1, E2; +@@ + f(...) {<... +- hashcmp(E1, E2) != 0 ++ !hasheq(E1, E2) + ...>} diff --git a/http-walker.c b/http-walker.c index 3a8edc7..b3334bf 100644 --- a/http-walker.c +++ b/http-walker.c @@ -543,7 +543,7 @@ static int fetch_object(struct walker *walker, unsigned char *sha1) } else if (req->zret != Z_STREAM_END) { walker->corrupt_object_found++; ret = error("File %s (%s) corrupt", hex, req->url); - } else if (hashcmp(obj_req->oid.hash, req->real_sha1)) { + } else if (!hasheq(obj_req->oid.hash, req->real_sha1)) { ret = error("File %s has bad hash", hex); } else if (req->rename < 0) { struct strbuf buf = STRBUF_INIT; diff --git a/http.c b/http.c index 4162860..98ff122 100644 --- a/http.c +++ b/http.c @@ -2394,7 +2394,7 @@ int finish_http_object_request(struct http_object_request *freq) unlink_or_warn(freq->tmpfile.buf); return -1; } - if (hashcmp(freq->sha1, freq->real_sha1)) { + if (!hasheq(freq->sha1, freq->real_sha1)) { unlink_or_warn(freq->tmpfile.buf); return -1; } diff --git a/pack-check.c b/pack-check.c index d3a57df..fa5f0ff 100644 --- a/pack-check.c +++ b/pack-check.c @@ -79,10 +79,10 @@ static int verify_packfile(struct packed_git *p, } while (offset < pack_sig_ofs); the_hash_algo->final_fn(hash, &ctx); pack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL); - if (hashcmp(hash, pack_sig)) + if (!hasheq(hash, pack_sig)) err = error("%s pack checksum mismatch", p->pack_name); - if (hashcmp(index_base + index_size - the_hash_algo->hexsz, pack_sig)) + if (!hasheq(index_base + index_size - the_hash_algo->hexsz, pack_sig)) err = error("%s pack checksum does not match its index", p->pack_name); unuse_pack(w_curs); @@ -180,7 +180,7 @@ int verify_pack_index(struct packed_git *p) the_hash_algo->init_fn(&ctx); the_hash_algo->update_fn(&ctx, index_base, (unsigned int)(index_size - the_hash_algo->rawsz)); the_hash_algo->final_fn(hash, &ctx); - if (hashcmp(hash, index_base + index_size - the_hash_algo->rawsz)) + if (!hasheq(hash, index_base + index_size - the_hash_algo->rawsz)) err = error("Packfile index for %s hash mismatch", p->pack_name); return err; diff --git a/pack-write.c b/pack-write.c index 7d14716..29d17a9 100644 --- a/pack-write.c +++ b/pack-write.c @@ -260,7 +260,7 @@ void fixup_pack_header_footer(int pack_fd, if (partial_pack_offset == 0) { unsigned char hash[GIT_MAX_RAWSZ]; the_hash_algo->final_fn(hash, &old_hash_ctx); - if (hashcmp(hash, partial_pack_hash) != 0) + if (!hasheq(hash, partial_pack_hash)) die("Unexpected checksum for %s " "(disk corruption?)", pack_name); /* diff --git a/packfile.c b/packfile.c index c2e9629..1e9eacd 100644 --- a/packfile.c +++ b/packfile.c @@ -517,7 +517,7 @@ static int open_packed_git_1(struct packed_git *p) if (read_result != hashsz) return error("packfile %s signature is unavailable", p->pack_name); idx_hash = ((unsigned char *)p->index_data) + p->index_size - hashsz * 2; - if (hashcmp(hash, idx_hash)) + if (!hasheq(hash, idx_hash)) return error("packfile %s does not match index", p->pack_name); return 0; } diff --git a/read-cache.c b/read-cache.c index 88bb803..421a7f4 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1668,7 +1668,7 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size) the_hash_algo->init_fn(&c); the_hash_algo->update_fn(&c, hdr, size - the_hash_algo->rawsz); the_hash_algo->final_fn(hash, &c); - if (hashcmp(hash, (unsigned char *)hdr + size - the_hash_algo->rawsz)) + if (!hasheq(hash, (unsigned char *)hdr + size - the_hash_algo->rawsz)) return error("bad index file sha1 signature"); return 0; } @@ -2395,7 +2395,7 @@ static int verify_index_from(const struct index_state *istate, const char *path) if (n != the_hash_algo->rawsz) goto out; - if (hashcmp(istate->oid.hash, hash)) + if (!hasheq(istate->oid.hash, hash)) goto out; close(fd); diff --git a/sha1-file.c b/sha1-file.c index cc8a196..d85f4e9 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -2213,7 +2213,7 @@ static int check_stream_sha1(git_zstream *stream, } the_hash_algo->final_fn(real_sha1, &c); - if (hashcmp(expected_sha1, real_sha1)) { + if (!hasheq(expected_sha1, real_sha1)) { error(_("sha1 mismatch for %s (expected %s)"), path, sha1_to_hex(expected_sha1)); return -1; -- cgit v0.10.2-6-g49f6 From cc00e5ce6b50cf4197e137b87a33209568b1e662 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:55 -0400 Subject: convert hashmap comparison functions to oideq() The comparison functions used for hashmaps don't care about strict ordering; they only want to compare entries for equality. Let's use the oideq() function instead, which can potentially be better optimized. Note that unlike the previous patches mass-converting calls like "!oidcmp()", this patch could actually provide an improvement even with the current implementation. Those comparison functions are passed around as function pointers, so at compile-time the compiler cannot realize that the caller (which is in another file completely) will treat the return value as a boolean. Note that this does change the return values in quite a subtle way (it's still an int, but now the sign bit is irrelevant for ordering). Because of their funny hashmap-specific signature, it's unlikely that any of these static functions would be reused for more generic ordering. But to be double-sure, let's stop using "cmp" in their names. Calling them "eq" doesn't quite work either, because the hashmap convention is actually _inverted_. "0" means "same", and non-zero means "different". So I've called them "neq" by convention here. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/describe.c b/builtin/describe.c index 1e7c75b..22c0541 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -62,7 +62,7 @@ static const char *prio_names[] = { N_("head"), N_("lightweight"), N_("annotated"), }; -static int commit_name_cmp(const void *unused_cmp_data, +static int commit_name_neq(const void *unused_cmp_data, const void *entry, const void *entry_or_key, const void *peeled) @@ -70,7 +70,7 @@ static int commit_name_cmp(const void *unused_cmp_data, const struct commit_name *cn1 = entry; const struct commit_name *cn2 = entry_or_key; - return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled); + return !oideq(&cn1->peeled, peeled ? peeled : &cn2->peeled); } static inline struct commit_name *find_commit_name(const struct object_id *peeled) @@ -596,7 +596,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix) return cmd_name_rev(args.argc, args.argv, prefix); } - hashmap_init(&names, commit_name_cmp, NULL, 0); + hashmap_init(&names, commit_name_neq, NULL, 0); for_each_rawref(get_name, NULL); if (!hashmap_get_size(&names) && !always) die(_("No names found, cannot describe anything.")); diff --git a/oidmap.c b/oidmap.c index d9fb19b..b0841a0 100644 --- a/oidmap.c +++ b/oidmap.c @@ -1,14 +1,14 @@ #include "cache.h" #include "oidmap.h" -static int cmpfn(const void *hashmap_cmp_fn_data, - const void *entry, const void *entry_or_key, - const void *keydata) +static int oidmap_neq(const void *hashmap_cmp_fn_data, + const void *entry, const void *entry_or_key, + const void *keydata) { const struct oidmap_entry *entry_ = entry; if (keydata) - return oidcmp(&entry_->oid, (const struct object_id *) keydata); - return oidcmp(&entry_->oid, + return !oideq(&entry_->oid, (const struct object_id *) keydata); + return !oideq(&entry_->oid, &((const struct oidmap_entry *) entry_or_key)->oid); } @@ -21,7 +21,7 @@ static int hash(const struct object_id *oid) void oidmap_init(struct oidmap *map, size_t initial_size) { - hashmap_init(&map->map, cmpfn, NULL, initial_size); + hashmap_init(&map->map, oidmap_neq, NULL, initial_size); } void oidmap_free(struct oidmap *map, int free_entries) diff --git a/patch-ids.c b/patch-ids.c index 8f7c25d..960ea24 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -28,14 +28,14 @@ int commit_patch_id(struct commit *commit, struct diff_options *options, /* * When we cannot load the full patch-id for both commits for whatever * reason, the function returns -1 (i.e. return error(...)). Despite - * the "cmp" in the name of this function, the caller only cares about + * the "neq" in the name of this function, the caller only cares about * the return value being zero (a and b are equivalent) or non-zero (a * and b are different), and returning non-zero would keep both in the * result, even if they actually were equivalent, in order to err on * the side of safety. The actual value being negative does not have * any significance; only that it is non-zero matters. */ -static int patch_id_cmp(const void *cmpfn_data, +static int patch_id_neq(const void *cmpfn_data, const void *entry, const void *entry_or_key, const void *unused_keydata) @@ -53,7 +53,7 @@ static int patch_id_cmp(const void *cmpfn_data, commit_patch_id(b->commit, opt, &b->patch_id, 0)) return error("Could not get patch ID for %s", oid_to_hex(&b->commit->object.oid)); - return oidcmp(&a->patch_id, &b->patch_id); + return !oideq(&a->patch_id, &b->patch_id); } int init_patch_ids(struct patch_ids *ids) @@ -63,7 +63,7 @@ int init_patch_ids(struct patch_ids *ids) ids->diffopts.detect_rename = 0; ids->diffopts.flags.recursive = 1; diff_setup_done(&ids->diffopts); - hashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256); + hashmap_init(&ids->patches, patch_id_neq, &ids->diffopts, 256); return 0; } -- cgit v0.10.2-6-g49f6 From 6a29d7b7a72c7b131ae3cda9e0b47b67a27a51f1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:22:59 -0400 Subject: read-cache: use oideq() in ce_compare functions These functions return the full oidcmp() value, but the callers really only care whether it is non-zero. We can use the more strict !oideq(), which a compiler may be able to optimize further. This does change the meaning of the return value subtly, but it's unlikely that anybody would try to use them for ordering. They're static-local in this file, and they already return other error values that would confuse an ordering (e.g., open() failure gives -1). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/read-cache.c b/read-cache.c index 421a7f4..eb7cea6 100644 --- a/read-cache.c +++ b/read-cache.c @@ -213,7 +213,7 @@ static int ce_compare_data(const struct cache_entry *ce, struct stat *st) if (fd >= 0) { struct object_id oid; if (!index_fd(&oid, fd, st, OBJ_BLOB, ce->name, 0)) - match = oidcmp(&oid, &ce->oid); + match = !oideq(&oid, &ce->oid); /* index_fd() closed the file descriptor already */ } return match; @@ -254,7 +254,7 @@ static int ce_compare_gitlink(const struct cache_entry *ce) */ if (resolve_gitlink_ref(ce->name, "HEAD", &oid) < 0) return 0; - return oidcmp(&oid, &ce->oid); + return !oideq(&oid, &ce->oid); } static int ce_modified_check_fs(const struct cache_entry *ce, struct stat *st) -- cgit v0.10.2-6-g49f6 From d9f62dfa0ddc31bdad93054acca9fd42ca781f7f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Aug 2018 17:23:03 -0400 Subject: show_dirstat: simplify same-content check We use two nested conditionals to store a content_changed variable, but only bother to look at the result once, directly after we set it. We can drop the variable entirely and just use a single "if". This needless complexity is the result of 2ff3a80334 (Teach --dirstat not to completely ignore rearranged lines within a file, 2011-04-11). Before that, we held onto the content_changed variable much longer. While we're touching the condition, we can swap out oidcmp() for !oideq(). Our coccinelle patches didn't previously find this case because of the intermediate variable, but now it's a simple boolean in a conditional. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/diff.c b/diff.c index 5d3219b..605ba4b 100644 --- a/diff.c +++ b/diff.c @@ -2933,16 +2933,11 @@ static void show_dirstat(struct diff_options *options) struct diff_filepair *p = q->queue[i]; const char *name; unsigned long copied, added, damage; - int content_changed; name = p->two->path ? p->two->path : p->one->path; - if (p->one->oid_valid && p->two->oid_valid) - content_changed = oidcmp(&p->one->oid, &p->two->oid); - else - content_changed = 1; - - if (!content_changed) { + if (p->one->oid_valid && p->two->oid_valid && + oideq(&p->one->oid, &p->two->oid)) { /* * The SHA1 has not changed, so pre-/post-content is * identical. We can therefore skip looking at the @@ -2989,7 +2984,7 @@ static void show_dirstat(struct diff_options *options) * made to the preimage. * If the resulting damage is zero, we know that * diffcore_count_changes() considers the two entries to - * be identical, but since content_changed is true, we + * be identical, but since the oid changed, we * know that there must have been _some_ kind of change, * so we force all entries to have damage > 0. */ -- cgit v0.10.2-6-g49f6