Add callback to git_objects_table

This adds create and free callback to the git_objects_table so
that more of the creation and destruction of objects can be table
driven instead of using switch statements.  This also makes the
semantics of certain object creation functions consistent so that
we can make better use of function pointers.  This also fixes a
theoretical error case where an object allocation fails and we
end up storing NULL into the cache.

15 files changed, 128 insertions, 184 deletions

diff --git a/src/blob.c b/src/blob.c
index 732b0f3de..501c13d1a 100644
--- a/src/blob.c
+++ b/src/blob.c
@@ -35,17 +35,17 @@ int git_blob__getbuf(git_buf *buffer, git_blob *blob)
 		git_odb_object_size(blob->odb_object));
 }
 
-void git_blob__free(git_blob *blob)
+void git_blob__free(void *blob)
 {
-	git_odb_object_free(blob->odb_object);
+	git_odb_object_free(((git_blob *)blob)->odb_object);
 	git__free(blob);
 }
 
-int git_blob__parse(git_blob *blob, git_odb_object *odb_obj)
+int git_blob__from_odb_object(void *blob, git_odb_object *odb_obj)
 {
 	assert(blob);
 	git_cached_obj_incref((git_cached_obj *)odb_obj);
-	blob->odb_object = odb_obj;
+	((git_blob *)blob)->odb_object = odb_obj;
 	return 0;
 }
 
diff --git a/src/blob.h b/src/blob.h
index 524734b1f..4873505fd 100644
--- a/src/blob.h
+++ b/src/blob.h
@@ -17,8 +17,8 @@ struct git_blob {
 	git_odb_object *odb_object;
 };
 
-void git_blob__free(git_blob *blob);
-int git_blob__parse(git_blob *blob, git_odb_object *obj);
+void git_blob__free(void *blob);
+int git_blob__from_odb_object(void *blob, git_odb_object *obj);
 int git_blob__getbuf(git_buf *buffer, git_blob *blob);
 
 #endif
diff --git a/src/cache.c b/src/cache.c
index 263f736fa..c51be895e 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -70,12 +70,6 @@ int git_cache_init(git_cache *cache)
 	return 0;
 }
 
-void git_cache_free(git_cache *cache)
-{
-	git_oidmap_free(cache->map);
-	git_mutex_free(&cache->lock);
-}
-
 void git_cache_clear(git_cache *cache)
 {
 	git_cached_obj *evict = NULL;
@@ -93,6 +87,14 @@ void git_cache_clear(git_cache *cache)
 	git_mutex_unlock(&cache->lock);
 }
 
+void git_cache_free(git_cache *cache)
+{
+	git_cache_clear(cache);
+
+	git_oidmap_free(cache->map);
+	git_mutex_free(&cache->lock);
+}
+
 /* Call with lock, yo */
 static void cache_evict_entries(git_cache *cache, size_t evict_count)
 {
diff --git a/src/cache.h b/src/cache.h
index 13b630e89..e95d521fe 100644
--- a/src/cache.h
+++ b/src/cache.h
@@ -21,17 +21,17 @@ enum {
 };
 
 typedef struct {
-	git_oid oid;
-	int16_t type;
-	uint16_t flags;
-	size_t size;
+	git_oid    oid;
+	int16_t    type;  /* git_otype value */
+	uint16_t   flags; /* GIT_CACHE_STORE value */
+	size_t     size;
 	git_atomic refcount;
 } git_cached_obj;
 
 typedef struct {
 	git_oidmap *map;
-	git_mutex lock;
-	size_t used_memory;
+	git_mutex   lock;
+	size_t      used_memory;
 } git_cache;
 
 extern bool git_cache__enabled;
diff --git a/src/commit.c b/src/commit.c
index 2cee44cd2..3eca5b341 100644
--- a/src/commit.c
+++ b/src/commit.c
@@ -31,8 +31,10 @@ static void clear_parents(git_commit *commit)
 	git_vector_clear(&commit->parent_ids);
 }
 
-void git_commit__free(git_commit *commit)
+void git_commit__free(void *_commit)
 {
+	git_commit *commit = _commit;
+
 	clear_parents(commit);
 	git_vector_free(&commit->parent_ids);
 
@@ -166,10 +168,9 @@ int git_commit_create(
 	return retval;
 }
 
-int git_commit__parse_buffer(git_commit *commit, const void *data, size_t len)
+int git_commit__parse(void *_commit, const char *buffer, const char *buffer_end)
 {
-	const char *buffer = data;
-	const char *buffer_end = (const char *)data + len;
+	git_commit *commit = _commit;
 	git_oid parent_id;
 
 	if (git_vector_init(&commit->parent_ids, 4, NULL) < 0)
@@ -241,13 +242,6 @@ bad_buffer:
 	return -1;
 }
 
-int git_commit__parse(git_commit *commit, git_odb_object *obj)
-{
-	assert(commit);
-	return git_commit__parse_buffer(
-		commit, git_odb_object_data(obj), git_odb_object_size(obj));
-}
-
 #define GIT_COMMIT_GETTER(_rvalue, _name, _return) \
 	_rvalue git_commit_##_name(const git_commit *commit) \
 	{\
diff --git a/src/commit.h b/src/commit.h
index 1ab164c0b..0c2c3ab5d 100644
--- a/src/commit.h
+++ b/src/commit.h
@@ -27,8 +27,7 @@ struct git_commit {
 	char *message;
 };
 
-void git_commit__free(git_commit *c);
-int git_commit__parse(git_commit *commit, git_odb_object *obj);
+void git_commit__free(void *commit);
+int git_commit__parse(void *commit, const char *buf, const char *bufend);
 
-int git_commit__parse_buffer(git_commit *commit, const void *data, size_t len);
 #endif
diff --git a/src/object.c b/src/object.c
index 80b765ef9..3698808c3 100644
--- a/src/object.c
+++ b/src/object.c
@@ -18,63 +18,39 @@
 
 static const int OBJECT_BASE_SIZE = 4096;
 
-static struct {
+typedef struct {
 	const char	*str;	/* type name string */
-	int			loose; /* valid loose object type flag */
 	size_t		size;	/* size in bytes of the object structure */
-} git_objects_table[] = {
+
+	int  (*from_odb)(void *self, git_odb_object *obj);
+	int  (*parse)(void *self, const char *buf, const char *buf_end);
+	void (*free)(void *self);
+} git_object_def;
+
+static git_object_def git_objects_table[] = {
 	/* 0 = GIT_OBJ__EXT1 */
-	{ "", 0, 0},
+	{ "", 0, NULL, NULL, NULL },
 
 	/* 1 = GIT_OBJ_COMMIT */
-	{ "commit", 1, sizeof(struct git_commit)},
+	{ "commit", sizeof(git_commit), NULL, git_commit__parse, git_commit__free },
 
 	/* 2 = GIT_OBJ_TREE */
-	{ "tree", 1, sizeof(struct git_tree) },
+	{ "tree", sizeof(git_tree), NULL, git_tree__parse, git_tree__free },
 
 	/* 3 = GIT_OBJ_BLOB */
-	{ "blob", 1, sizeof(struct git_blob) },
+	{ "blob", sizeof(git_blob), git_blob__from_odb_object, NULL, git_blob__free },
 
 	/* 4 = GIT_OBJ_TAG */
-	{ "tag", 1, sizeof(struct git_tag) },
+	{ "tag", sizeof(git_tag), NULL, git_tag__parse, git_tag__free },
 
 	/* 5 = GIT_OBJ__EXT2 */
-	{ "", 0, 0 },
-
+	{ "", 0, NULL, NULL, NULL },
 	/* 6 = GIT_OBJ_OFS_DELTA */
-	{ "OFS_DELTA", 0, 0 },
-
+	{ "OFS_DELTA", 0, NULL, NULL, NULL },
 	/* 7 = GIT_OBJ_REF_DELTA */
-	{ "REF_DELTA", 0, 0	}
+	{ "REF_DELTA", 0, NULL, NULL, NULL },
 };
 
-static int create_object(git_object **object_out, git_otype type)
-{
-	git_object *object = NULL;
-
-	assert(object_out);
-
-	*object_out = NULL;
-
-	switch (type) {
-	case GIT_OBJ_COMMIT:
-	case GIT_OBJ_TAG:
-	case GIT_OBJ_BLOB:
-	case GIT_OBJ_TREE:
-		object = git__malloc(git_object__size(type));
-		GITERR_CHECK_ALLOC(object);
-		memset(object, 0x0, git_object__size(type));
-		break;
-
-	default:
-		giterr_set(GITERR_INVALID, "The given type is invalid");
-		return -1;
-	}
-
-	*object_out = object;
-	return 0;
-}
-
 int git_object__from_odb_object(
 	git_object **object_out,
 	git_repository *repo,
@@ -82,46 +58,47 @@ int git_object__from_odb_object(
 	git_otype type)
 {
 	int error;
+	size_t object_size;
+	git_object_def *def;
 	git_object *object = NULL;
 
+	assert(object_out);
+	*object_out = NULL;
+
+	/* Validate type match */
 	if (type != GIT_OBJ_ANY && type != odb_obj->cached.type) {
 		giterr_set(GITERR_INVALID,
 			"The requested type does not match the type in the ODB");
 		return GIT_ENOTFOUND;
 	}
 
-	if ((error = create_object(&object, odb_obj->cached.type)) < 0)
-		return error;
+	if ((object_size = git_object__size(odb_obj->cached.type)) == 0) {
+		giterr_set(GITERR_INVALID, "The requested type is invalid");
+		return GIT_ENOTFOUND;
+	}
+
+	/* Allocate and initialize base object */
+	object = git__calloc(1, object_size);
+	GITERR_CHECK_ALLOC(object);
 
-	/* Initialize parent object */
 	git_oid_cpy(&object->cached.oid, &odb_obj->cached.oid);
-	object->cached.size = odb_obj->cached.size;
 	object->cached.type = odb_obj->cached.type;
+	object->cached.size = odb_obj->cached.size;
 	object->repo = repo;
 
-	switch (object->cached.type) {
-	case GIT_OBJ_COMMIT:
-		error = git_commit__parse((git_commit *)object, odb_obj);
-		break;
-
-	case GIT_OBJ_TREE:
-		error = git_tree__parse((git_tree *)object, odb_obj);
-		break;
-
-	case GIT_OBJ_TAG:
-		error = git_tag__parse((git_tag *)object, odb_obj);
-		break;
+	/* Parse raw object data */
+	def = &git_objects_table[odb_obj->cached.type];
+	assert(def->free && (def->from_odb || def->parse));
 
-	case GIT_OBJ_BLOB:
-		error = git_blob__parse((git_blob *)object, odb_obj);
-		break;
-
-	default:
-		break;
+	if (def->from_odb) {
+		error = def->from_odb(object, odb_obj);
+	} else {
+		const char *data = (const char *)git_odb_object_data(odb_obj);
+		error = def->parse(object, data, data + git_odb_object_size(odb_obj));
 	}
 
 	if (error < 0) {
-		git_object__free(object);
+		def->free(object);
 		return error;
 	}
 
@@ -129,6 +106,17 @@ int git_object__from_odb_object(
 	return 0;
 }
 
+void git_object__free(void *obj)
+{
+	git_otype type = ((git_object *)obj)->cached.type;
+
+	if (type < 0 || ((size_t)type) >= ARRAY_SIZE(git_objects_table) ||
+		!git_objects_table[type].free)
+		git__free(obj);
+	else
+		git_objects_table[type].free(obj);
+}
+
 int git_object_lookup_prefix(
 	git_object **object_out,
 	git_repository *repo,
@@ -222,35 +210,6 @@ int git_object_lookup(git_object **object_out, git_repository *repo, const git_o
 	return git_object_lookup_prefix(object_out, repo, id, GIT_OID_HEXSZ, type);
 }
 
-void git_object__free(void *_obj)
-{
-	git_object *object = (git_object *)_obj;
-
-	assert(object);
-
-	switch (object->cached.type) {
-	case GIT_OBJ_COMMIT:
-		git_commit__free((git_commit *)object);
-		break;
-
-	case GIT_OBJ_TREE:
-		git_tree__free((git_tree *)object);
-		break;
-
-	case GIT_OBJ_TAG:
-		git_tag__free((git_tag *)object);
-		break;
-
-	case GIT_OBJ_BLOB:
-		git_blob__free((git_blob *)object);
-		break;
-
-	default:
-		git__free(object);
-		break;
-	}
-}
-
 void git_object_free(git_object *object)
 {
 	if (object == NULL)
@@ -304,7 +263,7 @@ int git_object_typeisloose(git_otype type)
 	if (type < 0 || ((size_t) type) >= ARRAY_SIZE(git_objects_table))
 		return 0;
 
-	return git_objects_table[type].loose;
+	return (git_objects_table[type].size > 0) ? 1 : 0;
 }
 
 size_t git_object__size(git_otype type)
diff --git a/src/odb.c b/src/odb.c
index 16a842aa8..53630dddc 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -82,23 +82,24 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj)
 }
 
 
-static git_odb_object *new_odb_object(const git_oid *oid, git_rawobj *source)
+static git_odb_object *odb_object__alloc(const git_oid *oid, git_rawobj *source)
 {
-	git_odb_object *object = git__malloc(sizeof(git_odb_object));
-	memset(object, 0x0, sizeof(git_odb_object));
+	git_odb_object *object = git__calloc(1, sizeof(git_odb_object));
 
-	git_oid_cpy(&object->cached.oid, oid);
-	object->cached.size = source->len;
-	object->cached.type = source->type;
-	object->buffer = source->data;
+	if (object != NULL) {
+		git_oid_cpy(&object->cached.oid, oid);
+		object->cached.type = source->type;
+		object->cached.size = source->len;
+		object->buffer      = source->data;
+	}
 
 	return object;
 }
 
-void git_odb_object__free(git_odb_object *object)
+void git_odb_object__free(void *object)
 {
 	if (object != NULL) {
-		git__free(object->buffer);
+		git__free(((git_odb_object *)object)->buffer);
 		git__free(object);
 	}
 }
@@ -679,6 +680,7 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 	int error;
 	bool refreshed = false;
 	git_rawobj raw;
+	git_odb_object *object;
 
 	assert(out && db && id);
 
@@ -713,7 +715,10 @@ attempt_lookup:
 	if (error && error != GIT_PASSTHROUGH)
 		return error;
 
-	*out = git_cache_store_raw(odb_cache(db), new_odb_object(id, &raw));
+	if ((object = odb_object__alloc(id, &raw)) == NULL)
+		return -1;
+
+	*out = git_cache_store_raw(odb_cache(db), object);
 	return 0;
 }
 
@@ -726,6 +731,7 @@ int git_odb_read_prefix(
 	git_rawobj raw;
 	void *data = NULL;
 	bool found = false, refreshed = false;
+	git_odb_object *object;
 
 	assert(out && db);
 
@@ -777,7 +783,10 @@ attempt_lookup:
 	if (!found)
 		return git_odb__error_notfound("no match for prefix", short_id);
 
-	*out = git_cache_store_raw(odb_cache(db), new_odb_object(&found_full_oid, &raw));
+	if ((object = odb_object__alloc(&found_full_oid, &raw)) == NULL)
+		return -1;
+
+	*out = git_cache_store_raw(odb_cache(db), object);
 	return 0;
 }
 
diff --git a/src/odb.h b/src/odb.h
index 22c6e1668..0d9f9e2ea 100644
--- a/src/odb.h
+++ b/src/odb.h
@@ -39,8 +39,6 @@ struct git_odb {
 	git_cache own_cache;
 };
 
-void git_odb_object__free(git_odb_object *object);
-
 /*
  * Hash a git_rawobj internally.
  * The `git_rawobj` is supposed to be previously initialized
@@ -98,4 +96,7 @@ int git_odb__read_header_or_object(
 	git_odb_object **out, size_t *len_p, git_otype *type_p,
 	git_odb *db, const git_oid *id);
 
+/* fully free the object; internal method, DO NOT EXPORT */
+void git_odb_object__free(void *object);
+
 #endif
diff --git a/src/oidmap.h b/src/oidmap.h
index dfa951af3..a29c7cd35 100644
--- a/src/oidmap.h
+++ b/src/oidmap.h
@@ -19,7 +19,7 @@
 __KHASH_TYPE(oid, const git_oid *, void *);
 typedef khash_t(oid) git_oidmap;
 
-GIT_INLINE(khint_t) hash_git_oid(const git_oid *oid)
+GIT_INLINE(khint_t) git_oidmap_hash(const git_oid *oid)
 {
 	khint_t h;
 	memcpy(&h, oid, sizeof(khint_t));
@@ -27,7 +27,7 @@ GIT_INLINE(khint_t) hash_git_oid(const git_oid *oid)
 }
 
 #define GIT__USE_OIDMAP \
-	__KHASH_IMPL(oid, static kh_inline, const git_oid *, void *, 1, hash_git_oid, git_oid_equal)
+	__KHASH_IMPL(oid, static kh_inline, const git_oid *, void *, 1, git_oidmap_hash, git_oid_equal)
 
 #define git_oidmap_alloc() kh_init(oid)
 #define git_oidmap_free(h) kh_destroy(oid,h), h = NULL
diff --git a/src/tag.c b/src/tag.c
index b76895d0c..7dadc7e60 100644
--- a/src/tag.c
+++ b/src/tag.c
@@ -15,8 +15,9 @@
 #include "git2/signature.h"
 #include "git2/odb_backend.h"
 
-void git_tag__free(git_tag *tag)
+void git_tag__free(void *_tag)
 {
+	git_tag *tag = _tag;
 	git_signature_free(tag->tagger);
 	git__free(tag->message);
 	git__free(tag->tag_name);
@@ -69,18 +70,17 @@ static int tag_error(const char *str)
 	return -1;
 }
 
-int git_tag__parse_buffer(git_tag *tag, const char *buffer, size_t length)
+int git_tag__parse(void *_tag, const char *buffer, const char *buffer_end)
 {
 	static const char *tag_types[] = {
 		NULL, "commit\n", "tree\n", "blob\n", "tag\n"
 	};
 
+	git_tag *tag = _tag;
 	unsigned int i;
 	size_t text_len;
 	char *search;
 
-	const char *buffer_end = buffer + length;
-
 	if (git_oid__parse(&tag->target, &buffer, buffer_end, "object ") < 0)
 		return tag_error("Object field invalid");
 
@@ -317,7 +317,7 @@ int git_tag_create_frombuffer(git_oid *oid, git_repository *repo, const char *bu
 		return -1;
 
 	/* validate the buffer */
-	if (git_tag__parse_buffer(&tag, buffer, strlen(buffer)) < 0)
+	if (git_tag__parse(&tag, buffer, buffer + strlen(buffer)) < 0)
 		return -1;
 
 	/* validate the target */
@@ -390,15 +390,8 @@ int git_tag_delete(git_repository *repo, const char *tag_name)
 
 	if ((error = git_reference_delete(tag_ref)) == 0)
 		git_reference_free(tag_ref);
-	
-	return error;
-}
 
-int git_tag__parse(git_tag *tag, git_odb_object *obj)
-{
-	assert(tag);
-	return git_tag__parse_buffer(
-		tag, git_odb_object_data(obj), git_odb_object_size(obj));
+	return error;
 }
 
 typedef struct {
diff --git a/src/tag.h b/src/tag.h
index c8e421ee6..fb01a6f75 100644
--- a/src/tag.h
+++ b/src/tag.h
@@ -22,8 +22,7 @@ struct git_tag {
 	char *message;
 };
 
-void git_tag__free(git_tag *tag);
-int git_tag__parse(git_tag *tag, git_odb_object *obj);
-int git_tag__parse_buffer(git_tag *tag, const char *data, size_t len);
+void git_tag__free(void *tag);
+int git_tag__parse(void *tag, const char *buf, const char *buf_end);
 
 #endif
diff --git a/src/tree.c b/src/tree.c
index cc43b920c..e66fa2370 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -219,15 +219,16 @@ git_tree_entry *git_tree_entry_dup(const git_tree_entry *entry)
 	return copy;
 }
 
-void git_tree__free(git_tree *tree)
+void git_tree__free(void *tree)
 {
+	git_vector *entries = &((git_tree *)tree)->entries;
 	size_t i;
 	git_tree_entry *e;
 
-	git_vector_foreach(&tree->entries, i, e)
+	git_vector_foreach(entries, i, e)
 		git_tree_entry_free(e);
 
-	git_vector_free(&tree->entries);
+	git_vector_free(entries);
 	git__free(tree);
 }
 
@@ -371,10 +372,11 @@ static int tree_error(const char *str, const char *path)
 	return -1;
 }
 
-static int tree_parse_buffer(
-	git_tree *tree, const char *buffer, const char *buffer_end)
+int git_tree__parse(void *tree, const char *buffer, const char *buffer_end)
 {
-	if (git_vector_init(&tree->entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0)
+	git_vector *tree_entries = &((git_tree *)tree)->entries;
+
+	if (git_vector_init(tree_entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0)
 		return -1;
 
 	while (buffer < buffer_end) {
@@ -397,7 +399,7 @@ static int tree_parse_buffer(
 			entry = alloc_entry(buffer);
 			GITERR_CHECK_ALLOC(entry);
 
-			if (git_vector_insert(&tree->entries, entry) < 0) {
+			if (git_vector_insert(tree_entries, entry) < 0) {
 				git__free(entry);
 				return -1;
 			}
@@ -417,17 +419,6 @@ static int tree_parse_buffer(
 	return 0;
 }
 
-int git_tree__parse(git_tree *tree, git_odb_object *obj)
-{
-	const char *buf;
-
-	assert(tree && obj);
-
-	buf = (const char *)git_odb_object_data(obj);
-
-	return tree_parse_buffer(tree, buf, buf + git_odb_object_size(obj));
-}
-
 static size_t find_next_dir(const char *dirname, git_index *index, size_t start)
 {
 	size_t dirlen, i, entries = git_index_entrycount(index);
diff --git a/src/tree.h b/src/tree.h
index b77bfd961..cf47fb478 100644
--- a/src/tree.h
+++ b/src/tree.h
@@ -37,8 +37,8 @@ GIT_INLINE(bool) git_tree_entry__is_tree(const struct git_tree_entry *e)
 
 extern int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2);
 
-void git_tree__free(git_tree *tree);
-int git_tree__parse(git_tree *tree, git_odb_object *obj);
+void git_tree__free(void *tree);
+int git_tree__parse(void *tree, const char *buf, const char *buf_end);
 
 /**
  * Lookup the first position in the tree with a given prefix.
diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c
index b99d27991..792b57626 100644
--- a/tests-clar/commit/parse.c
+++ b/tests-clar/commit/parse.c
@@ -269,6 +269,7 @@ void test_commit_parse__entire_commit(void)
 	const int broken_commit_count = sizeof(failing_commit_cases) / sizeof(*failing_commit_cases);
 	const int working_commit_count = sizeof(passing_commit_cases) / sizeof(*passing_commit_cases);
 	int i;
+	const char *buf;
 
 	for (i = 0; i < broken_commit_count; ++i) {
 		git_commit *commit;
@@ -276,9 +277,8 @@ void test_commit_parse__entire_commit(void)
 		memset(commit, 0x0, sizeof(git_commit));
 		commit->object.repo = g_repo;
 
-		cl_git_fail(git_commit__parse_buffer(
-			commit, failing_commit_cases[i], strlen(failing_commit_cases[i]))
-		);
+		buf = failing_commit_cases[i];
+		cl_git_fail(git_commit__parse(commit, buf, buf + strlen(buf)));
 
 		git_commit__free(commit);
 	}
@@ -290,11 +290,8 @@ void test_commit_parse__entire_commit(void)
 		memset(commit, 0x0, sizeof(git_commit));
 		commit->object.repo = g_repo;
 
-		cl_git_pass(git_commit__parse_buffer(
-                     commit,
-                     passing_commit_cases[i],
-                     strlen(passing_commit_cases[i]))
-         );
+		buf = passing_commit_cases[i];
+		cl_git_pass(git_commit__parse(commit, buf, buf + strlen(buf)));
 
 		if (!i)
 			cl_assert_equal_s("", git_commit_message(commit));