Re: [PATCH v2 1/2] read-cache: skip index SHA verification

From: Jeff King <peff@peff.net>
Date: Mar 28, 2017, 15:37 UTC
Message-ID: <20170328153745.yi5fb5kkfa27q36o@sigill.intra.peff.net>
In-Reply-To: <7b40fea5-8843-b95b-5ea5-c8035ea8a36e@jeffhostetler.com>

On Tue, Mar 28, 2017 at 11:27:19AM -0400, Jeff Hostetler wrote:

Show 13 quoted lines

> > Hrm, there shouldn't be any dependency of the config on the index (and
> > there are a handful of options which impact the index already). Did you
> > try it and run into problems?
> 
> Yeah, I tried adding a new "core.verifyindex" property and the
> corresponding global variable.  But read_index() and verify_hdr()
> was being called BEFORE the config was loaded.  And it wasn't clear
> how best to solve that.
> 
> The issue was in "git status" where cmd_status() called
> status_init_config() which called gitmodules_config() before
> git_config().  but gitmodules_config() called read_index(),
> so my settings weren't loaded yet in verify_hdr().

Ugh, yeah, the callback-oriented interface suffers from these kind of dependency cycles. You can fix it by doing a limited "basic config that should always be loaded" git_config() call before anything else, and then following up with the application-level config.

For something low-level that should _always_ be respected, even in plumbing programs, I think we're better off lazy-loading the config inside the function. The configset cache makes them more or less free.

I.e., something like:

diff --git a/read-cache.c b/read-cache.c
index e44775182..89bbf8d1e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1376,17 +1376,23 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size)
 	git_SHA_CTX c;
 	unsigned char sha1[20];
 	int hdr_version;
+	int do_checksum = 0;
 
 	if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
 		return error("bad signature");
 	hdr_version = ntohl(hdr->hdr_version);
 	if (hdr_version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < hdr_version)
 		return error("bad index version %d", hdr_version);
-	git_SHA1_Init(&c);
-	git_SHA1_Update(&c, hdr, size - 20);
-	git_SHA1_Final(sha1, &c);
-	if (hashcmp(sha1, (unsigned char *)hdr + size - 20))
-		return error("bad index file sha1 signature");
+
+	git_config_get_bool("core.checksumindex", &do_checksum);
+	if (do_checksum) {
+		git_SHA1_Init(&c);
+		git_SHA1_Update(&c, hdr, size - 20);
+		git_SHA1_Final(sha1, &c);
+		if (hashcmp(sha1, (unsigned char *)hdr + size - 20))
+			return error("bad index file sha1 signature");
+	}
+
 	return 0;
 }

-Peff