Fix object handling in git_repository

All loaded objects through git_repository_lookup are properly parsed &
free'd on failure.

Signed-off-by: Vicent Marti <tanoku@gmail.com>

src/commit.c

@@ -53,9 +53,6 @@ void git_commit__free(git_commit *commit)
 {
 	clear_parents(commit);
 
-	if (commit->odb_open)
-		git_obj_close(&commit->odb_object);
-
 	free(commit->author);
 	free(commit->committer);
 	free(commit->message);
@@ -72,26 +69,14 @@ int git_commit__parse(git_commit *commit, unsigned int parse_flags, int close_db
 {
 	int error = 0;
 
-	if (!commit->odb_open) {
-		error = git_odb_read(&commit->odb_object, commit->object.repo->db, &commit->object.id);
-		if (error < 0)
+	if ((error = git_repository__open_dbo((git_repository_object *)commit)) < 0)
 		return error;
 
-		if (commit->odb_object.type != GIT_OBJ_COMMIT) {
-			git_obj_close(&commit->odb_object);
-			return GIT_EOBJTYPE;
-		}
-
-		commit->odb_open = 1;
-	}
-
 	error = git_commit__parse_buffer(commit,
-			commit->odb_object.data, commit->odb_object.len, parse_flags);
+			commit->object.dbo.data, commit->object.dbo.len, parse_flags);
 
-	if (close_db_object) {
-		git_obj_close(&commit->odb_object);
-		commit->odb_open = 0;
-	}
+	if (close_db_object)
+		git_repository__close_dbo((git_repository_object *)commit);
 
 	return error;
 }

src/commit.h

@@ -23,7 +23,6 @@ typedef struct git_commit_parents {
 
 struct git_commit {
 	git_repository_object object;
-	git_obj odb_object;
 
 	time_t commit_time;
 	git_commit_parents *parents;

src/repository.c

@@ -85,7 +85,9 @@ void git_repository_free(git_repository *repo)
 	while ((object = (git_repository_object *)
 				git_hashtable_iterator_next(&it)) != NULL) {
 
-		switch (object->type) {
+		git_obj_close(&object->dbo);
+	
+		switch (object->dbo.type) {
 		case GIT_OBJ_COMMIT:
 			git_commit__free((git_commit *)object);
 			break;
@@ -109,8 +111,39 @@ void git_repository_free(git_repository *repo)
 	free(repo);
 }
 
+int git_repository__open_dbo(git_repository_object *object)
+{
+	int error;
+
+	if (object->dbo_open)
+		return GIT_SUCCESS;
+
+	error = git_odb_read(&object->dbo, object->repo->db, &object->id);
+	if (error < 0)
+		return error;
+
+	object->dbo_open = 1;
+	return GIT_SUCCESS;
+}
+
+void git_repository__close_dbo(git_repository_object *object)
+{
+	if (!object->dbo_open) {
+		git_obj_close(&object->dbo);
+		object->dbo_open = 0;
+	}
+}
+
 git_repository_object *git_repository_lookup(git_repository *repo, const git_oid *id, git_otype type)
 {
+	static const size_t object_sizes[] = {
+		0,
+		sizeof(git_commit),
+		sizeof(git_tree),
+		sizeof(git_repository_object), /* TODO: sizeof(git_blob) */ 
+		sizeof(git_tag)
+	};
+
 	git_repository_object *object = NULL;
 	git_obj obj_file;
 
@@ -120,24 +153,61 @@ git_repository_object *git_repository_lookup(git_repository *repo, const git_oid
 	if (object != NULL)
 		return object;
 
-	if (git_odb_read(&obj_file, repo->db, id) < 0 ||
-		(type != GIT_OBJ_ANY && type != obj_file.type))
+	if (git_odb_read(&obj_file, repo->db, id) < 0)
+		return NULL;
+
+	if (type != GIT_OBJ_ANY && type != obj_file.type)
 		return NULL;
 
-	object = git__malloc(sizeof(git_commit));
+	type = obj_file.type;
+
+	object = git__malloc(object_sizes[type]);
 
 	if (object == NULL)
 		return NULL;
 
-	memset(object, 0x0, sizeof(git_commit));
+	memset(object, 0x0, object_sizes[type]);
 
 	/* Initialize parent object */
 	git_oid_cpy(&object->id, id);
 	object->repo = repo;
-	object->type = obj_file.type;
+	object->dbo_open = 1;
+	memcpy(&object->dbo, &obj_file, sizeof(git_obj));
 
-	git_hashtable_insert(repo->objects, &object->id, object);
-	git_obj_close(&obj_file);
+	switch (type) {
 
+	case GIT_OBJ_COMMIT:
+		if (git_commit__parse_basic((git_commit *)object) < 0) {
+			free(object);
+			return NULL;
+		}
+
+		break;
+
+	case GIT_OBJ_TREE:
+		if (git_tree__parse((git_tree *)object) < 0) {
+			free(object);
+			return NULL;
+		}
+
+		break;
+
+	case GIT_OBJ_TAG:
+		if (git_tag__parse((git_tag *)object) < 0) {
+			free(object);
+			return NULL;
+		}
+
+		break;
+
+	default:
+		/* blobs get no parsing */
+		break;
+	}
+
+	git_obj_close(&object->dbo);
+	object->dbo_open = 0;
+
+	git_hashtable_insert(repo->objects, &object->id, object);
 	return object;
 }

src/repository.h

@@ -11,7 +11,8 @@
 struct git_repository_object {
 	git_oid id;
 	git_repository *repo;
-	git_otype type;
+	git_obj dbo;
+	int dbo_open;
 };
 
 struct git_repository {
@@ -19,7 +20,8 @@ struct git_repository {
 	git_hashtable *objects;
 };
 
-int git_repository__insert(git_repository *repo, git_repository_object *obj);
-git_repository_object *git_repository__lookup(git_repository *repo, const git_oid *id);
+
+int git_repository__open_dbo(git_repository_object *object);
+void git_repository__close_dbo(git_repository_object *object);
 
 #endif

src/tag.c

@@ -45,47 +45,26 @@ const git_oid *git_tag_id(git_tag *t)
 
 const git_repository_object *git_tag_target(git_tag *t)
 {
-	if (t->target)
-		return t->target;
-
-	git_tag__parse(t);
 	return t->target;
 }
 
 git_otype git_tag_type(git_tag *t)
 {
-	if (t->type)
-		return t->type;
-
-	git_tag__parse(t);
 	return t->type;
 }
 
-
 const char *git_tag_name(git_tag *t)
 {
-	if (t->tag_name)
-		return t->tag_name;
-
-	git_tag__parse(t);
 	return t->tag_name;
 }
 
 const git_person *git_tag_tagger(git_tag *t)
 {
-	if (t->tagger)
-		return t->tagger;
-
-	git_tag__parse(t);
 	return t->tagger;
 }
 
 const char *git_tag_message(git_tag *t)
 {
-	if (t->message)
-		return t->message;
-
-	git_tag__parse(t);
 	return t->message;
 }
 
@@ -181,21 +160,14 @@ static int parse_tag_buffer(git_tag *tag, char *buffer, const char *buffer_end)
 int git_tag__parse(git_tag *tag)
 {
 	int error = 0;
-	git_obj odb_object;
 
-	error = git_odb_read(&odb_object, tag->object.repo->db, &tag->object.id);
+	error = git_repository__open_dbo((git_repository_object *)tag);
 	if (error < 0)
 		return error;
 
-	if (odb_object.type != GIT_OBJ_TAG) {
-		error = GIT_EOBJTYPE;
-		goto cleanup;
-	}
-
-	error = parse_tag_buffer(tag, odb_object.data, odb_object.data + odb_object.len);
+	error = parse_tag_buffer(tag, tag->object.dbo.data, tag->object.dbo.data + tag->object.dbo.len);
 
-cleanup:
-	git_obj_close(&odb_object);
+	git_repository__close_dbo((git_repository_object *)tag);
 	return error;
 }
 

src/tree.c

@@ -80,19 +80,15 @@ int entry_cmp(const void *key, const void *array_member)
 
 const git_tree_entry *git_tree_entry_byname(git_tree *tree, const char *filename)
 {
-	if (tree->entries == NULL)
-		git_tree__parse(tree);
-
 	return bsearch(filename, tree->entries, tree->entry_count, sizeof(git_tree_entry), entry_cmp);
 }
 
 const git_tree_entry *git_tree_entry_byindex(git_tree *tree, int idx)
 {
 	if (tree->entries == NULL)
-		git_tree__parse(tree);
+		return NULL;
 
-	return (tree->entries && idx >= 0 && idx < (int)tree->entry_count) ? 
-		&tree->entries[idx] : NULL;
+	return (idx >= 0 && idx < (int)tree->entry_count) ? &tree->entries[idx] : NULL;
 }
 
 size_t git_tree_entrycount(git_tree *tree)
@@ -105,22 +101,21 @@ int git_tree__parse(git_tree *tree)
 	static const size_t avg_entry_size = 40;
 
 	int error = 0;
-	git_obj odb_object;
 	char *buffer, *buffer_end;
 	size_t entries_size;
 
 	if (tree->entries != NULL)
 		return GIT_SUCCESS;
 
-	error = git_odb_read(&odb_object, tree->object.repo->db, &tree->object.id);
+	error = git_repository__open_dbo((git_repository_object *)tree);
 	if (error < 0)
 		return error;
 
-	buffer = odb_object.data;
-	buffer_end = odb_object.data + odb_object.len;
+	buffer = tree->object.dbo.data;
+	buffer_end = buffer + tree->object.dbo.len;
 
 	tree->entry_count = 0;
-	entries_size = (odb_object.len / avg_entry_size) + 1;
+	entries_size = (tree->object.dbo.len / avg_entry_size) + 1;
 	tree->entries = git__malloc(entries_size * sizeof(git_tree_entry));
 
 	while (buffer < buffer_end) {
@@ -160,6 +155,6 @@ int git_tree__parse(git_tree *tree)
 		buffer += GIT_OID_RAWSZ;
 	}
 
-	git_obj_close(&odb_object);
+	git_repository__close_dbo((git_repository_object *)tree);
 	return error;
 }