Re: [PATCH 05/15] midx: add entries to write_midx_context

From: Derrick Stolee <stolee@gmail.com>
Date: Dec 4, 2020, 13:39 UTC
Message-ID: <f9ca5bd8-3a7a-232b-f038-3311087d60f1@gmail.com>
In-Reply-To: <xmqqim9ih8hz.fsf@gitster.c.googlers.com>

On 12/3/2020 4:42 PM, Junio C Hamano wrote:

Show 17 quoted lines

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> -static size_t write_midx_oid_lookup(struct hashfile *f, unsigned char hash_len,
>> -				    struct pack_midx_entry *objects,
>> -				    uint32_t nr_objects)
>> +static size_t write_midx_oid_lookup(struct hashfile *f,
>> +				    void *data)
>>  {
>> -	struct pack_midx_entry *list = objects;
>> +	struct write_midx_context *ctx = (struct write_midx_context *)data;
>> +	unsigned char hash_len = the_hash_algo->rawsz;
>> +	struct pack_midx_entry *list = ctx->entries;
> 
> I know this is meant to be a faithful rewrite, but can we lose this
> "length of the hash function output must be smaller than 256"
> imposed by "unsigned char" at some point, perhaps after this series
> settles?  .rawsz field is size_t IIRC.

There's no reason why this should be "unsigned char" except that I was probably thinking about the "hash version" byte in the header when I added this line. Clearly there isn't a failure (yet), but it's better to be safe.

Thanks, -Stolee