On Mon, Apr 23, 2018 at 11:39:18PM +0000, brian m. carlson wrote:
> There are several instances of the constant 20 and 20-based values in
> the packfile code. Abstract away dependence on SHA-1 by using the
> values from the_hash_algo instead.
While we're abstracting away 20. There's the only 20 left in
sha1_file.c that should also be gone. But I guess you could do that
later since you need to rename fill_sha1_path to
fill_loose_object_path or something.
> @@ -507,15 +509,15 @@ static int open_packed_git_1(struct packed_git *p)
> " while index indicates %"PRIu32" objects",
> p->pack_name, ntohl(hdr.hdr_entries),
> p->num_objects);
> - if (lseek(p->pack_fd, p->pack_size - sizeof(sha1), SEEK_SET) == -1)
> + if (lseek(p->pack_fd, p->pack_size - hashsz, SEEK_SET) == -1)
> return error("end of packfile %s is unavailable", p->pack_name);
> - read_result = read_in_full(p->pack_fd, sha1, sizeof(sha1));
> + read_result = read_in_full(p->pack_fd, hash, hashsz);
> if (read_result < 0)
> return error_errno("error reading from %s", p->pack_name);
> - if (read_result != sizeof(sha1))
> + if (read_result != hashsz)
> return error("packfile %s signature is unavailable", p->pack_name);
> - idx_sha1 = ((unsigned char *)p->index_data) + p->index_size - 40;
> - if (hashcmp(sha1, idx_sha1))
> + idx_hash = ((unsigned char *)p->index_data) + p->index_size - hashsz * 2;
> + if (hashcmp(hash, idx_hash))
Since the hash size is abstracted away, shouldn't this hashcmp become
oidcmp? (which still does not do the right thing, but at least it's
one less place to worry about)
Same comment for other hashcmp in this patch.
> @@ -675,7 +677,8 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
> p->pack_size = st.st_size;
> p->pack_local = local;
> p->mtime = st.st_mtime;
> - if (path_len < 40 || get_sha1_hex(path + path_len - 40, p->sha1))
> + if (path_len < the_hash_algo->hexsz ||
> + get_sha1_hex(path + path_len - the_hash_algo->hexsz, p->sha1))
get_sha1_hex looks out of place when we start going with
the_hash_algo. Maybe change to get_oid_hex() too.
> @@ -1678,10 +1683,10 @@ int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32
>
> index_lookup = index_fanout + 4 * 256;
> if (p->index_version == 1) {
> - index_lookup_width = 24;
> + index_lookup_width = hashsz + 4;
You did good research to spot this 24 constant ;-)
--
Duy
