Jeff King <peff@xxxxxxxx> writes:
> On Fri, Nov 01, 2024 at 11:07:48AM -0500, karthik nayak wrote:
>
>> Thanks for letting me know, I think the fix is simply
>>
>> diff --git a/packfile.c b/packfile.c
>> index f626d38071..737cd60377 100644
>> --- a/packfile.c
>> +++ b/packfile.c
>> @@ -27,8 +27,8 @@
>> #include "pack-objects.h"
>>
>> struct packfile_config {
>> - unsigned long packed_git_window_size;
>> - unsigned long packed_git_limit;
>> + unsigned long long packed_git_window_size;
>> + unsigned long long packed_git_limit;
>> };
>>
>> #define PACKFILE_CONFIG_INIT \
>
> Wait, why did these change from "size_t" to "unsigned long" in your
> series in the first place? If the goal is moving them into a struct,
> they should otherwise retain the same types, no?
>
> Two asides, one for your series and one #leftoverbits:
>
> 1. Since these are now fields of packfile_config, do they need the
> long packed_git_ prefix anymore? Just window_size and limit would
> be nicer, I'd think.
>
Moving them to repo_settings means we'd have to keep the long names,
otherwise, I agree the shorter names would be better.
> 2. I can imagine you might have used "unsigned long" because they are
> parsed with git_config_ulong(). That is OK on Linux, where size_t
> and "unsigned long" are the same size (either 32- or 64-bits). But
> on Windows I think it means that you cannot configure a window
> larger than 4GB on a 64-bit system. Or ironically you cannot set a
> total limit larger than 4GB, even though the default is 32TB. ;)
Yup that's the reason I changed them. TIL about size_t and how it works.
Thanks, I'll change the types accordingly and push a new version soon.
>
> -Peff
Attachment:
signature.asc
Description: PGP signature
