Re: [PATCH 10/13] pack-objects: refactor path-walk delta phase

[Derrick Stolee <stolee@gmail.com>]

On 3/12/2025 5:21 PM, Taylor Blau wrote:
> On Mon, Mar 10, 2025 at 01:50:52AM +0000, Derrick Stolee via GitGitGadget wrote:

>> +static void find_deltas_for_region(struct object_entry *list UNUSED,
> 
> Interesting, it looks like "list" here is UNUSED in this patch. On first
> read I figured that you were going to make use of it in later patches,
> but it looks like it remains UNUSED at the tip of my local copy of this
> series.
> 
> Is this a stray change from when you were writing this? Something else?

Actually, the intention was to make this method less focused on globals,
but it in fact uses 'to_pack.objects' when it should use 'list'. Thanks
for the careful eye here. 
>> +				   struct packing_region *region,
>> +				   unsigned int *processed)
>> +{
>> +	struct object_entry **delta_list;
>> +	uint32_t delta_list_nr = 0;
> 
> I know that we have a lot of "_nr" and "_alloc" variables in the
> pack-objects code that use uint32_t as their type, but we should prefer
> size_t for these in the future, even if it breaks the existing pattern.
Unfortunately, the fact that these are used in the find_deltas() method
as an 'unsigned *' reference, these types need to be as they are. I'm
hesitant to use 64-bit integers that will eventually cast down to 32-bit
in these instances.

Thanks,
-Stolee