My first pull request to libgit2 project.

It was this particular issue #3817, I chose to resolve. It was a bug in git_config_parse_int64() function in parsing MIN_INT64 (-9223372036854775808)

It is a minor bug, which occurs in the rarest of the cases. Neverthless, it was fun to debug and fix this one. A cursory look at the code tells the following.

Check if the number is positive or negative

if (*p == '-' || *p == '+')
        if (*p++ == '-')
neg = 1;
  • ovfl - overflow flag
  • nn - number
  • n - assign after overflow

Code:

for (; nptr_len > 0; p++,ndig++,nptr_len--) {
    c = *p;
    v = base;
    //Set v as (int) digit
    nn = n*base + v;
    if (nn < n)
        ovfl = 1;
    n = nn;
}

And then, multiply with -1 appropriately.

*result = neg ? -n : n;

Some review⌗

INT64 can store values from (-2^63^) to (2^63^ - 1)

That is, –9,223,372,036,854,775,808 to 9,223,372,036,854,775,807. The MSB is used to specify the sign. So, 63 bits are used to denote the number to be stored.

Solution⌗

In the original code, the given string is

  • Determine if the number is positive or negative.
  • Convert the string to positive integer
  • Using the neg flag, set the parsed number as positive or negative.

This will have a problem in parsing MIN_INT64 because, –9,223,372,036,854,775,808 (a valid INT64 value) is initially parsed as 9,223,372,036,854,775,808 (invalid INT64 value) which will result in an overflow.

We can make sure that this does not happen by parsing the positive and negative integers, as such. My fix was to modify

nn = n*base + v;

as

nn = n * base + (neg ? -v : v);

Here is the detailed diff of the patch.

Pull request successfully merged into master :)