Commit 19c88310cb79153e19b93c59020b2f7c34794f6e

Carlos Martín Nieto 2014-07-10T13:48:13

tree-cache: move to use a pool allocator This simplifies freeing the entries quite a bit; though there aren't that many failure paths right now, introducing filling the cache from a tree will introduce more. This makes sure not to leak memory on errors.

diff --git a/src/index.c b/src/index.c
index 8b757f2..8ea1fd4 100644
--- a/src/index.c
+++ b/src/index.c
@@ -405,6 +405,8 @@ int git_index_open(git_index **index_out, const char *index_path)
 		return -1;
 	}
 
+	git_pool_init(&index->tree_pool, 1, 0);
+
 	if (index_path != NULL) {
 		index->index_file_path = git__strdup(index_path);
 		if (!index->index_file_path)
@@ -435,6 +437,7 @@ int git_index_open(git_index **index_out, const char *index_path)
 	return 0;
 
 fail:
+	git_pool_clear(&index->tree_pool);
 	git_index_free(index);
 	return error;
 }
@@ -517,8 +520,8 @@ int git_index_clear(git_index *index)
 
 	assert(index);
 
-	git_tree_cache_free(index->tree);
 	index->tree = NULL;
+	git_pool_clear(&index->tree_pool);
 
 	if (git_mutex_lock(&index->lock) < 0) {
 		giterr_set(GITERR_OS, "Failed to lock index");
@@ -621,6 +624,9 @@ int git_index_read(git_index *index, int force)
 	if (error < 0)
 		return error;
 
+	index->tree = NULL;
+	git_pool_clear(&index->tree_pool);
+
 	error = git_index_clear(index);
 
 	if (!error)
@@ -1871,7 +1877,7 @@ static size_t read_extension(git_index *index, const char *buffer, size_t buffer
 	if (dest.signature[0] >= 'A' && dest.signature[0] <= 'Z') {
 		/* tree cache */
 		if (memcmp(dest.signature, INDEX_EXT_TREECACHE_SIG, 4) == 0) {
-			if (git_tree_cache_read(&index->tree, buffer + 8, dest.extension_size) < 0)
+			if (git_tree_cache_read(&index->tree, buffer + 8, dest.extension_size, &index->tree_pool) < 0)
 				return 0;
 		} else if (memcmp(dest.signature, INDEX_EXT_UNMERGED_SIG, 4) == 0) {
 			if (read_reuc(index, buffer + 8, dest.extension_size) < 0)
diff --git a/src/index.h b/src/index.h
index 50a0b4b..2eb93fb 100644
--- a/src/index.h
+++ b/src/index.h
@@ -35,6 +35,7 @@ struct git_index {
 	unsigned int no_symlinks:1;
 
 	git_tree_cache *tree;
+	git_pool tree_pool;
 
 	git_vector names;
 	git_vector reuc;
diff --git a/src/tree-cache.c b/src/tree-cache.c
index aec9493..d76d928 100644
--- a/src/tree-cache.c
+++ b/src/tree-cache.c
@@ -6,6 +6,7 @@
  */
 
 #include "tree-cache.h"
+#include "pool.h"
 
 static git_tree_cache *find_child(
 	const git_tree_cache *tree, const char *path, const char *end)
@@ -69,7 +70,8 @@ const git_tree_cache *git_tree_cache_get(const git_tree_cache *tree, const char 
 }
 
 static int read_tree_internal(git_tree_cache **out,
-		const char **buffer_in, const char *buffer_end, git_tree_cache *parent)
+			      const char **buffer_in, const char *buffer_end,
+			      git_tree_cache *parent, git_pool *pool)
 {
 	git_tree_cache *tree = NULL;
 	const char *name_start, *buffer;
@@ -83,7 +85,7 @@ static int read_tree_internal(git_tree_cache **out,
 	if (++buffer >= buffer_end)
 		goto corrupted;
 
-	if (git_tree_cache_new(&tree, name_start, parent) < 0)
+	if (git_tree_cache_new(&tree, name_start, parent, pool) < 0)
 		return -1;
 
 	/* Blank-terminated ASCII decimal number of entries in this tree */
@@ -118,13 +120,13 @@ static int read_tree_internal(git_tree_cache **out,
 	if (tree->children_count > 0) {
 		unsigned int i;
 
-		tree->children = git__malloc(tree->children_count * sizeof(git_tree_cache *));
+		tree->children = git_pool_malloc(pool, tree->children_count * sizeof(git_tree_cache *));
 		GITERR_CHECK_ALLOC(tree->children);
 
 		memset(tree->children, 0x0, tree->children_count * sizeof(git_tree_cache *));
 
 		for (i = 0; i < tree->children_count; ++i) {
-			if (read_tree_internal(&tree->children[i], &buffer, buffer_end, tree) < 0)
+			if (read_tree_internal(&tree->children[i], &buffer, buffer_end, tree, pool) < 0)
 				goto corrupted;
 		}
 	}
@@ -134,16 +136,15 @@ static int read_tree_internal(git_tree_cache **out,
 	return 0;
 
  corrupted:
-	git_tree_cache_free(tree);
 	giterr_set(GITERR_INDEX, "Corrupted TREE extension in index");
 	return -1;
 }
 
-int git_tree_cache_read(git_tree_cache **tree, const char *buffer, size_t buffer_size)
+int git_tree_cache_read(git_tree_cache **tree, const char *buffer, size_t buffer_size, git_pool *pool)
 {
 	const char *buffer_end = buffer + buffer_size;
 
-	if (read_tree_internal(tree, &buffer, buffer_end, NULL) < 0)
+	if (read_tree_internal(tree, &buffer, buffer_end, NULL, pool) < 0)
 		return -1;
 
 	if (buffer < buffer_end) {
@@ -154,13 +155,13 @@ int git_tree_cache_read(git_tree_cache **tree, const char *buffer, size_t buffer
 	return 0;
 }
 
-int git_tree_cache_new(git_tree_cache **out, const char *name, git_tree_cache *parent)
+int git_tree_cache_new(git_tree_cache **out, const char *name, git_tree_cache *parent, git_pool *pool)
 {
 	size_t name_len;
 	git_tree_cache *tree;
 
 	name_len = strlen(name);
-	tree = git__malloc(sizeof(git_tree_cache) + name_len + 1);
+	tree = git_pool_malloc(pool, sizeof(git_tree_cache) + name_len + 1);
 	GITERR_CHECK_ALLOC(tree);
 
 	memset(tree, 0x0, sizeof(git_tree_cache));
@@ -173,20 +174,3 @@ int git_tree_cache_new(git_tree_cache **out, const char *name, git_tree_cache *p
 	*out = tree;
 	return 0;
 }
-
-void git_tree_cache_free(git_tree_cache *tree)
-{
-	unsigned int i;
-
-	if (tree == NULL)
-		return;
-
-	if (tree->children != NULL) {
-		for (i = 0; i < tree->children_count; ++i)
-			git_tree_cache_free(tree->children[i]);
-
-		git__free(tree->children);
-	}
-
-	git__free(tree);
-}
diff --git a/src/tree-cache.h b/src/tree-cache.h
index 592a352..d41a51f 100644
--- a/src/tree-cache.h
+++ b/src/tree-cache.h
@@ -9,6 +9,7 @@
 #define INCLUDE_tree_cache_h__
 
 #include "common.h"
+#include "pool.h"
 #include "git2/oid.h"
 
 typedef struct {
@@ -22,10 +23,10 @@ typedef struct {
 	char name[GIT_FLEX_ARRAY];
 } git_tree_cache;
 
-int git_tree_cache_read(git_tree_cache **tree, const char *buffer, size_t buffer_size);
+int git_tree_cache_read(git_tree_cache **tree, const char *buffer, size_t buffer_size, git_pool *pool);
 void git_tree_cache_invalidate_path(git_tree_cache *tree, const char *path);
 const git_tree_cache *git_tree_cache_get(const git_tree_cache *tree, const char *path);
-int git_tree_cache_new(git_tree_cache **out, const char *name, git_tree_cache *parent);
+int git_tree_cache_new(git_tree_cache **out, const char *name, git_tree_cache *parent, git_pool *pool);
 void git_tree_cache_free(git_tree_cache *tree);
 
 #endif