overflow checking: don't make callers set oom

Have the ALLOC_OVERFLOW testing macros also simply set_oom in the
case where a computation would overflow, so that callers don't
need to.

12 files changed, 36 insertions, 43 deletions

diff --git a/src/buffer.c b/src/buffer.c
index 8098bb1e7..0d0314439 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -105,7 +105,6 @@ int git_buf_grow_by(git_buf *buffer, size_t additional_size)
 {
 	if (GIT_ALLOC_OVERFLOW_ADD(buffer->size, additional_size)) {
 		buffer->ptr = git_buf__oom;
-		giterr_set_oom();
 		return -1;
 	}
 
diff --git a/src/common.h b/src/common.h
index b53798eaf..8b7d6936d 100644
--- a/src/common.h
+++ b/src/common.h
@@ -176,25 +176,19 @@ GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int v
 
 /** Check for integer overflow from addition or multiplication */
 #define GIT_ALLOC_OVERFLOW_ADD(one, two) \
-	((one) + (two) < (one))
+	(((one) + (two) < (one)) ? (giterr_set_oom(), 1) : 0)
 
 /** Check for integer overflow from multiplication */
 #define GIT_ALLOC_OVERFLOW_MULTIPLY(one, two) \
-	(one && ((one) * (two)) / (one) != (two))
+	((one && ((one) * (two)) / (one) != (two)) ? (giterr_set_oom(), 1) : 0)
 
 /** Check for additive overflow, failing if it would occur. */
 #define GITERR_CHECK_ALLOC_ADD(one, two) \
-	if (GIT_ALLOC_OVERFLOW_ADD(one, two)) { \
-		giterr_set_oom(); \
-		return -1; \
-	}
+	if (GIT_ALLOC_OVERFLOW_ADD(one, two)) { return -1; }
 
 /** Check for multiplicative overflow, failing if it would occur. */
 #define GITERR_CHECK_ALLOC_MULTIPLY(nelem, elsize) \
-	if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize)) { \
-		giterr_set_oom(); \
-		return -1; \
-	}
+	if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize)) { return -1; }
 
 /* NOTE: other giterr functions are in the public errors.h header file */
 
diff --git a/src/config_file.c b/src/config_file.c
index 36f78563b..39e9ff841 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -905,7 +905,6 @@ static char *reader_readline(struct reader *reader, bool skip_whitespace)
 
 	if (GIT_ALLOC_OVERFLOW_ADD(line_len, 1) ||
 		(line = git__malloc(line_len + 1)) == NULL) {
-		giterr_set_oom();
 		return NULL;
 	}
 
@@ -1620,7 +1619,6 @@ static char *fixup_line(const char *ptr, int quote_count)
 
 	if (GIT_ALLOC_OVERFLOW_ADD(ptr_len, 1) ||
 		(str = git__malloc(ptr_len + 1)) == NULL) {
-		giterr_set_oom();
 		return NULL;
 	}
 
diff --git a/src/delta.c b/src/delta.c
index f704fdfb1..242f3abe3 100644
--- a/src/delta.c
+++ b/src/delta.c
@@ -138,7 +138,7 @@ static int lookup_index_alloc(
 	index_len += hash_len;
 
 	if (!git__is_ulong(index_len)) {
-		giterr_set_oom();
+		giterr_set(GITERR_NOMEMORY, "Overly large delta");
 		return -1;
 	}
 
diff --git a/src/index.c b/src/index.c
index 70090d12c..35f512ca4 100644
--- a/src/index.c
+++ b/src/index.c
@@ -833,10 +833,8 @@ static git_index_reuc_entry *reuc_entry_alloc(const char *path)
 	struct reuc_entry_internal *entry;
 
 	if (GIT_ALLOC_OVERFLOW_ADD(structlen, pathlen) ||
-		GIT_ALLOC_OVERFLOW_ADD(structlen + pathlen, 1)) {
-		giterr_set_oom();
+		GIT_ALLOC_OVERFLOW_ADD(structlen + pathlen, 1))
 		return NULL;
-	}
 
 	entry = git__calloc(1, structlen + pathlen + 1);
 	if (!entry)
diff --git a/src/pack-objects.c b/src/pack-objects.c
index cd0deb29a..288077078 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -504,10 +504,8 @@ static git_pobject **compute_write_order(git_packbuilder *pb)
 	unsigned int i, wo_end, last_untagged;
 	git_pobject **wo;
 
-	if ((wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL) {
-		giterr_set_oom();
+	if ((wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL)
 		return NULL;
-	}
 
 	for (i = 0; i < pb->nr_objects; i++) {
 		git_pobject *po = pb->object_list + i;
diff --git a/src/pool.c b/src/pool.c
index 1599fc7a2..919e13d08 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -117,10 +117,8 @@ static void *pool_alloc_page(git_pool *pool, uint32_t size)
 	}
 
 	if (GIT_ALLOC_OVERFLOW_ADD(alloc_size, sizeof(git_pool_page)) ||
-		!(page = git__calloc(1, alloc_size + sizeof(git_pool_page)))) {
-		giterr_set_oom();
+		!(page = git__calloc(1, alloc_size + sizeof(git_pool_page))))
 		return NULL;
-	}
 
 	page->size  = alloc_size;
 	page->avail = alloc_size - size;
diff --git a/src/refs.c b/src/refs.c
index 33e931db5..af3190ef9 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -41,10 +41,8 @@ static git_reference *alloc_ref(const char *name)
 
 	if (GIT_ALLOC_OVERFLOW_ADD(reflen, namelen) ||
 		GIT_ALLOC_OVERFLOW_ADD(reflen + namelen, 1) ||
-		(ref = git__calloc(1, reflen + namelen + 1)) == NULL) {
-		giterr_set_oom();
+		(ref = git__calloc(1, reflen + namelen + 1)) == NULL)
 		return NULL;
-	}
 
 	memcpy(ref->name, name, namelen + 1);
 
diff --git a/src/tree.c b/src/tree.c
index 2c8b89291..573e56447 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -89,10 +89,8 @@ static git_tree_entry *alloc_entry(const char *filename)
 
 	if (GIT_ALLOC_OVERFLOW_ADD(tree_len, filename_len) ||
 		GIT_ALLOC_OVERFLOW_ADD(tree_len + filename_len, 1) ||
-		!(entry = git__malloc(tree_len + filename_len + 1))) {
-		giterr_set_oom();
+		!(entry = git__malloc(tree_len + filename_len + 1)))
 		return NULL;
-	}
 
 	memset(entry, 0x0, sizeof(git_tree_entry));
 	memcpy(entry->filename, filename, filename_len);
diff --git a/src/util.h b/src/util.h
index 7693ef809..40e06976d 100644
--- a/src/util.h
+++ b/src/util.h
@@ -64,10 +64,8 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n)
 
 	length = p_strnlen(str, n);
 
-	if (GIT_ALLOC_OVERFLOW_ADD(length, 1)) {
-		giterr_set_oom();
+	if (GIT_ALLOC_OVERFLOW_ADD(length, 1))
 		return NULL;
-	}
 
 	ptr = git__malloc(length + 1);
 
@@ -87,10 +85,8 @@ GIT_INLINE(char *) git__substrdup(const char *start, size_t n)
 {
 	char *ptr;
 
-	if (GIT_ALLOC_OVERFLOW_ADD(n, 1) || !(ptr = git__malloc(n+1))) {
-		giterr_set_oom();
+	if (GIT_ALLOC_OVERFLOW_ADD(n, 1) || !(ptr = git__malloc(n+1)))
 		return NULL;
-	}
 
 	memcpy(ptr, start, n);
 	ptr[n] = '\0';
@@ -111,11 +107,8 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size)
  */
 GIT_INLINE(void *) git__reallocarray(void *ptr, size_t nelem, size_t elsize)
 {
-	void *new_ptr = NULL;
-	if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize) ||
-		!(new_ptr = realloc(ptr, nelem * elsize)))
-		giterr_set_oom();
-	return new_ptr;
+	return GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize) ?
+		NULL : realloc(ptr, nelem * elsize);
 }
 
 /**
diff --git a/src/win32/dir.c b/src/win32/dir.c
index 9953289f6..7f2a5a56e 100644
--- a/src/win32/dir.c
+++ b/src/win32/dir.c
@@ -20,10 +20,8 @@ git__DIR *git__opendir(const char *dir)
 
 	if (GIT_ALLOC_OVERFLOW_ADD(sizeof(*new), dirlen) ||
 		GIT_ALLOC_OVERFLOW_ADD(sizeof(*new) + dirlen, 1) ||
-		!(new = git__calloc(1, sizeof(*new) + dirlen + 1))) {
-		giterr_set_oom();
+		!(new = git__calloc(1, sizeof(*new) + dirlen + 1)))
 		return NULL;
-	}
 
 	memcpy(new->dir, dir, dirlen);
 
diff --git a/tests/core/errors.c b/tests/core/errors.c
index b6364100b..11e0bb2bb 100644
--- a/tests/core/errors.c
+++ b/tests/core/errors.c
@@ -149,3 +149,24 @@ void test_core_errors__integer_overflow_alloc_add(void)
 	cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass);
 	cl_assert_equal_s("Out of memory", giterr_last()->message);
 }
+
+void test_core_errors__integer_overflow_sets_oom(void)
+{
+	giterr_clear();
+	cl_assert(!GIT_ALLOC_OVERFLOW_ADD(SIZE_MAX-1, 1));
+	cl_assert_equal_p(NULL, giterr_last());
+
+	giterr_clear();
+	cl_assert(!GIT_ALLOC_OVERFLOW_ADD(42, 69));
+	cl_assert_equal_p(NULL, giterr_last());
+
+	giterr_clear();
+	cl_assert(GIT_ALLOC_OVERFLOW_ADD(SIZE_MAX, SIZE_MAX));
+	cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass);
+	cl_assert_equal_s("Out of memory", giterr_last()->message);
+
+	giterr_clear();
+	cl_assert(GIT_ALLOC_OVERFLOW_MULTIPLY(SIZE_MAX, SIZE_MAX));
+	cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass);
+	cl_assert_equal_s("Out of memory", giterr_last()->message);
+}