Re: [PATCH v2 01/24] builtin/pack-objects: make hash agnostic

From: Jeff King <peff@peff.net>
Date: Feb 24, 2020, 04:20 UTC
Message-ID: <20200224042004.GA731094@coredump.intra.peff.net>
In-Reply-To: <20200224034259.GG6462@camp.crustytoothpaste.net>

On Mon, Feb 24, 2020 at 03:42:59AM +0000, brian m. carlson wrote:

> > Yeah, looks obviously correct (and I think this is new from the
> > pack-reuse patches of mine that Christian sent recently).
> 
> I believe it is, which is why I CC'd you on it.

Heh, yeah. I knew you knew, but was saying so for the rest of the audience. :)

Show 9 quoted lines

> > I think it would also be correct to cast the mmap'd bytes to a "struct
> > object_id" given that the struct contains the hash bytes as the first
> > member. I worry a little that we'd get no compiler warning of the
> > breakage if that assumption changes, but it also seems unlikely to do
> > so.
> 
> In the future, struct object_id will get a new member (an algorithm
> value), but I think it's fine to make the assumption that the hash bytes
> are first.

Yeah, I think that would continue to work, although weirdness would ensue if anybody ever dereferenced the algorithm member in one of the type-punned structs. If we can avoid it entirely, I think we should (and it was easy to remove the spot in pack-check).

Patches incoming.

-Peff