Merge pull request #421 from nulltoken/ntk/fix/config-get-set-long

config: make git_config_[get|set]_long() able to properly deal with 8 bytes wide values

include/git2/config.h

@@ -164,7 +164,7 @@ GIT_EXTERN(int) git_config_get_int(git_config *cfg, const char *name, int *out);
  * @param out pointer to the variable where the value should be stored
  * @return GIT_SUCCESS or an error code
  */
-GIT_EXTERN(int) git_config_get_long(git_config *cfg, const char *name, long int *out);
+GIT_EXTERN(int) git_config_get_long(git_config *cfg, const char *name, long long *out);
 
 /**
  * Get the value of a boolean config variable.
@@ -210,7 +210,7 @@ GIT_EXTERN(int) git_config_set_int(git_config *cfg, const char *name, int value)
  * @param value Long integer value for the variable
  * @return GIT_SUCCESS or an error code
  */
-GIT_EXTERN(int) git_config_set_long(git_config *cfg, const char *name, long int value);
+GIT_EXTERN(int) git_config_set_long(git_config *cfg, const char *name, long long value);
 
 /**
  * Set the value of a boolean config variable.

src/config.c

@@ -161,16 +161,16 @@ int git_config_delete(git_config *cfg, const char *name)
  * Setters
  **************/
 
-int git_config_set_long(git_config *cfg, const char *name, long int value)
+int git_config_set_long(git_config *cfg, const char *name, long long value)
 {
 	char str_value[32]; /* All numbers should fit in here */
-	p_snprintf(str_value, sizeof(str_value), "%ld", value);
+	p_snprintf(str_value, sizeof(str_value), "%lld", value);
 	return git_config_set_string(cfg, name, str_value);
 }
 
 int git_config_set_int(git_config *cfg, const char *name, int value)
 {
-	return git_config_set_long(cfg, name, value);
+	return git_config_set_long(cfg, name, (long long)value);
 }
 
 int git_config_set_bool(git_config *cfg, const char *name, int value)
@@ -196,19 +196,19 @@ int git_config_set_string(git_config *cfg, const char *name, const char *value)
  * Getters
  ***********/
 
-int git_config_get_long(git_config *cfg, const char *name, long int *out)
+int git_config_get_long(git_config *cfg, const char *name, long long *out)
 {
 	const char *value, *num_end;
 	int ret;
-	long int num;
+	long long num;
 
 	ret = git_config_get_string(cfg, name, &value);
 	if (ret < GIT_SUCCESS)
-		return git__rethrow(ret, "Failed to get value for %s", name);
+		return git__rethrow(ret, "Failed to retrieve value for '%s'", name);
 
-	ret = git__strtol32(&num, value, &num_end, 0);
+	ret = git__strtol64(&num, value, &num_end, 0);
 	if (ret < GIT_SUCCESS)
-		return git__rethrow(ret, "Failed to get value for %s", name);
+		return git__rethrow(ret, "Failed to convert value for '%s'", name);
 
 	switch (*num_end) {
 	case '\0':
@@ -226,7 +226,7 @@ int git_config_get_long(git_config *cfg, const char *name, long int *out)
 		num *= 1024 * 1024 * 1024;
 		break;
 	default:
-		return git__throw(GIT_EINVALIDTYPE, "Failed to get value for %s. Value is of invalid type", name);
+		return git__throw(GIT_EINVALIDTYPE, "Failed to get value for '%s'. Value is of invalid type", name);
 	}
 
 	*out = num;
@@ -236,12 +236,18 @@ int git_config_get_long(git_config *cfg, const char *name, long int *out)
 
 int git_config_get_int(git_config *cfg, const char *name, int *out)
 {
-	long int tmp;
-	int ret;
+	long long tmp_long;
+	int tmp_int, ret;
 
-	ret = git_config_get_long(cfg, name, &tmp);
+	ret = git_config_get_long(cfg, name, &tmp_long);
+	if (ret < GIT_SUCCESS)
+		return git__rethrow(ret, "Failed to convert value for '%s'", name);
+	
+	tmp_int = tmp_long & 0xFFFFFFFF;
+	if (tmp_int != tmp_long)
+		return git__throw(GIT_EOVERFLOW, "Value for '%s' is too large", name);
 
-	*out = (int) tmp;
+	*out = tmp_int;
 
 	return ret;
 }

src/index.c

@@ -543,7 +543,7 @@ static int read_tree_internal(git_index_tree **out,
 {
 	git_index_tree *tree;
 	const char *name_start, *buffer;
-	long count;
+	int count;
 	int error = GIT_SUCCESS;
 
 	if ((tree = git__malloc(sizeof(git_index_tree))) == NULL)
@@ -685,7 +685,7 @@ static int read_unmerged(git_index *index, const char *buffer, size_t size)
 		buffer += len;
 
 		for (i = 0; i < 3; i++) {
-			long tmp;
+			int tmp;
 
 			if (git__strtol32(&tmp, buffer, &endptr, 8) < GIT_SUCCESS ||
 				!endptr || endptr == buffer || *endptr || (unsigned)tmp > UINT_MAX)

src/pkt.c

@@ -155,7 +155,7 @@ static ssize_t parse_len(const char *line)
 {
 	char num[PKT_LEN_SIZE + 1];
 	int i, error;
-	long len;
+	int len;
 	const char *num_end;
 
 	memcpy(num, line, PKT_LEN_SIZE);

src/revwalk.c

@@ -172,7 +172,7 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo
 	unsigned char *parents_start;
 
 	int i, parents = 0;
-	long commit_time;
+	int commit_time;
 
 	buffer += strlen("tree ") + GIT_OID_HEXSZ + 1;
 

src/signature.c

@@ -156,7 +156,7 @@ int git_signature_now(git_signature **sig_out, const char *name, const char *ema
 
 static int parse_timezone_offset(const char *buffer, int *offset_out)
 {
-	long dec_offset;
+	int dec_offset;
 	int mins, hours, offset;
 
 	const char *offset_start;
@@ -236,7 +236,7 @@ static const char *scan_for_previous_token(const char *buffer, const char *left_
 
 static int parse_time(git_time_t *time_out, const char *buffer)
 {
-	long time;
+	int time;
 	int error;
 
 	if (*buffer == '+' || *buffer == '-')

src/tree.c

@@ -157,7 +157,7 @@ static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buf
 
 	while (buffer < buffer_end) {
 		git_tree_entry *entry;
-		long tmp;
+		int tmp;
 
 		entry = git__calloc(1, sizeof(git_tree_entry));
 		if (entry == NULL) {

src/util.c

@@ -46,10 +46,10 @@ int git__fnmatch(const char *pattern, const char *name, int flags)
 	}
 }
 
-int git__strtol32(long *result, const char *nptr, const char **endptr, int base)
+int git__strtol64(long long *result, const char *nptr, const char **endptr, int base)
 {
 	const char *p;
-	long n, nn;
+	long long n, nn;
 	int c, ovfl, v, neg, ndig;
 
 	p = nptr;
@@ -124,6 +124,23 @@ Return:
 	return GIT_SUCCESS;
 }
 
+int git__strtol32(int *result, const char *nptr, const char **endptr, int base)
+{
+	int tmp_int, error = GIT_SUCCESS;
+	long long tmp_long;
+
+	if ((error = git__strtol64(&tmp_long, nptr, endptr, base)) < GIT_SUCCESS)
+		return error;
+
+	tmp_int = tmp_long & 0xFFFFFFFF;
+	if (tmp_int != tmp_long)
+		return git__throw(GIT_EOVERFLOW, "Failed to convert. '%s' is too large", nptr);
+
+	*result = tmp_int;
+	
+	return error;
+}
+
 void git__strntolower(char *str, size_t len)
 {
 	size_t i;

src/util.h

@@ -75,7 +75,8 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size)
 extern int git__prefixcmp(const char *str, const char *prefix);
 extern int git__suffixcmp(const char *str, const char *suffix);
 
-extern int git__strtol32(long *n, const char *buff, const char **end_buf, int base);
+extern int git__strtol32(int *n, const char *buff, const char **end_buf, int base);
+extern int git__strtol64(long long *n, const char *buff, const char **end_buf, int base);
 
 extern void git__hexdump(const char *buffer, size_t n);
 extern uint32_t git__hash(const void *key, int len, uint32_t seed);

tests/resources/config/config9

 [core]
 	dummy2 = 42
+	verylong = 1
 	dummy = 1

tests/t00-core.c

@@ -555,6 +555,47 @@ BEGIN_TEST(rmdir1, "make sure non-empty dir cannot be deleted recusively")
 	must_pass(git_futils_rmdir_r(empty_tmp_dir, 0));
 END_TEST
 
+BEGIN_TEST(strtol0, "parsing out 32 integers from a string")
+	int i;
+
+	must_pass(git__strtol32(&i, "123", NULL, 10));
+	must_be_true(i == 123);
+
+	must_pass(git__strtol32(&i, "  +123 ", NULL, 10));
+	must_be_true(i == 123);
+
+	must_pass(git__strtol32(&i, "  +2147483647 ", NULL, 10));
+	must_be_true(i == 2147483647);
+
+	must_pass(git__strtol32(&i, "  -2147483648 ", NULL, 10));
+	must_be_true(i == -2147483648LL);
+
+	must_fail(git__strtol32(&i, "  2147483657 ", NULL, 10));
+	must_fail(git__strtol32(&i, "  -2147483657 ", NULL, 10));
+END_TEST
+
+BEGIN_TEST(strtol1, "parsing out 64 integers from a string")
+	long long i;
+
+	must_pass(git__strtol64(&i, "123", NULL, 10));
+	must_be_true(i == 123);
+
+	must_pass(git__strtol64(&i, "  +123 ", NULL, 10));
+	must_be_true(i == 123);
+
+	must_pass(git__strtol64(&i, "  +2147483647 ", NULL, 10));
+	must_be_true(i == 2147483647);
+
+	must_pass(git__strtol64(&i, "  -2147483648 ", NULL, 10));
+	must_be_true(i == -2147483648LL);
+
+	must_pass(git__strtol64(&i, "  2147483657 ", NULL, 10));
+	must_be_true(i == 2147483657LL);
+
+	must_pass(git__strtol64(&i, "  -2147483657 ", NULL, 10));
+	must_be_true(i == -2147483657LL);
+END_TEST
+
 BEGIN_SUITE(core)
 	ADD_TEST(string0);
 	ADD_TEST(string1);
@@ -581,4 +622,7 @@ BEGIN_SUITE(core)
 
 	ADD_TEST(rmdir0);
 	ADD_TEST(rmdir1);
+
+	ADD_TEST(strtol0);
+	ADD_TEST(strtol1);
 END_SUITE

tests/t15-config.c

@@ -133,7 +133,7 @@ END_TEST
 
 BEGIN_TEST(config5, "test number suffixes")
 	git_config *cfg;
-	long int i;
+	long long i;
 
 	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config5"));
 
@@ -194,6 +194,7 @@ END_TEST
 BEGIN_TEST(config9, "replace a value")
 	git_config *cfg;
 	int i;
+	long long l, expected = +9223372036854775803;
 
 	/* By freeing the config, we make sure we flush the values  */
 	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
@@ -209,6 +210,23 @@ BEGIN_TEST(config9, "replace a value")
 	must_pass(git_config_set_int(cfg, "core.dummy", 1));
 	git_config_free(cfg);
 
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
+	must_pass(git_config_set_long(cfg, "core.verylong", expected));
+	git_config_free(cfg);
+
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
+	must_pass(git_config_get_long(cfg, "core.verylong", &l));
+	must_be_true(l == expected);
+	git_config_free(cfg);
+
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
+	must_fail(git_config_get_int(cfg, "core.verylong", &i));
+	git_config_free(cfg);
+
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
+	must_pass(git_config_set_long(cfg, "core.verylong", 1));
+	git_config_free(cfg);
+
 END_TEST
 
 BEGIN_TEST(config10, "a repo's config overrides the global config")