Re: [PATCH 0/2] git-config and large integers

From: Jeff King <peff@peff.net>
Date: Aug 21, 2013, 02:34 UTC
Message-ID: <20130821023429.GA25296@sigill.intra.peff.net>
In-Reply-To: <xmqqli3wufmc.fsf@gitster.dls.corp.google.com>

On Tue, Aug 20, 2013 at 04:06:19PM -0700, Junio C Hamano wrote:

Show 22 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > I was playing with a hook for file size limits that wanted to store the
> > limit in git-config. It turns out we don't do a very good job of big
> > integers:
> >
> >   $ git config foo.size 2g
> >   $ git config --int foo.size
> >   -2147483648
> >
> > Oops. After this series, we properly notice the error:
> >
> >   $ git config --int foo.size
> >   fatal: bad config value for 'foo.size' in .git/config
> >
> > and even better, provide a way to access large values:
> >
> >   $ git config --ulong foo.size
> >   2147483648
> 
> I may be missing something, but why do we even need a new option for
> the command that is known to always produce textual output?

We could do all math with int64_t (for example) and then print the stringified representation. But then we would not be matching the same checks that git is doing internally.

For example, on a 32-bit system, setting core.packedGitLimit to 4G will produce an error for "git config --int core.packedgitlimit", as we cannot represent the size internally. We could do the conversion in such a way that we print the correct size, but it does not represent what happens when "git pack-objects" is run (you get the same error).

> As you said "Oops", the first example that shows a string of digits
> prefixed by a minus sign for input "2g" is buggy, and I think it is
> perfectly reasonable to fix it to show a stringified representation
> of 2*1024*1024*1024 when asked for "--int".

The negative value is a little bit of a sidetrack. For both "git config --int" and for internal use, we do not correctly range-check integer values. And that's why we print the negative value, when we should say "this is a bogus out-of-range value". The latter is what we have always done for values outside of range, both internal and external, and it is only that our range check was bogus (and we fed negative crap to the code instead of complaining). That is fixed by the first patch.

And that leads to the second patch. The "--int" option provides a range check of (typically) -2^32 to 2^32-1. But many of our values internally use a larger range. We can either drop that range check (which means we will let you inspect values with config that git internally will barf on, with no clue), or we need to add another option with a different range to retrieve those values. I chose to add another option because I think the range check has value.

Does that explain the problem more fully?

-Peff