On Tue, Feb 12, 2019 at 01:22:29AM +0000, brian m. carlson wrote: > Replace the uses of sha1_to_hex in the pack bitmap code with hash_to_hex > to allow the use of SHA-256 as well. Rename a few variables since they > are no longer limited to SHA-1. Sounds good, although... > -static uint32_t find_object_pos(const unsigned char *sha1) > +static uint32_t find_object_pos(const unsigned char *hash) Isn't this really just a "struct object_id"? Why don't we want to use that here? I suspect it may be partially because our khash is storing raw sha1s. But shouldn't we also be converted that to store object_ids? In particular: > @@ -181,7 +181,7 @@ static struct stored_bitmap *store_bitmap(struct bitmap_index *index, > stored->root = root; > stored->xor = xor_with; > stored->flags = flags; > - oidread(&stored->oid, sha1); > + oidread(&stored->oid, hash); > > hash_pos = kh_put_sha1(index->bitmaps, stored->oid.hash, &ret); This last line (which is actually from the previous patch) is going to always truncate the stored data to 20 bytes, isn't it? I think we need to define a kh_oid. We have the "set" version already in oidset.[ch]; I think we need to make that more public. -Peff
