On Thu, Apr 27, 2017 at 01:52:09AM -0400, Jeff King wrote: > On Sun, Apr 23, 2017 at 09:34:35PM +0000, brian m. carlson wrote: > > Use a > > struct object_id to hold the pack checksum, even though it is not > > strictly an object ID. Doing so ensures resilience against future hash > > size changes, and allows us to remove hard-coded assumptions about how > > big the buffer needs to be. > > But this part seems questionable to me. Sure, we may change the pack > checksum in the future. But there is a reasonable chance that it won't > follow the same rules for selecting a hash as object_id. And even if it > does, calling it object_id just seems misleading. > > What's the gain in converting it here? I know we want to get rid of the > bare "20", but we could switch it out for GIT_SHA1_RAWSZ. I suspect you > prefer in the long run to get rid of even those GIT_SHA1_RAWSZ defines, > though. Could we define a new struct csumfile_hash, csumfile_cmp, etc > (and arguably change the name of "struct sha1file" and friends). They'd > be boring wrappers around sha1 now, but I think the endgame will involve > us being able to read multiple versions of packs, with distinct > checksum algorithms. When I wrote this originally, the GIT_MAX_*SZ patch was in object-id-part9 and hadn't been merged yet. And I think your concerns about this being kinda gross are legitimate. I'll admit I had some hesitance about it at first. So I'll reroll this leaving it as an unsigned char with GIT_MAX_RAWSZ. I feel confident that we're not going to pick a third, different algorithm for the pack checksum, so that will get us to the point that we have a big enough buffer, and we can incrementally improve on it later by using a different struct if we like. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature