Re: [PATCH v2 2/6] midx: add progress to write_midx_file Add progress to write_midx_file. Progress is displayed when the MIDX_PROGRESS flag is set.

[William Baker <williamtbakeremail@gmail.com>]

On 9/20/19 1:10 PM, Junio C Hamano wrote:
>> diff --git a/midx.c b/midx.c
>> index b2673f52e8..54e4e93b2b 100644
>> --- a/midx.c
>> +++ b/midx.c
>> @@ -449,6 +449,8 @@ struct pack_list {
>>  	uint32_t nr;
>>  	uint32_t alloc;
>>  	struct multi_pack_index *m;
>> +	struct progress *progress;
>> +	uint32_t pack_paths_checked;
>>  };
> 
> What is the reason behind u32 here?  Do we want to be consistently
> limited to 4G on all platforms?
> 
> I have a feeling that we do not care too deeply all that much for
> the purpose of progress output, in which case, just an ordinary
> "unsigned" may be much less misleading.

I went with u32 here to match the data type used to track how many
entries are in the pack_list ('nr' is a u32).

I could switch to this to an unsigned but we would run the (extremely
unlikely) risk of having more than 65k packs on a system where
unsigned is 16 bits.

> FWIW, the function that uses this field is display_progress(), which
> takes uint64_t there.

Thanks for pointing this out.  Given that display_progress() expects a
u64 using that type here for 'pack_paths_checked' could help make things
more straightforward.

-William