[PATCH] Support sizes >=2G in various config options accepting 'g' sizes.

[PATCH] Support sizes >=2G in various config options accepting 'g' sizes.

[Nix]

The config options core.packedGitWindowSize, core.packedGitLimit,
core.deltaBaseCacheLimit and core.bigFileThreshold all claim
to support suffixes up to and including 'g'.  This implies that
they should accept sizes >=2G on 64-bit systems: certainly,
specifying a size of 3g should not silently be translated to zero
or transformed into a large negative value due to integer
overflow.  However, due to use of git_config_int() rather than
git_config_ulong(), that is exactly what happens:

% git config core.bigFileThreshold 2g
% git gc --aggressive # with extra debugging code to print out
                      # core.bigfilethreshold after parsing
bigfilethreshold: -2147483648
[...]

This is probably irrelevant for core.deltaBaseCacheLimit, but
is problematic for the other values.  (It is particularly
problematic for core.packedGitLimit, which can't even be set to
its default value in the config file due to this bug.)

I haven't tried to fix things on 32-bit platforms, because there
is no real point setting any values to >2G on such platforms
anyway, and minimal likelihood that anyone would try.  The only
real fix possible would be a diagnostic warning of an attempt to
set a ridiculously high value, unless we want to use 'long long'
everywhere, which I doubt.

Signed-off-by: Nick Alcock <nix@esperi.org.uk>
---
 config.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/config.c b/config.c
index 4183f80..919f581 100644
--- a/config.c
+++ b/config.c
@@ -550,7 +550,7 @@ static int git_default_core_config(const char *var, const char *value)
 
 	if (!strcmp(var, "core.packedgitwindowsize")) {
 		int pgsz_x2 = getpagesize() * 2;
-		packed_git_window_size = git_config_int(var, value);
+		packed_git_window_size = git_config_ulong(var, value);
 
 		/* This value must be multiple of (pagesize * 2) */
 		packed_git_window_size /= pgsz_x2;
@@ -561,18 +561,18 @@ static int git_default_core_config(const char *var, const char *value)
 	}
 
 	if (!strcmp(var, "core.bigfilethreshold")) {
-		long n = git_config_int(var, value);
+		long n = git_config_ulong(var, value);
 		big_file_threshold = 0 < n ? n : 0;
 		return 0;
 	}
 
 	if (!strcmp(var, "core.packedgitlimit")) {
-		packed_git_limit = git_config_int(var, value);
+		packed_git_limit = git_config_ulong(var, value);
 		return 0;
 	}
 
 	if (!strcmp(var, "core.deltabasecachelimit")) {
-		delta_base_cache_limit = git_config_int(var, value);
+		delta_base_cache_limit = git_config_ulong(var, value);
 		return 0;
 	}
 
-- 
1.7.6.1.138.g03ab.dirty

Re: [PATCH] Support sizes >=2G in various config options accepting 'g' sizes.

[Nix]

On 4 Sep 2011, Clemens Buchacher uttered the following:

> On Sep 4, 2011 11:25 PM, "Nix" <nix@esperi.org.uk> wrote:
>>
>> I haven't tried to fix things on 32-bit platforms, because there
>> is no real point setting any values to >2G on such platforms
>> anyway, and minimal likelihood that anyone would try.
>
> I absolutely would not count on that.

I was just operating under the assumption that since nobody had spotted
this in years... OK, OK, perhaps that's a bad idea.

>> The only
>> real fix possible would be a diagnostic warning of an attempt to
>> set a ridiculously high value, unless we want to use 'long long'
>> everywhere, which I doubt.
>
> I think an error message would be appropriate. Best if we die immediately
> when that option is read.

Yeah. None of the affected options impact the pack format (as, say,
pack.depth does) so there is no danger of 32-bit users being barred from
reading packs created by 64-bit users with high values for these
settings.

(We have no way of guaranteeing that we can even report this, though:
 we can only even read in a >32-bit number if NO_STRTOULL is not
 defined. Still, I agree that if we *can* report this, we should.)

>                           We wouldn't want e.g. clone to die when it just
> finished downloading. And some documentation for those limits would be
> great, while you're at it. :-)

I'll send a new patch tomorrow.

... hm, pack.packsizelimit is also affected. I'll plug that too.

-- 
NULL && (void)