Re: [PATCH/RFC v2 01/16] Modify cache_header to prepare for other index formats
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:25
Thomas Gummerer [off-list ref] writes:
quoted hunk
diff --git a/read-cache.c b/read-cache.c index 2f8159f..5d61d92 100644 --- a/read-cache.c +++ b/read-cache.c@@ -1433,7 +1446,7 @@ int read_index_from(struct index_state *istate, const char *path) errno = EINVAL; mmap_size = xsize_t(st.st_size); - if (mmap_size < sizeof(struct cache_header) + 20) + if (mmap_size < sizeof(struct cache_version_header) + 20) die("index file smaller than expected");
At the design level, I have a large problem with this change. I
understand that you wanted to make sure that some versions can lack
the num-entries word in the header, but then what is the point of
keeping that "+20" here? Are all versions of the file format still
required to have the 20-byte trailing SHA-1 sum over the whole file?
Side note: I am actually fine with that "sum at the end"
requirement, but then it needs to be documented what are
assumed to be unomittable and why.
I also do not see why v5 *needs* to drop the num-entries
word from the header in the first place.
At the practical level, we used to error out, upon seeing a file
that claims to be v2 in the header but is too small to hold the
version header, the number of entries word and the trailing SHA-1
sum. We no longer do this and happily call verify_hdr() in the
following code even when the file is too small, no?
quoted hunk
@@ -1442,11 +1455,13 @@ int read_index_from(struct index_state *istate, const char *path) die_errno("unable to map index file"); hdr = mmap; + hdr_v2 = mmap + sizeof(*hdr); if (verify_hdr(hdr, mmap_size) < 0) goto unmap;