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 --- read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'read-cache.c') diff --git a/read-cache.c b/read-cache.c index 7b1354d759..eb4919e77f 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) -- cgit v1.2.3 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 --- read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'read-cache.c') diff --git a/read-cache.c b/read-cache.c index eb4919e77f..88bb80326b 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)); -- cgit v1.2.3 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 --- read-cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'read-cache.c') diff --git a/read-cache.c b/read-cache.c index 88bb80326b..421a7f4953 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); -- cgit v1.2.3 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 --- read-cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'read-cache.c') diff --git a/read-cache.c b/read-cache.c index 421a7f4953..eb7cea6272 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 v1.2.3