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.

From: William Baker <williamtbakeremail@gmail.com>
Date: Sep 23, 2019, 21:12 UTC
Message-ID: <373fc8e9-2ef3-d501-b6e0-b953e047ac78@gmail.com>
In-Reply-To: <xmqqlfuiu3sv.fsf@gitster-ct.c.googlers.com>

On 9/20/19 1:10 PM, Junio C Hamano wrote:

Show 18 quoted lines

>> 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