Thomas Gummerer <t.gumme...@gmail.com> writes:

> 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?

> @@ -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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to