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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> Jeff King <peff@xxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]