From 3fb72c7b4d90fb7f95ca60f448c7739cca6c1846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 22 Jun 2019 12:03:30 +0200 Subject: config: use unsigned_mult_overflows to check for overflows parse_unit_factor() checks if a K, M or G is present after a number and multiplies it by 2^10, 2^20 or 2^30, respectively. One of its callers checks if the result is smaller than the number alone to detect overflows. The other one passes 1 as the number and does multiplication and overflow check itself in a similar manner. This works, but is inconsistent, and it would break if we added support for a bigger unit factor. E.g. 16777217T is 2^64 + 2^40, i.e. too big for a 64-bit number. Modulo 2^64 we get 2^40 == 1TB, which is bigger than the raw number 16777217 == 2^24 + 1, so the overflow would go undetected by that method. Let both callers pass 1 and handle overflow check and multiplication themselves. Do the check before the multiplication, using unsigned_mult_overflows, which is simpler and can deal with larger unit factors. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano diff --git a/config.c b/config.c index 01c6e9d..3c00369 100644 --- a/config.c +++ b/config.c @@ -870,8 +870,8 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max) return 0; } uval = val < 0 ? -val : val; - uval *= factor; - if (uval > max || (val < 0 ? -val : val) > uval) { + if (unsigned_mult_overflows(factor, uval) || + factor * uval > max) { errno = ERANGE; return 0; } @@ -888,21 +888,22 @@ static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max) if (value && *value) { char *end; uintmax_t val; - uintmax_t oldval; + uintmax_t factor = 1; errno = 0; val = strtoumax(value, &end, 0); if (errno == ERANGE) return 0; - oldval = val; - if (!parse_unit_factor(end, &val)) { + if (!parse_unit_factor(end, &factor)) { errno = EINVAL; return 0; } - if (val > max || oldval > val) { + if (unsigned_mult_overflows(factor, val) || + factor * val > max) { errno = ERANGE; return 0; } + val *= factor; *ret = val; return 1; } -- cgit v0.10.2-6-g49f6 From 664178e8e23fbd6d79b02ea51374015023c02102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 22 Jun 2019 12:03:36 +0200 Subject: config: don't multiply in parse_unit_factor() parse_unit_factor() multiplies the number that is passed to it with the value of a recognized unit factor (K, M or G for 2^10, 2^20 and 2^30, respectively). All callers pass in 1 as a number, though, which allows them to check the actual multiplication for overflow before they are doing it themselves. Ignore the passed in number and don't multiply, as this feature of parse_unit_factor() is not used anymore. Rename the output parameter to reflect that it's not about the end result anymore, but just about the unit factor. Suggested-by: Johannes Schindelin Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano diff --git a/config.c b/config.c index 3c00369..a8bd1d8 100644 --- a/config.c +++ b/config.c @@ -834,20 +834,22 @@ static int git_parse_source(config_fn_t fn, void *data, return error_return; } -static int parse_unit_factor(const char *end, uintmax_t *val) +static int parse_unit_factor(const char *end, uintmax_t *factor) { - if (!*end) + if (!*end) { + *factor = 1; return 1; + } else if (!strcasecmp(end, "k")) { - *val *= 1024; + *factor = 1024; return 1; } else if (!strcasecmp(end, "m")) { - *val *= 1024 * 1024; + *factor = 1024 * 1024; return 1; } else if (!strcasecmp(end, "g")) { - *val *= 1024 * 1024 * 1024; + *factor = 1024 * 1024 * 1024; return 1; } return 0; @@ -859,7 +861,7 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max) char *end; intmax_t val; uintmax_t uval; - uintmax_t factor = 1; + uintmax_t factor; errno = 0; val = strtoimax(value, &end, 0); @@ -888,7 +890,7 @@ static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max) if (value && *value) { char *end; uintmax_t val; - uintmax_t factor = 1; + uintmax_t factor; errno = 0; val = strtoumax(value, &end, 0); -- cgit v0.10.2-6-g49f6 From 39c575c96967f325a995cf9716a46f7e924714f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 22 Jun 2019 12:03:40 +0200 Subject: config: simplify parsing of unit factors Just return the value of the factor or zero for unrecognized strings instead of using an output reference and a separate return value to indicate success. This is shorter and simpler. It basically reverts that function to before c8deb5a146 ("Improve error messages when int/long cannot be parsed from config", 2007-12-25), while keeping the better messages, so restore its old name, get_unit_factor(), as well. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano diff --git a/config.c b/config.c index a8bd1d8..26196bd 100644 --- a/config.c +++ b/config.c @@ -834,24 +834,16 @@ static int git_parse_source(config_fn_t fn, void *data, return error_return; } -static int parse_unit_factor(const char *end, uintmax_t *factor) +static uintmax_t get_unit_factor(const char *end) { - if (!*end) { - *factor = 1; + if (!*end) return 1; - } - else if (!strcasecmp(end, "k")) { - *factor = 1024; - return 1; - } - else if (!strcasecmp(end, "m")) { - *factor = 1024 * 1024; - return 1; - } - else if (!strcasecmp(end, "g")) { - *factor = 1024 * 1024 * 1024; - return 1; - } + else if (!strcasecmp(end, "k")) + return 1024; + else if (!strcasecmp(end, "m")) + return 1024 * 1024; + else if (!strcasecmp(end, "g")) + return 1024 * 1024 * 1024; return 0; } @@ -867,7 +859,8 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max) val = strtoimax(value, &end, 0); if (errno == ERANGE) return 0; - if (!parse_unit_factor(end, &factor)) { + factor = get_unit_factor(end); + if (!factor) { errno = EINVAL; return 0; } @@ -896,7 +889,8 @@ static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max) val = strtoumax(value, &end, 0); if (errno == ERANGE) return 0; - if (!parse_unit_factor(end, &factor)) { + factor = get_unit_factor(end); + if (!factor) { errno = EINVAL; return 0; } -- cgit v0.10.2-6-g49f6