Commit b709e95146b9d56e2c009915ccea7a86c77d4202

Russell Belfer 2012-05-04T11:06:12

Fix memory leaks and use after free

diff --git a/include/git2/attr.h b/include/git2/attr.h
index dfa1d27..6a05496 100644
--- a/include/git2/attr.h
+++ b/include/git2/attr.h
@@ -160,10 +160,10 @@ GIT_EXTERN(int) git_attr_get(
  *             array itself if you allocated it).
  */
 GIT_EXTERN(int) git_attr_get_many(
-    git_repository *repo,
+	git_repository *repo,
 	uint32_t flags,
 	const char *path,
-    size_t num_attr,
+	size_t num_attr,
 	const char **names,
 	const char **values);
 
@@ -186,7 +186,7 @@ GIT_EXTERN(int) git_attr_get_many(
  * @param payload Passed on as extra parameter to callback function.
  */
 GIT_EXTERN(int) git_attr_foreach(
-    git_repository *repo,
+	git_repository *repo,
 	uint32_t flags,
 	const char *path,
 	int (*callback)(const char *name, const char *value, void *payload),
diff --git a/src/attr.c b/src/attr.c
index 56d04d3..b7ac635 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -334,8 +334,6 @@ int git_attr_cache__push_file(
 	}
 
 	/* if not in cache, load data, parse, and cache */
-	if (git_attr_file__new(&file, source, relfile, &cache->pool) < 0)
-		return -1;
 
 	if (source == GIT_ATTR_FILE_FROM_FILE)
 		error = load_attr_file(filename, &content);
@@ -354,6 +352,9 @@ int git_attr_cache__push_file(
 	if (blob)
 		content = git_blob_rawcontent(blob);
 
+	if ((error = git_attr_file__new(&file, source, relfile, &cache->pool)) < 0)
+		goto finish;
+
 	if (parse && (error = parse(repo, content, file)) < 0)
 		goto finish;
 
diff --git a/src/config_file.c b/src/config_file.c
index ed5caf9..746d965 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -265,6 +265,7 @@ static int config_set(git_config_file *cfg, const char *name, const char *value)
 		cvar_free(old_var);
 
 	if (config_write(b, key, NULL, value) < 0) {
+		git_strmap_delete(b->values, var->key);
 		cvar_free(var);
 		return -1;
 	}
diff --git a/src/diff.c b/src/diff.c
index b845f9e..524cc9f 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -506,7 +506,7 @@ static int diff_from_iterators(
 	git_diff_list **diff_ptr)
 {
 	const git_index_entry *oitem, *nitem;
-	char *ignore_prefix = NULL;
+	git_buf ignore_prefix = GIT_BUF_INIT;
 	git_diff_list *diff = git_diff_list_alloc(repo, opts);
 	if (!diff)
 		goto fail;
@@ -536,8 +536,8 @@ static int diff_from_iterators(
 			git_delta_t delta_type = GIT_DELTA_ADDED;
 
 			/* contained in ignored parent directory, so this can be skipped. */
-			if (ignore_prefix != NULL &&
-				git__prefixcmp(nitem->path, ignore_prefix) == 0)
+			if (git_buf_len(&ignore_prefix) &&
+				git__prefixcmp(nitem->path, git_buf_cstr(&ignore_prefix)) == 0)
 			{
 				if (git_iterator_advance(new_iter, &nitem) < 0)
 					goto fail;
@@ -555,7 +555,7 @@ static int diff_from_iterators(
 					(oitem && git__prefixcmp(oitem->path, nitem->path) == 0))
 				{
 					if (is_ignored)
-						ignore_prefix = nitem->path;
+						git_buf_sets(&ignore_prefix, nitem->path);
 
 					if (git_iterator_advance_into_directory(new_iter, &nitem) < 0)
 						goto fail;
@@ -589,12 +589,16 @@ static int diff_from_iterators(
 
 	git_iterator_free(old_iter);
 	git_iterator_free(new_iter);
+	git_buf_free(&ignore_prefix);
+
 	*diff_ptr = diff;
 	return 0;
 
 fail:
 	git_iterator_free(old_iter);
 	git_iterator_free(new_iter);
+	git_buf_free(&ignore_prefix);
+
 	git_diff_list_free(diff);
 	*diff_ptr = NULL;
 	return -1;
diff --git a/src/ignore.c b/src/ignore.c
index 6f70b97..fc6194b 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -8,7 +8,7 @@
 static int parse_ignore_file(
 	git_repository *repo, const char *buffer, git_attr_file *ignores)
 {
-	int error;
+	int error = 0;
 	git_attr_fnmatch *match = NULL;
 	const char *scan = NULL;
 	char *context = NULL;
diff --git a/src/strmap.h b/src/strmap.h
index 55fbd7c..da5ca0d 100644
--- a/src/strmap.h
+++ b/src/strmap.h
@@ -36,18 +36,28 @@ typedef khash_t(str) git_strmap;
 #define git_strmap_set_value_at(h, idx, v) kh_val(h, idx) = v
 #define git_strmap_delete_at(h, idx)       kh_del(str, h, idx)
 
-#define git_strmap_insert(h, key, val, err) do { \
-	khiter_t __pos = kh_put(str, h, key, &err); \
-	if (err >= 0) kh_val(h, __pos) = val; \
-	} while (0)
-
-#define git_strmap_insert2(h, key, val, old, err) do { \
-	khiter_t __pos = kh_put(str, h, key, &err); \
-	if (err >= 0) { \
-		old = (err == 0) ? kh_val(h, __pos) : NULL; \
+#define git_strmap_insert(h, key, val, rval) do { \
+	khiter_t __pos = kh_put(str, h, key, &rval); \
+	if (rval >= 0) { \
+		if (rval == 0) kh_key(h, __pos) = key; \
 		kh_val(h, __pos) = val; \
 	} } while (0)
 
+#define git_strmap_insert2(h, key, val, oldv, rval) do { \
+	khiter_t __pos = kh_put(str, h, key, &rval); \
+	if (rval >= 0) { \
+		if (rval == 0) { \
+			oldv = kh_val(h, __pos); \
+			kh_key(h, __pos) = key; \
+		} else { oldv = NULL; } \
+		kh_val(h, __pos) = val; \
+	} } while (0)
+
+#define git_strmap_delete(h, key) do { \
+	khiter_t __pos = git_strmap_lookup_index(h, key); \
+	if (git_strmap_valid_index(h, __pos)) \
+		git_strmap_delete_at(h, __pos); } while (0)
+
 #define git_strmap_foreach		kh_foreach
 #define git_strmap_foreach_value	kh_foreach_value
 
diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c
index 3fb4877..06f51a1 100644
--- a/tests-clar/diff/tree.c
+++ b/tests-clar/diff/tree.c
@@ -205,4 +205,6 @@ void test_diff_tree__bare(void)
 	cl_assert(exp.line_dels == 1);
 
 	git_diff_list_free(diff);
+	git_tree_free(a);
+	git_tree_free(b);
 }