Convert attrs and diffs to use string pools

This converts the git attr related code (including ignores) and
the git diff related code (and implicitly the status code) to use
`git_pools` for storing strings.  This reduces the number of small
blocks allocated dramatically.

src/attr.c

@@ -167,6 +167,7 @@ int git_attr_add_macro(
 {
 	int error;
 	git_attr_rule *macro = NULL;
+	git_pool *pool;
 
 	if (git_attr_cache__init(repo) < 0)
 		return -1;
@@ -174,13 +175,15 @@ int git_attr_add_macro(
 	macro = git__calloc(1, sizeof(git_attr_rule));
 	GITERR_CHECK_ALLOC(macro);
 
-	macro->match.pattern = git__strdup(name);
+	pool = &git_repository_attr_cache(repo)->pool;
+
+	macro->match.pattern = git_pool_strdup(pool, name);
 	GITERR_CHECK_ALLOC(macro->match.pattern);
 
 	macro->match.length = strlen(macro->match.pattern);
 	macro->match.flags = GIT_ATTR_FNMATCH_MACRO;
 
-	error = git_attr_assignment__parse(repo, &macro->assigns, &values);
+	error = git_attr_assignment__parse(repo, pool, &macro->assigns, &values);
 
 	if (!error)
 		error = git_attr_cache__insert_macro(repo, macro);
@@ -221,7 +224,7 @@ int git_attr_cache__lookup_or_create_file(
 		return 0;
 	}
 
-	if (git_attr_file__new(&file) < 0)
+	if (git_attr_file__new(&file, &cache->pool) < 0)
 		return -1;
 
 	if (loader)
@@ -384,6 +387,10 @@ int git_attr_cache__init(git_repository *repo)
 			return -1;
 	}
 
+	/* allocate string pool */
+	if (git_pool_init(&cache->pool, 1, 0) < 0)
+		return -1;
+
 	cache->initialized = 1;
 
 	/* insert default macros */
@@ -393,30 +400,33 @@ int git_attr_cache__init(git_repository *repo)
 void git_attr_cache_flush(
 	git_repository *repo)
 {
-	git_hashtable *table;
+	git_attr_cache *cache;
 
 	if (!repo)
 		return;
 
-	if ((table = git_repository_attr_cache(repo)->files) != NULL) {
-		git_attr_file *file;
-
-		GIT_HASHTABLE_FOREACH_VALUE(table, file, git_attr_file__free(file));
-		git_hashtable_free(table);
+	cache = git_repository_attr_cache(repo);
 
-		git_repository_attr_cache(repo)->files = NULL;
+	if (cache->files != NULL) {
+		git_attr_file *file;
+		GIT_HASHTABLE_FOREACH_VALUE(
+			cache->files, file, git_attr_file__free(file));
+		git_hashtable_free(cache->files);
+		cache->files = NULL;
 	}
 
-	if ((table = git_repository_attr_cache(repo)->macros) != NULL) {
+	if (cache->macros != NULL) {
 		git_attr_rule *rule;
 
-		GIT_HASHTABLE_FOREACH_VALUE(table, rule, git_attr_rule__free(rule));
-		git_hashtable_free(table);
-
-		git_repository_attr_cache(repo)->macros = NULL;
+		GIT_HASHTABLE_FOREACH_VALUE(
+			cache->macros, rule, git_attr_rule__free(rule));
+		git_hashtable_free(cache->macros);
+		cache->macros = NULL;
 	}
 
-	git_repository_attr_cache(repo)->initialized = 0;
+	git_pool_clear(&cache->pool);
+
+	cache->initialized = 0;
 }
 
 int git_attr_cache__insert_macro(git_repository *repo, git_attr_rule *macro)

src/attr.h

@@ -14,6 +14,7 @@
 
 typedef struct {
 	int initialized;
+	git_pool pool;
 	git_hashtable *files;	/* hash path to git_attr_file of rules */
 	git_hashtable *macros;	/* hash name to vector<git_attr_assignment> */
 	const char *cfg_attr_file; /* cached value of core.attributesfile */

src/attr_file.c

@@ -9,21 +9,32 @@ const char *git_attr__false = "[internal]__FALSE__";
 static int sort_by_hash_and_name(const void *a_raw, const void *b_raw);
 static void git_attr_rule__clear(git_attr_rule *rule);
 
-int git_attr_file__new(git_attr_file **attrs_ptr)
+int git_attr_file__new(git_attr_file **attrs_ptr, git_pool *pool)
 {
 	git_attr_file *attrs = NULL;
 
 	attrs = git__calloc(1, sizeof(git_attr_file));
 	GITERR_CHECK_ALLOC(attrs);
 
-	if (git_vector_init(&attrs->rules, 4, NULL) < 0) {
-		git__free(attrs);
-		attrs = NULL;
+	if (pool)
+		attrs->pool = pool;
+	else {
+		attrs->pool = git__calloc(1, sizeof(git_pool));
+		if (!attrs->pool || git_pool_init(attrs->pool, 1, 0) < 0)
+			goto fail;
+		attrs->pool_is_allocated = true;
 	}
 
+	if (git_vector_init(&attrs->rules, 4, NULL) < 0)
+		goto fail;
+
 	*attrs_ptr = attrs;
+	return 0;
 
-	return attrs ? 0 : -1;
+fail:
+	git_attr_file__free(attrs);
+	attrs_ptr = NULL;
+	return -1;
 }
 
 int git_attr_file__set_path(
@@ -76,8 +87,10 @@ int git_attr_file__from_buffer(
 		}
 
 		/* parse the next "pattern attr attr attr" line */
-		if (!(error = git_attr_fnmatch__parse(&rule->match, context, &scan)) &&
-			!(error = git_attr_assignment__parse(repo, &rule->assigns, &scan)))
+		if (!(error = git_attr_fnmatch__parse(
+				&rule->match, attrs->pool, context, &scan)) &&
+			!(error = git_attr_assignment__parse(
+				repo, attrs->pool, &rule->assigns, &scan)))
 		{
 			if (rule->match.flags & GIT_ATTR_FNMATCH_MACRO)
 				/* should generate error/warning if this is coming from any
@@ -141,12 +154,18 @@ void git_attr_file__free(git_attr_file *file)
 	git__free(file->path);
 	file->path = NULL;
 
+	if (file->pool_is_allocated) {
+		git_pool_clear(file->pool);
+		git__free(file->pool);
+	}
+	file->pool = NULL;
+
 	git__free(file);
 }
 
-unsigned long git_attr_file__name_hash(const char *name)
+uint32_t git_attr_file__name_hash(const char *name)
 {
-	unsigned long h = 5381;
+	uint32_t h = 5381;
 	int c;
 	assert(name);
 	while ((c = (int)*name++) != 0)
@@ -293,6 +312,7 @@ int git_attr_path__init(
  */
 int git_attr_fnmatch__parse(
 	git_attr_fnmatch *spec,
+	git_pool *pool,
 	const char *source,
 	const char **base)
 {
@@ -358,7 +378,7 @@ int git_attr_fnmatch__parse(
 		/* given an unrooted fullpath match from a file inside a repo,
 		 * prefix the pattern with the relative directory of the source file
 		 */
-		spec->pattern = git__malloc(sourcelen + spec->length + 1);
+		spec->pattern = git_pool_malloc(pool, sourcelen + spec->length + 1);
 		if (spec->pattern) {
 			memcpy(spec->pattern, source, sourcelen);
 			memcpy(spec->pattern + sourcelen, pattern, spec->length);
@@ -366,7 +386,7 @@ int git_attr_fnmatch__parse(
 			spec->pattern[spec->length] = '\0';
 		}
 	} else {
-		spec->pattern = git__strndup(pattern, spec->length);
+		spec->pattern = git_pool_strndup(pool, pattern, spec->length);
 	}
 
 	if (!spec->pattern) {
@@ -405,14 +425,11 @@ static int sort_by_hash_and_name(const void *a_raw, const void *b_raw)
 
 static void git_attr_assignment__free(git_attr_assignment *assign)
 {
-	git__free(assign->name);
+	/* name and value are stored in a git_pool associated with the
+	 * git_attr_file, so they do not need to be freed here
+	 */
 	assign->name = NULL;
-
-	if (assign->is_allocated) {
-		git__free((void *)assign->value);
 	assign->value = NULL;
-	}
-
 	git__free(assign);
 }
 
@@ -428,6 +445,7 @@ static int merge_assignments(void **old_raw, void *new_raw)
 
 int git_attr_assignment__parse(
 	git_repository *repo,
+	git_pool *pool,
 	git_vector *assigns,
 	const char **base)
 {
@@ -454,7 +472,6 @@ int git_attr_assignment__parse(
 
 		assign->name_hash = 5381;
 		assign->value = git_attr__true;
-		assign->is_allocated = 0;
 
 		/* look for magic name prefixes */
 		if (*scan == '-') {
@@ -482,7 +499,7 @@ int git_attr_assignment__parse(
 		}
 
 		/* allocate permanent storage for name */
-		assign->name = git__strndup(name_start, scan - name_start);
+		assign->name = git_pool_strndup(pool, name_start, scan - name_start);
 		GITERR_CHECK_ALLOC(assign->name);
 
 		/* if there is an equals sign, find the value */
@@ -491,9 +508,8 @@ int git_attr_assignment__parse(
 
 			/* if we found a value, allocate permanent storage for it */
 			if (scan > value_start) {
-				assign->value = git__strndup(value_start, scan - value_start);
+				assign->value = git_pool_strndup(pool, value_start, scan - value_start);
 				GITERR_CHECK_ALLOC(assign->value);
-				assign->is_allocated = 1;
 			}
 		}
 
@@ -548,7 +564,7 @@ static void git_attr_rule__clear(git_attr_rule *rule)
 		git_vector_free(&rule->assigns);
 	}
 
-	git__free(rule->match.pattern);
+	/* match.pattern is stored in a git_pool, so no need to free */
 	rule->match.pattern = NULL;
 	rule->match.length = 0;
 }

src/attr_file.h

@@ -10,6 +10,7 @@
 #include "git2/attr.h"
 #include "vector.h"
 #include "hashtable.h"
+#include "pool.h"
 
 #define GIT_ATTR_FILE			".gitattributes"
 #define GIT_ATTR_FILE_INREPO	"info/attributes"
@@ -36,20 +37,21 @@ typedef struct {
 typedef struct {
 	git_refcount unused;
 	const char *name;
-    unsigned long name_hash;
+    uint32_t name_hash;
 } git_attr_name;
 
 typedef struct {
 	git_refcount rc;		/* for macros */
 	char *name;
-    unsigned long name_hash;
+    uint32_t name_hash;
     const char *value;
-	int is_allocated;
 } git_attr_assignment;
 
 typedef struct {
 	char *path;				/* cache the path this was loaded from */
 	git_vector rules;		/* vector of <rule*> or <fnmatch*> */
+	git_pool *pool;
+	bool pool_is_allocated;
 } git_attr_file;
 
 typedef struct {
@@ -62,7 +64,7 @@ typedef struct {
  * git_attr_file API
  */
 
-extern int git_attr_file__new(git_attr_file **attrs_ptr);
+extern int git_attr_file__new(git_attr_file **attrs_ptr, git_pool *pool);
 extern void git_attr_file__free(git_attr_file *file);
 
 extern int git_attr_file__from_buffer(
@@ -84,7 +86,7 @@ extern int git_attr_file__lookup_one(
 	git_vector_rforeach(&(file)->rules, (iter), (rule)) \
 		if (git_attr_rule__match((rule), (path)))
 
-extern unsigned long git_attr_file__name_hash(const char *name);
+extern uint32_t git_attr_file__name_hash(const char *name);
 
 
 /*
@@ -93,6 +95,7 @@ extern unsigned long git_attr_file__name_hash(const char *name);
 
 extern int git_attr_fnmatch__parse(
 	git_attr_fnmatch *spec,
+	git_pool *pool,
 	const char *source,
 	const char **base);
 
@@ -114,6 +117,7 @@ extern int git_attr_path__init(
 
 extern int git_attr_assignment__parse(
 	git_repository *repo, /* needed to expand macros */
+	git_pool *pool,
 	git_vector *assigns,
 	const char **scan);
 

src/diff.c

@@ -54,24 +54,6 @@ static bool diff_path_matches_pathspec(git_diff_list *diff, const char *path)
 	return false;
 }
 
-static void diff_delta__free(git_diff_delta *delta)
-{
-	if (!delta)
-		return;
-
-	if (delta->new.flags & GIT_DIFF_FILE_FREE_PATH) {
-		git__free((char *)delta->new.path);
-		delta->new.path = NULL;
-	}
-
-	if (delta->old.flags & GIT_DIFF_FILE_FREE_PATH) {
-		git__free((char *)delta->old.path);
-		delta->old.path = NULL;
-	}
-
-	git__free(delta);
-}
-
 static git_diff_delta *diff_delta__alloc(
 	git_diff_list *diff,
 	git_delta_t status,
@@ -81,12 +63,11 @@ static git_diff_delta *diff_delta__alloc(
 	if (!delta)
 		return NULL;
 
-	delta->old.path = git__strdup(path);
+	delta->old.path = git_pool_strdup(&diff->pool, path);
 	if (delta->old.path == NULL) {
 		git__free(delta);
 		return NULL;
 	}
-	delta->old.flags |= GIT_DIFF_FILE_FREE_PATH;
 	delta->new.path = delta->old.path;
 
 	if (diff->opts.flags & GIT_DIFF_REVERSE) {
@@ -101,7 +82,8 @@ static git_diff_delta *diff_delta__alloc(
 	return delta;
 }
 
-static git_diff_delta *diff_delta__dup(const git_diff_delta *d)
+static git_diff_delta *diff_delta__dup(
+	const git_diff_delta *d, git_pool *pool)
 {
 	git_diff_delta *delta = git__malloc(sizeof(git_diff_delta));
 	if (!delta)
@@ -109,33 +91,29 @@ static git_diff_delta *diff_delta__dup(const git_diff_delta *d)
 
 	memcpy(delta, d, sizeof(git_diff_delta));
 
-	delta->old.path = git__strdup(d->old.path);
-	if (delta->old.path == NULL) {
-		git__free(delta);
-		return NULL;
-	}
-	delta->old.flags |= GIT_DIFF_FILE_FREE_PATH;
+	delta->old.path = git_pool_strdup(pool, d->old.path);
+	if (delta->old.path == NULL)
+		goto fail;
 
 	if (d->new.path != d->old.path) {
-		delta->new.path = git__strdup(d->new.path);
-		if (delta->new.path == NULL) {
-			git__free(delta->old.path);
-			git__free(delta);
-			return NULL;
-		}
-		delta->new.flags |= GIT_DIFF_FILE_FREE_PATH;
+		delta->new.path = git_pool_strdup(pool, d->new.path);
+		if (delta->new.path == NULL)
+			goto fail;
 	} else {
 		delta->new.path = delta->old.path;
-		delta->new.flags &= ~GIT_DIFF_FILE_FREE_PATH;
 	}
 
 	return delta;
+
+fail:
+	git__free(delta);
+	return NULL;
 }
 
 static git_diff_delta *diff_delta__merge_like_cgit(
-	const git_diff_delta *a, const git_diff_delta *b)
+	const git_diff_delta *a, const git_diff_delta *b, git_pool *pool)
 {
-	git_diff_delta *dup = diff_delta__dup(a);
+	git_diff_delta *dup = diff_delta__dup(a, pool);
 	if (!dup)
 		return NULL;
 
@@ -146,9 +124,7 @@ static git_diff_delta *diff_delta__merge_like_cgit(
 
 	dup->new.mode = b->new.mode;
 	dup->new.size = b->new.size;
-	dup->new.flags =
-		(dup->new.flags & GIT_DIFF_FILE_FREE_PATH) |
-		(b->new.flags & ~GIT_DIFF_FILE_FREE_PATH);
+	dup->new.flags = b->new.flags;
 
 	/* Emulate C git for merging two diffs (a la 'git diff <sha>').
 	 *
@@ -210,7 +186,7 @@ static int diff_delta__from_one(
 	delta->new.flags |= GIT_DIFF_FILE_VALID_OID;
 
 	if (git_vector_insert(&diff->deltas, delta) < 0) {
-		diff_delta__free(delta);
+		git__free(delta);
 		return -1;
 	}
 
@@ -249,7 +225,7 @@ static int diff_delta__from_two(
 		delta->new.flags |= GIT_DIFF_FILE_VALID_OID;
 
 	if (git_vector_insert(&diff->deltas, delta) < 0) {
-		diff_delta__free(delta);
+		git__free(delta);
 		return -1;
 	}
 
@@ -259,19 +235,15 @@ static int diff_delta__from_two(
 #define DIFF_SRC_PREFIX_DEFAULT "a/"
 #define DIFF_DST_PREFIX_DEFAULT "b/"
 
-static char *diff_strdup_prefix(const char *prefix)
+static char *diff_strdup_prefix(git_pool *pool, const char *prefix)
 {
 	size_t len = strlen(prefix);
-	char *str = git__malloc(len + 2);
-	if (str != NULL) {
-		memcpy(str, prefix, len + 1);
+
 	/* append '/' at end if needed */
-		if (len > 0 && str[len - 1] != '/') {
-			str[len] = '/';
-			str[len + 1] = '\0';
-		}
-	}
-	return str;
+	if (len > 0 && prefix[len - 1] != '/')
+		return git_pool_strcat(pool, prefix, "/");
+	else
+		return git_pool_strndup(pool, prefix, len + 1);
 }
 
 static int diff_delta__cmp(const void *a, const void *b)
@@ -300,6 +272,10 @@ static git_diff_list *git_diff_list_alloc(
 
 	diff->repo = repo;
 
+	if (git_vector_init(&diff->deltas, 0, diff_delta__cmp) < 0 ||
+		git_pool_init(&diff->pool, 1, 0) < 0)
+		goto fail;
+
 	/* load config values that affect diff behavior */
 	if (git_repository_config__weakptr(&cfg, repo) < 0)
 		goto fail;
@@ -319,9 +295,9 @@ static git_diff_list *git_diff_list_alloc(
 	memcpy(&diff->opts, opts, sizeof(git_diff_options));
 	memset(&diff->opts.pathspec, 0, sizeof(diff->opts.pathspec));
 
-	diff->opts.src_prefix = diff_strdup_prefix(
+	diff->opts.src_prefix = diff_strdup_prefix(&diff->pool,
 		opts->src_prefix ? opts->src_prefix : DIFF_SRC_PREFIX_DEFAULT);
-	diff->opts.dst_prefix = diff_strdup_prefix(
+	diff->opts.dst_prefix = diff_strdup_prefix(&diff->pool,
 		opts->dst_prefix ? opts->dst_prefix : DIFF_DST_PREFIX_DEFAULT);
 
 	if (!diff->opts.src_prefix || !diff->opts.dst_prefix)
@@ -333,9 +309,6 @@ static git_diff_list *git_diff_list_alloc(
 		diff->opts.dst_prefix = swap;
 	}
 
-	if (git_vector_init(&diff->deltas, 0, diff_delta__cmp) < 0)
-		goto fail;
-
 	/* only copy pathspec if it is "interesting" so we can test
 	 * diff->pathspec.length > 0 to know if it is worth calling
 	 * fnmatch as we iterate.
@@ -349,11 +322,10 @@ static git_diff_list *git_diff_list_alloc(
 	for (i = 0; i < opts->pathspec.count; ++i) {
 		int ret;
 		const char *pattern = opts->pathspec.strings[i];
-		git_attr_fnmatch *match =
-			git__calloc(1, sizeof(git_attr_fnmatch));
+		git_attr_fnmatch *match = git__calloc(1, sizeof(git_attr_fnmatch));
 		if (!match)
 			goto fail;
-		ret = git_attr_fnmatch__parse(match, NULL, &pattern);
+		ret = git_attr_fnmatch__parse(match, &diff->pool, NULL, &pattern);
 		if (ret == GIT_ENOTFOUND) {
 			git__free(match);
 			continue;
@@ -381,23 +353,18 @@ void git_diff_list_free(git_diff_list *diff)
 		return;
 
 	git_vector_foreach(&diff->deltas, i, delta) {
-		diff_delta__free(delta);
+		git__free(delta);
 		diff->deltas.contents[i] = NULL;
 	}
 	git_vector_free(&diff->deltas);
 
 	git_vector_foreach(&diff->pathspec, i, match) {
-		if (match != NULL) {
-			git__free(match->pattern);
-			match->pattern = NULL;
 		git__free(match);
 		diff->pathspec.contents[i] = NULL;
 	}
-	}
 	git_vector_free(&diff->pathspec);
 
-	git__free(diff->opts.src_prefix);
-	git__free(diff->opts.dst_prefix);
+	git_pool_clear(&diff->pool);
 	git__free(diff);
 }
 
@@ -709,6 +676,7 @@ int git_diff_merge(
 	const git_diff_list *from)
 {
 	int error = 0;
+	git_pool onto_pool;
 	git_vector onto_new;
 	git_diff_delta *delta;
 	unsigned int i, j;
@@ -718,7 +686,8 @@ int git_diff_merge(
 	if (!from->deltas.length)
 		return 0;
 
-	if (git_vector_init(&onto_new, onto->deltas.length, diff_delta__cmp) < 0)
+	if (git_vector_init(&onto_new, onto->deltas.length, diff_delta__cmp) < 0 ||
+		git_pool_init(&onto_pool, 1, 0) < 0)
 		return -1;
 
 	for (i = 0, j = 0; i < onto->deltas.length || j < from->deltas.length; ) {
@@ -727,13 +696,13 @@ int git_diff_merge(
 		int cmp = !f ? -1 : !o ? 1 : strcmp(o->old.path, f->old.path);
 
 		if (cmp < 0) {
-			delta = diff_delta__dup(o);
+			delta = diff_delta__dup(o, &onto_pool);
 			i++;
 		} else if (cmp > 0) {
-			delta = diff_delta__dup(f);
+			delta = diff_delta__dup(f, &onto_pool);
 			j++;
 		} else {
-			delta = diff_delta__merge_like_cgit(o, f);
+			delta = diff_delta__merge_like_cgit(o, f, &onto_pool);
 			i++;
 			j++;
 		}
@@ -744,12 +713,14 @@ int git_diff_merge(
 
 	if (!error) {
 		git_vector_swap(&onto->deltas, &onto_new);
+		git_pool_swap(&onto->pool, &onto_pool);
 		onto->new_src = from->new_src;
 	}
 
 	git_vector_foreach(&onto_new, i, delta)
-		diff_delta__free(delta);
+		git__free(delta);
 	git_vector_free(&onto_new);
+	git_pool_clear(&onto_pool);
 
 	return error;
 }

src/diff.h

@@ -12,6 +12,7 @@
 #include "buffer.h"
 #include "iterator.h"
 #include "repository.h"
+#include "pool.h"
 
 enum {
 	GIT_DIFFCAPS_HAS_SYMLINKS     = (1 << 0), /* symlinks on platform? */
@@ -26,6 +27,7 @@ struct git_diff_list {
 	git_diff_options opts;
 	git_vector       pathspec;
 	git_vector       deltas;    /* vector of git_diff_file_delta */
+	git_pool pool;
 	git_iterator_type_t old_src;
 	git_iterator_type_t new_src;
 	uint32_t diffcaps;

src/ignore.c

@@ -35,7 +35,9 @@ static int load_ignore_file(
 			GITERR_CHECK_ALLOC(match);
 		}
 
-		if (!(error = git_attr_fnmatch__parse(match, context, &scan))) {
+		if (!(error = git_attr_fnmatch__parse(
+			match, ignores->pool, context, &scan)))
+		{
 			match->flags = match->flags | GIT_ATTR_FNMATCH_IGNORE;
 			scan = git__next_line(scan);
 			error = git_vector_insert(&ignores->rules, match);

src/pool.c

@@ -13,7 +13,7 @@ struct git_pool_page {
 #define GIT_POOL_MIN_USABLE	4
 #define GIT_POOL_MIN_PAGESZ	2 * sizeof(void*)
 
-static int pool_alloc_page(git_pool *pool, uint32_t size, void **ptr);
+static void *pool_alloc_page(git_pool *pool, uint32_t size);
 static void pool_insert_page(git_pool *pool, git_pool_page *page);
 
 int git_pool_init(
@@ -62,11 +62,25 @@ void git_pool_clear(git_pool *pool)
 
 	pool->free_list = NULL;
 
+	pool->items = 0;
+
 	pool->has_string_alloc     = 0;
 	pool->has_multi_item_alloc = 0;
 	pool->has_large_page_alloc = 0;
 }
 
+void git_pool_swap(git_pool *a, git_pool *b)
+{
+	git_pool temp;
+
+	if (a == b)
+		return;
+
+	memcpy(&temp, a, sizeof(temp));
+	memcpy(a, b, sizeof(temp));
+	memcpy(b, &temp, sizeof(temp));
+}
+
 static void pool_insert_page(git_pool *pool, git_pool_page *page)
 {
 	git_pool_page *scan;
@@ -88,8 +102,7 @@ static void pool_insert_page(git_pool *pool, git_pool_page *page)
 	scan->next = page;
 }
 
-static int pool_alloc_page(
-	git_pool *pool, uint32_t size, void **ptr)
+static void *pool_alloc_page(git_pool *pool, uint32_t size)
 {
 	git_pool_page *page;
 	uint32_t alloc_size;
@@ -103,7 +116,7 @@ static int pool_alloc_page(
 
 	page = git__calloc(1, alloc_size + sizeof(git_pool_page));
 	if (!page)
-		return -1;
+		return NULL;
 
 	page->size  = alloc_size;
 	page->avail = alloc_size - size;
@@ -115,9 +128,9 @@ static int pool_alloc_page(
 		pool->full = page;
 	}
 
-	*ptr = page->data;
+	pool->items++;
 
-	return 0;
+	return page->data;
 }
 
 GIT_INLINE(void) pool_remove_page(
@@ -129,22 +142,26 @@ GIT_INLINE(void) pool_remove_page(
 		prev->next = page->next;
 }
 
-int git_pool_malloc(git_pool *pool, uint32_t items, void **ptr)
+void *git_pool_malloc(git_pool *pool, uint32_t items)
 {
 	git_pool_page *scan = pool->open, *prev;
 	uint32_t size = items * pool->item_size;
+	void *ptr = NULL;
 
 	pool->has_string_alloc = 0;
 	if (items > 1)
 		pool->has_multi_item_alloc = 1;
 	else if (pool->free_list != NULL) {
-		*ptr = pool->free_list;
+		ptr = pool->free_list;
 		pool->free_list = *((void **)pool->free_list);
+		return ptr;
 	}
 
 	/* just add a block if there is no open one to accomodate this */
 	if (size >= pool->page_size || !scan || scan->avail < size)
-		return pool_alloc_page(pool, size, ptr);
+		return pool_alloc_page(pool, size);
+
+	pool->items++;
 
 	/* find smallest block in free list with space */
 	for (scan = pool->open, prev = NULL;
@@ -152,7 +169,7 @@ int git_pool_malloc(git_pool *pool, uint32_t items, void **ptr)
 		 prev = scan, scan = scan->next);
 
 	/* allocate space from the block */
-	*ptr = &scan->data[scan->size - scan->avail];
+	ptr = &scan->data[scan->size - scan->avail];
 	scan->avail -= size;
 
 	/* move to full list if there is almost no space left */
@@ -167,7 +184,7 @@ int git_pool_malloc(git_pool *pool, uint32_t items, void **ptr)
 		pool_insert_page(pool, scan);
 	}
 
-	return 0;
+	return ptr;
 }
 
 char *git_pool_strndup(git_pool *pool, const char *str, size_t n)
@@ -176,8 +193,10 @@ char *git_pool_strndup(git_pool *pool, const char *str, size_t n)
 
 	assert(pool && str && pool->item_size == sizeof(char));
 
-	if (!git_pool_malloc(pool, n, &ptr))
+	if ((ptr = git_pool_malloc(pool, n + 1)) != NULL) {
 		memcpy(ptr, str, n);
+		*(((char *)ptr) + n) = '\0';
+	}
 	pool->has_string_alloc = 1;
 
 	return ptr;
@@ -187,7 +206,29 @@ char *git_pool_strdup(git_pool *pool, const char *str)
 {
 	assert(pool && str && pool->item_size == sizeof(char));
 
-	return git_pool_strndup(pool, str, strlen(str) + 1);
+	return git_pool_strndup(pool, str, strlen(str));
+}
+
+char *git_pool_strcat(git_pool *pool, const char *a, const char *b)
+{
+	void *ptr;
+	size_t len_a, len_b;
+
+	assert(pool && a && b && pool->item_size == sizeof(char));
+
+	len_a = a ? strlen(a) : 0;
+	len_b = b ? strlen(b) : 0;
+
+	if ((ptr = git_pool_malloc(pool, len_a + len_b + 1)) != NULL) {
+		if (len_a)
+			memcpy(ptr, a, len_a);
+		if (len_b)
+			memcpy(((char *)ptr) + len_a, b, len_b);
+		*(((char *)ptr) + len_a + len_b) = '\0';
+	}
+	pool->has_string_alloc = 1;
+
+	return ptr;
 }
 
 void git_pool_free(git_pool *pool, void *ptr)

src/pool.h

@@ -33,11 +33,14 @@ typedef struct {
 	void *free_list;     /* optional: list of freed blocks */
 	uint32_t item_size;  /* size of single alloc unit in bytes */
 	uint32_t page_size;  /* size of page in bytes */
+	uint32_t items;
 	unsigned has_string_alloc : 1; /* was the strdup function used */
 	unsigned has_multi_item_alloc : 1; /* was items ever > 1 in malloc */
 	unsigned has_large_page_alloc : 1; /* are any pages > page_size */
 } git_pool;
 
+#define GIT_POOL_INIT_STRINGPOOL { 0, 0, 0, 1, 4000, 0, 0, 0, 0 }
+
 /**
  * Initialize a pool.
  *
@@ -49,7 +52,7 @@ typedef struct {
  * To allocate items of fixed size, use like this:
  *
  *     git_pool_init(&pool, sizeof(item), 0);
- *     git_pool_malloc(&pool, 1, &my_item_ptr);
+ *     my_item = git_pool_malloc(&pool, 1);
  *
  * Of course, you can use this in other ways, but those are the
  * two most common patterns.
@@ -63,9 +66,14 @@ extern int git_pool_init(
 extern void git_pool_clear(git_pool *pool);
 
 /**
+ * Swap two pools with one another
+ */
+extern void git_pool_swap(git_pool *a, git_pool *b);
+
+/**
  * Allocate space for one or more items from a pool.
  */
-extern int git_pool_malloc(git_pool *pool, uint32_t items, void **ptr);
+extern void *git_pool_malloc(git_pool *pool, uint32_t items);
 
 /**
  * Allocate space and duplicate string data into it.
@@ -82,6 +90,13 @@ extern char *git_pool_strndup(git_pool *pool, const char *str, size_t n);
 extern char *git_pool_strdup(git_pool *pool, const char *str);
 
 /**
+ * Allocate space for the concatenation of two strings.
+ *
+ * This is allowed only for pools with item_size == sizeof(char)
+ */
+extern char *git_pool_strcat(git_pool *pool, const char *a, const char *b);
+
+/**
  * Push a block back onto the free list for the pool.
  *
  * This is allowed only if the item_size is >= sizeof(void*).

tests-clar/attr/file.c

@@ -11,7 +11,7 @@ void test_attr_file__simple_read(void)
 	git_attr_assignment *assign;
 	git_attr_rule *rule;
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr0"), file));
 	cl_assert_equal_s(cl_fixture("attr/attr0"), file->path);
 	cl_assert(file->rules.length == 1);
@@ -27,7 +27,6 @@ void test_attr_file__simple_read(void)
 	cl_assert(assign != NULL);
 	cl_assert_equal_s("binary", assign->name);
 	cl_assert(GIT_ATTR_TRUE(assign->value));
-	cl_assert(!assign->is_allocated);
 
 	git_attr_file__free(file);
 }
@@ -38,7 +37,7 @@ void test_attr_file__match_variants(void)
 	git_attr_rule *rule;
 	git_attr_assignment *assign;
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr1"), file));
 	cl_assert_equal_s(cl_fixture("attr/attr1"), file->path);
 	cl_assert(file->rules.length == 10);
@@ -56,7 +55,6 @@ void test_attr_file__match_variants(void)
 	cl_assert_equal_s("attr0", assign->name);
 	cl_assert(assign->name_hash == git_attr_file__name_hash(assign->name));
 	cl_assert(GIT_ATTR_TRUE(assign->value));
-	cl_assert(!assign->is_allocated);
 
 	rule = get_rule(1);
 	cl_assert_equal_s("pat1", rule->match.pattern);
@@ -125,7 +123,7 @@ void test_attr_file__assign_variants(void)
 	git_attr_rule *rule;
 	git_attr_assignment *assign;
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr2"), file));
 	cl_assert_equal_s(cl_fixture("attr/attr2"), file->path);
 	cl_assert(file->rules.length == 11);
@@ -191,7 +189,7 @@ void test_attr_file__check_attr_examples(void)
 	git_attr_rule *rule;
 	git_attr_assignment *assign;
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr3"), file));
 	cl_assert_equal_s(cl_fixture("attr/attr3"), file->path);
 	cl_assert(file->rules.length == 3);

tests-clar/attr/lookup.c

@@ -9,7 +9,7 @@ void test_attr_lookup__simple(void)
 	git_attr_path path;
 	const char *value = NULL;
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr0"), file));
 	cl_assert_equal_s(cl_fixture("attr/attr0"), file->path);
 	cl_assert(file->rules.length == 1);
@@ -127,7 +127,7 @@ void test_attr_lookup__match_variants(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr1"), file));
 	cl_assert_equal_s(cl_fixture("attr/attr1"), file->path);
 	cl_assert(file->rules.length == 10);
@@ -144,6 +144,7 @@ void test_attr_lookup__match_variants(void)
 void test_attr_lookup__assign_variants(void)
 {
 	git_attr_file *file;
+
 	struct attr_expected cases[] = {
 		/* pat0 -> simple assign */
 		{ "pat0", "simple", EXPECT_TRUE, NULL },
@@ -187,7 +188,7 @@ void test_attr_lookup__assign_variants(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr2"), file));
 	cl_assert(file->rules.length == 11);
 
@@ -199,6 +200,7 @@ void test_attr_lookup__assign_variants(void)
 void test_attr_lookup__check_attr_examples(void)
 {
 	git_attr_file *file;
+
 	struct attr_expected cases[] = {
 		{ "foo.java", "diff", EXPECT_STRING, "java" },
 		{ "foo.java", "crlf", EXPECT_FALSE, NULL },
@@ -222,7 +224,7 @@ void test_attr_lookup__check_attr_examples(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_file(NULL, cl_fixture("attr/attr3"), file));
 	cl_assert(file->rules.length == 3);
 
@@ -234,6 +236,7 @@ void test_attr_lookup__check_attr_examples(void)
 void test_attr_lookup__from_buffer(void)
 {
 	git_attr_file *file;
+
 	struct attr_expected cases[] = {
 		{ "abc", "foo", EXPECT_TRUE, NULL },
 		{ "abc", "bar", EXPECT_TRUE, NULL },
@@ -247,7 +250,7 @@ void test_attr_lookup__from_buffer(void)
 		{ NULL, NULL, 0, NULL }
 	};
 
-	cl_git_pass(git_attr_file__new(&file));
+	cl_git_pass(git_attr_file__new(&file, NULL));
 	cl_git_pass(git_attr_file__from_buffer(NULL, "a* foo\nabc bar\n* baz", file));
 	cl_assert(file->rules.length == 3);
 

tests-clar/core/pool.c

@@ -11,9 +11,8 @@ void test_core_pool__0(void)
 	cl_git_pass(git_pool_init(&p, 1, 4000));
 
 	for (i = 1; i < 10000; i *= 2) {
-		cl_git_pass(git_pool_malloc(&p, i, &ptr));
+		ptr = git_pool_malloc(&p, i);
 		cl_assert(ptr != NULL);
-
 		cl_assert(git_pool__ptr_in_pool(&p, ptr));
 		cl_assert(!git_pool__ptr_in_pool(&p, &i));
 	}
@@ -32,12 +31,11 @@ void test_core_pool__1(void)
 {
 	int i;
 	git_pool p;
-	void *ptr;
 
 	cl_git_pass(git_pool_init(&p, 1, 4000));
 
 	for (i = 2010; i > 0; i--)
-		cl_git_pass(git_pool_malloc(&p, i, &ptr));
+		cl_assert(git_pool_malloc(&p, i) != NULL);
 
 	/* with fixed page size, allocation must end up with these values */
 	cl_assert(git_pool__open_pages(&p) == 1);
@@ -48,7 +46,7 @@ void test_core_pool__1(void)
 	cl_git_pass(git_pool_init(&p, 1, 4100));
 
 	for (i = 2010; i > 0; i--)
-		cl_git_pass(git_pool_malloc(&p, i, &ptr));
+		cl_assert(git_pool_malloc(&p, i) != NULL);
 
 	/* with fixed page size, allocation must end up with these values */
 	cl_assert(git_pool__open_pages(&p) == 1);
@@ -71,7 +69,9 @@ void test_core_pool__2(void)
 	cl_git_pass(git_pool_init(&p, sizeof(git_oid), 100));
 
 	for (i = 1000; i < 10000; i++) {
-		cl_git_pass(git_pool_malloc(&p, 1, (void **)&oid));
+		oid = git_pool_malloc(&p, 1);
+		cl_assert(oid != NULL);
+
 		for (j = 0; j < 8; j++)
 			oid_hex[j] = to_hex[(i >> (4 * j)) & 0x0f];
 		cl_git_pass(git_oid_fromstr(oid, oid_hex));