threads / patch / 28825

patch, 2 partsSupport sizes >=2G in various config options, v2

Subject: [PATCH 0/2] Support sizes >=2G in various config options, v2

## tl;dr

3 messages between Nov 2, 2011 and Nov 2, 2011. Diffs are folded; open one to read it.

replies: 2people: 1as markdown or json

Nick Alcock· Nov 2, 2011, 15:46 UTC · lore
New in this version:
 - overflow detection, as suggested by Johannes Sixt (on 32-bit
   platforms too).
 - no renaming of NO_STRTOUMAX nor NO_STRTOULL.

I think this covers all the bases, including detection of configuration values that overflow signed but not unsigned type only after factor-application (as '3g' would on a 32-bit Linux box).

No new git testsuite failures. (No tests either, because I can't think of one that will reliably induce overflow on a 64-bit box without being totally ludicrous.)

Nick Alcock (2):
  Add strtoimax() compatibility function.
  Support sizes >=2G in various config options accepting 'g' sizes.
 Makefile           |    6 +++---
 compat/strtoimax.c |   10 ++++++++++
 config.c           |   43 +++++++++++++++++++++++++++++++++----------
 3 files changed, 46 insertions(+), 13 deletions(-)
 create mode 100644 compat/strtoimax.c
-- 
1.7.6.1.138.g03ab.dirty
Nick Alcock· Nov 2, 2011, 15:46 UTC · re: Nick Alcock · lore

[PATCH 1/2] Add strtoimax() compatibility function.

Since systems that omit strtoumax() will likely omit strtomax() too, and likewise for strtoull() and strtoll(), we arrange for the compatibility #defines NO_STRTOUMAX and NO_STRTOULL to cover both the signed and unsigned functions. (We cannot change their names without breaking existing makefile configurations.)

Signed-off-by: Nick Alcock <nix@esperi.org.uk>
---
 Makefile           |    6 +++---
 compat/strtoimax.c |   10 ++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 compat/strtoimax.c
Show changes to 2 files +13 −3

Makefile, compat/strtoimax.c

diff --git a/Makefile b/Makefile
index 303a8df..a1f7e34 100644
--- a/Makefile
+++ b/Makefile
@@ -58,8 +58,8 @@ include /home/compiler/.configure/site.mk
 #
 # Define NO_STRLCPY if you don't have strlcpy.
 #
-# Define NO_STRTOUMAX if you don't have strtoumax in the C library.
-# If your compiler also does not support long long or does not have
+# Define NO_STRTOUMAX if you don't have both strtoimax and strtoumax in the
+# C library. If your compiler also does not support long long or does not have
 # strtoull, define NO_STRTOULL.
 #
 # Define NO_SETENV if you don't have setenv in the C library.
@@ -1459,7 +1459,7 @@ ifdef NO_STRLCPY
 endif
 ifdef NO_STRTOUMAX
 	COMPAT_CFLAGS += -DNO_STRTOUMAX
-	COMPAT_OBJS += compat/strtoumax.o
+	COMPAT_OBJS += compat/strtoumax.o compat/strtoimax.o
 endif
 ifdef NO_STRTOULL
 	COMPAT_CFLAGS += -DNO_STRTOULL
diff --git a/compat/strtoimax.c b/compat/strtoimax.c
new file mode 100644
index 0000000..ac09ed8
--- /dev/null
+++ b/compat/strtoimax.c
@@ -0,0 +1,10 @@
+#include "../git-compat-util.h"
+
+intmax_t gitstrtoimax (const char *nptr, char **endptr, int base)
+{
+#if defined(NO_STRTOULL)
+	return strtol(nptr, endptr, base);
+#else
+	return strtoll(nptr, endptr, base);
+#endif
+}
-- 
1.7.6.1.138.g03ab.dirty
Nick Alcock· Nov 2, 2011, 15:46 UTC · re: Nick Alcock · lore

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

The config options core.packedGitWindowSize, core.packedGitLimit, core.deltaBaseCacheLimit, core.bigFileThreshold, pack.windowMemory and pack.packSizeLimit 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.)

This fixes things for 32-bit platforms as well. They get the usual bad config error if an overlarge value is specified, e.g.:

fatal: bad config value for 'core.bigfilethreshold' in /home/nix/.gitconfig

This is detected in all cases, even if the 32-bit platform has no size larger than 'long'. For signed integral configuration values, we also detect the case where the value is too large for the signed type but not the unsigned type.

Signed-off-by: Nick Alcock <nix@esperi.org.uk>
---
 config.c |   41 +++++++++++++++++++++++++++++++----------
 1 files changed, 31 insertions(+), 10 deletions(-)
Show changes to config.c +31 −10
diff --git a/config.c b/config.c
index edf9914..84ca156 100644
--- a/config.c
+++ b/config.c
@@ -333,7 +333,7 @@ static int git_parse_file(config_fn_t fn, void *data)
 	die("bad config file line %d in %s", cf->linenr, cf->name);
 }
 
-static int parse_unit_factor(const char *end, unsigned long *val)
+static int parse_unit_factor(const char *end, uintmax_t *val)
 {
 	if (!*end)
 		return 1;
@@ -356,11 +356,23 @@ static int git_parse_long(const char *value, long *ret)
 {
 	if (value && *value) {
 		char *end;
-		long val = strtol(value, &end, 0);
-		unsigned long factor = 1;
+		intmax_t val;
+		uintmax_t uval;
+		uintmax_t factor = 1;
+
+		errno = 0;
+		val = strtoimax(value, &end, 0);
+		if (errno == ERANGE)
+			return 0;
 		if (!parse_unit_factor(end, &factor))
 			return 0;
-		*ret = val * factor;
+		uval = abs(val);
+		uval *= factor;
+		if ((uval > maximum_signed_value_of_type(long)) ||
+		    (abs(val) > uval))
+			return 0;
+		val *= factor;
+		*ret = val;
 		return 1;
 	}
 	return 0;
@@ -370,9 +382,19 @@ int git_parse_ulong(const char *value, unsigned long *ret)
 {
 	if (value && *value) {
 		char *end;
-		unsigned long val = strtoul(value, &end, 0);
+		uintmax_t val;
+		uintmax_t oldval;
+
+		errno = 0;
+		val = strtoumax(value, &end, 0);
+		if (errno == ERANGE)
+			return 0;
+		oldval = val;
 		if (!parse_unit_factor(end, &val))
 			return 0;
+		if ((val > maximum_unsigned_value_of_type(long)) ||
+		    (oldval > val))
+			return 0;
 		*ret = val;
 		return 1;
 	}
@@ -553,7 +575,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;
@@ -564,18 +586,17 @@ static int git_default_core_config(const char *var, const char *value)
 	}
 
 	if (!strcmp(var, "core.bigfilethreshold")) {
-		long n = git_config_int(var, value);
-		big_file_threshold = 0 < n ? n : 0;
+		big_file_threshold = git_config_ulong(var, value);
 		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

← back to recent threads