Fix internal memory management on the library

String mememory is now managed in a much more sane manner.

Fixes include:

	- git_person email and name is no longer limited to 64 characters
	- git_tree_entry filename is no longer limited to 255 characters
	- raw objects are properly opened & closed the minimum amount of
	times required for parsing
	- unit tests no longer leak
	- removed 5 other misc memory leaks as reported by Valgrind
	- tree writeback no longer segfaults on rare ocassions

The git_person struct is no longer public. It is now managed by the
library, and getter methods are in place to access its internal
attributes.

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

src/commit.c

@@ -23,11 +23,14 @@
  * Boston, MA 02110-1301, USA.
  */
 
+#include "git/common.h"
+#include "git/odb.h"
+#include "git/repository.h"
+
 #include "common.h"
 #include "commit.h"
 #include "revwalk.h"
-#include "git/odb.h"
-#include "git/repository.h"
+#include "person.h"
 
 #define COMMIT_BASIC_PARSE 0x0
 #define COMMIT_FULL_PARSE 0x1
@@ -56,8 +59,9 @@ void git_commit__free(git_commit *commit)
 {
 	clear_parents(commit);
 
-	free(commit->author);
-	free(commit->committer);
+	git_person__free(commit->author);
+	git_person__free(commit->committer);
+
 	free(commit->message);
 	free(commit->message_short);
 	free(commit);
@@ -73,146 +77,12 @@ const git_oid *git_commit_id(git_commit *c)
 	return git_object_id((git_object *)c);
 }
 
-int git_commit__parse(git_commit *commit)
-{
-	const int close_db_object = 1;
-	int error = 0;
-
-	if ((error = git_object__source_open((git_object *)commit)) < 0)
-		return error;
-
-	error = git_commit__parse_buffer(commit,
-			commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_BASIC_PARSE);
-
-	if (close_db_object)
-		git_object__source_close((git_object *)commit);
-
-	return error;
-}
-
-int git_commit__parse_full(git_commit *commit)
-{
-	int error;
-
-	if (commit->full_parse)
-		return 0;
-
-	if (git_object__source_open((git_object *)commit) < 0)
-		return GIT_ERROR;
-
-	error = git_commit__parse_buffer(commit,
-			commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_FULL_PARSE);
-
-	git_object__source_close((git_object *)commit);
-
-	commit->full_parse = 1;
-	return error;
-}
 
 git_commit *git_commit_lookup(git_repository *repo, const git_oid *id)
 {
 	return (git_commit *)git_repository_lookup(repo, id, GIT_OBJ_COMMIT);
 }
 
-int git__parse_person(git_person *person, char **buffer_out,
-		const char *buffer_end, const char *header)
-{
-	const size_t header_len = strlen(header);
-
-	int i;
-	char *buffer = *buffer_out;
-	char *line_end, *name, *email;
-
-	line_end = memchr(buffer, '\n', buffer_end - buffer);
-	if (!line_end)
-		return GIT_EOBJCORRUPTED;
-
-	if (buffer + (header_len + 1) > line_end)
-		return GIT_EOBJCORRUPTED;
-
-	if (memcmp(buffer, header, header_len) != 0)
-		return GIT_EOBJCORRUPTED;
-
-	buffer += header_len;
-
-
-	/* Parse name field */
-	for (i = 0, name = person->name;
-		 i < 64 && buffer < line_end && *buffer != '<';
-		 ++i)
-		*name++ = *buffer++;
-
-	*(name - 1) = 0;
-
-	while (buffer < line_end && *buffer != '<')
-		buffer++;
-
-	if (++buffer >= line_end)
-		return GIT_EOBJCORRUPTED;
-
-	/* Parse email field */
-	for (i = 0, email = person->email;
-		 i < 64 && buffer < line_end && *buffer != '>';
-		 ++i)
-		*email++ = *buffer++;
-
-	*email = 0;
-
-	while (buffer < line_end && *buffer != '>')
-		buffer++;
-
-	if (++buffer >= line_end)
-		return GIT_EOBJCORRUPTED;
-
-	person->time = strtol(buffer, &buffer, 10);
-
-	if (person->time == 0)
-		return GIT_EOBJCORRUPTED;
-
-	*buffer_out = (line_end + 1);
-	return 0;
-}
-
-int git__write_person(git_odb_source *src, const char *header, const git_person *person)
-{
-	return git__source_printf(src, "%s %s <%s> %u\n", header, person->name, person->email, person->time);
-}
-
-int git__parse_oid(git_oid *oid, char **buffer_out,
-		const char *buffer_end, const char *header)
-{
-	const size_t sha_len = GIT_OID_HEXSZ;
-	const size_t header_len = strlen(header);
-
-	char *buffer = *buffer_out;
-
-	if (buffer + (header_len + sha_len + 1) > buffer_end)
-		return GIT_EOBJCORRUPTED;
-
-	if (memcmp(buffer, header, header_len) != 0)
-		return GIT_EOBJCORRUPTED;
-
-	if (buffer[header_len + sha_len] != '\n')
-		return GIT_EOBJCORRUPTED;
-
-	if (git_oid_mkstr(oid, buffer + header_len) < 0)
-		return GIT_EOBJCORRUPTED;
-
-	*buffer_out = buffer + (header_len + sha_len + 1);
-
-	return 0;
-}
-
-int git__write_oid(git_odb_source *src, const char *header, const git_oid *oid)
-{
-	char hex_oid[41];
-
-	git_oid_fmt(hex_oid, oid);
-	hex_oid[40] = 0;
-
-	return git__source_printf(src, "%s %s\n", header, hex_oid);
-}
-
 int git_commit__writeback(git_commit *commit, git_odb_source *src)
 {
 	git_commit_parents *parent;
@@ -232,12 +102,12 @@ int git_commit__writeback(git_commit *commit, git_odb_source *src)
 	if (commit->author == NULL)
 		return GIT_ERROR;
 
-	git__write_person(src, "author", commit->author);
+	git_person__write(src, "author", commit->author);
 
 	if (commit->committer == NULL)
 		return GIT_ERROR;
 
-	git__write_person(src, "committer", commit->committer);
+	git_person__write(src, "committer", commit->committer);
 
 	if (commit->message != NULL)
 		git__source_printf(src, "\n%s", commit->message);
@@ -245,13 +115,12 @@ int git_commit__writeback(git_commit *commit, git_odb_source *src)
 	return GIT_SUCCESS;
 }
 
-int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags)
+int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags)
 {
 	char *buffer = (char *)data;
 	const char *buffer_end = (char *)data + len;
 
 	git_oid oid;
-	git_person person;
 
 	if (git__parse_oid(&oid, &buffer, buffer_end, "tree ") < 0)
 		return GIT_EOBJCORRUPTED;
@@ -279,29 +148,31 @@ int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigne
 		commit->parents = node;
 	}
 
-	if (git__parse_person(&person, &buffer, buffer_end, "author ") < 0)
-		return GIT_EOBJCORRUPTED;
 
 	if (parse_flags & COMMIT_FULL_PARSE) {
 		if (commit->author)
-			free(commit->author);
+			git_person__free(commit->author);
 
 		commit->author = git__malloc(sizeof(git_person));
-		memcpy(commit->author, &person, sizeof(git_person));
-	}
+		if (git_person__parse(commit->author, &buffer, buffer_end, "author ") < 0)
+			return GIT_EOBJCORRUPTED;
 
-	if (git__parse_person(&person, &buffer, buffer_end, "committer ") < 0)
-		return GIT_EOBJCORRUPTED;
+	} else {
+		if ((buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL)
+			return GIT_EOBJCORRUPTED;
 
-	commit->commit_time = person.time;
+		buffer++;
+	}
 
-	if (parse_flags & COMMIT_FULL_PARSE) {
-		if (commit->committer)
-			free(commit->committer);
+	/* Always parse the committer; we need the commit time */
+	if (commit->committer)
+		git_person__free(commit->committer);
 
-		commit->committer = git__malloc(sizeof(git_person));
-		memcpy(commit->committer, &person, sizeof(git_person));
-	}
+	commit->committer = git__malloc(sizeof(git_person));
+	if (git_person__parse(commit->committer, &buffer, buffer_end, "committer ") < 0)
+		return GIT_EOBJCORRUPTED;
+
+	commit->commit_time = commit->committer->time;
 
 	/* parse commit message */
 	while (buffer <= buffer_end && *buffer == '\n')
@@ -329,9 +200,38 @@ int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigne
 	return 0;
 }
 
+int git_commit__parse(git_commit *commit)
+{
+	assert(commit && commit->object.source.open);
+	return commit_parse_buffer(commit,
+			commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_BASIC_PARSE);
+}
+
+int git_commit__parse_full(git_commit *commit)
+{
+	int error;
+
+	if (commit->full_parse)
+		return 0;
+
+	if (git_object__source_open((git_object *)commit) < 0)
+		return GIT_ERROR;
+
+	error = commit_parse_buffer(commit,
+			commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_FULL_PARSE);
+
+	git_object__source_close((git_object *)commit);
+
+	commit->full_parse = 1;
+	return error;
+}
+
+
+
 #define GIT_COMMIT_GETTER(_rvalue, _name) \
 	const _rvalue git_commit_##_name(git_commit *commit) \
 	{\
+		assert(commit); \
 		if (commit->_name) \
 			return commit->_name; \
 		git_commit__parse_full(commit); \
@@ -346,6 +246,8 @@ GIT_COMMIT_GETTER(char *, message_short)
 
 time_t git_commit_time(git_commit *commit)
 {
+	assert(commit);
+
 	if (commit->commit_time)
 		return commit->commit_time;
 
@@ -355,26 +257,27 @@ time_t git_commit_time(git_commit *commit)
 
 void git_commit_set_tree(git_commit *commit, git_tree *tree)
 {
+	assert(commit && tree);
 	commit->object.modified = 1;
 	commit->tree = tree;
 }
 
-void git_commit_set_author(git_commit *commit, const git_person *author)
+void git_commit_set_author(git_commit *commit, const char *name, const char *email, time_t time)
 {
+	assert(commit && name && email);
 	commit->object.modified = 1;
-	if (commit->author == NULL)
-		commit->author = git__malloc(sizeof(git_person));
 
-	memcpy(commit->author, author, sizeof(git_person));
+	git_person__free(commit->author);
+	commit->author = git_person__new(name, email, time);
 }
 
-void git_commit_set_committer(git_commit *commit, const git_person *committer)
+void git_commit_set_committer(git_commit *commit, const char *name, const char *email, time_t time)
 {
+	assert(commit && name && email);
 	commit->object.modified = 1;
-	if (commit->committer == NULL)
-		commit->committer = git__malloc(sizeof(git_person));
 
-	memcpy(commit->committer, committer, sizeof(git_person));
+	git_person__free(commit->committer);
+	commit->committer = git_person__new(name, email, time);
 }
 
 void git_commit_set_message(git_commit *commit, const char *message)
@@ -409,3 +312,4 @@ void git_commit_add_parent(git_commit *commit, git_commit *new_parent)
 	commit->parents = node;
 }
 
+

src/commit.h

@@ -31,14 +31,7 @@ struct git_commit {
 void git_commit__free(git_commit *c);
 int git_commit__parse(git_commit *commit);
 int git_commit__parse_full(git_commit *commit);
-int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags);
 
 int git_commit__writeback(git_commit *commit, git_odb_source *src);
 
-int git__parse_oid(git_oid *oid, char **buffer_out, const char *buffer_end, const char *header);
-int git__parse_person(git_person *person, char **buffer_out, const char *buffer_end, const char *header);
-
-int git__write_oid(git_odb_source *src, const char *header, const git_oid *oid);
-int git__write_person(git_odb_source *src, const char *header, const git_person *person);
-
 #endif

src/git/commit.h

@@ -108,16 +108,20 @@ GIT_EXTERN(void) git_commit_set_message(git_commit *commit, const char *message)
 /**
  * Set the committer of a commit
  * @param commit the commit object
- * @param committer the new committer
+ * @param name name of the new committer
+ * @param email email of the new committer
+ * @param time time when the committer committed the commit
  */
-GIT_EXTERN(void) git_commit_set_committer(git_commit *commit, const git_person *committer);
+GIT_EXTERN(void) git_commit_set_committer(git_commit *commit, const char *name, const char *email, time_t time);
 
 /**
  * Set the author of a commit
  * @param commit the commit object
- * @param author the new author
+ * @param name name of the new author
+ * @param email email of the new author
+ * @param time time when the author created the commit
  */
-GIT_EXTERN(void) git_commit_set_author(git_commit *commit, const git_person *author);
+GIT_EXTERN(void) git_commit_set_author(git_commit *commit, const char *name, const char *email, time_t time);
 
 /**
  * Set the tree which is pointed to by a commit

src/git/common.h

@@ -2,6 +2,7 @@
 #define INCLUDE_git_common_h__
 
 #include "thread-utils.h"
+#include <time.h>
 
 #ifdef __cplusplus
 # define GIT_BEGIN_DECL  extern "C" {
@@ -94,12 +95,13 @@ typedef struct git_repository git_repository;
 /** Representation of a generic object in a repository */
 typedef struct git_object git_object;
 
+
 /** Parsed representation of a person */
-typedef struct git_person {
-	char name[64]; /**< Full name */
-	char email[64]; /**< Email address */
-	time_t time; /**< Time when this person committed the change */
-} git_person;
+typedef struct git_person git_person;
+
+const char *git_person_name(git_person *person);
+const char *git_person_email(git_person *person);
+time_t git_person_time(git_person *person);
 
 /** @} */
 GIT_END_DECL

src/git/tag.h

@@ -100,9 +100,11 @@ GIT_EXTERN(void) git_tag_set_name(git_tag *tag, const char *name);
 /**
  * Set the tagger of a tag
  * @param tag The tag to modify
- * @param tagger the new tagger for the tag
+ * @param name the name of the new tagger
+ * @param email the email of the new tagger
+ * @param time the time when the tag was created
  */
-GIT_EXTERN(void) git_tag_set_tagger(git_tag *tag, const git_person *tagger);
+GIT_EXTERN(void) git_tag_set_tagger(git_tag *tag, const char *name, const char *email, time_t time);
 
 /**
  * Set the message of a tag

src/oid.c

@@ -25,6 +25,7 @@
 
 #include "common.h"
 #include "git/oid.h"
+#include "repository.h"
 #include <string.h>
 
 static signed char from_hex[] = {
@@ -116,3 +117,37 @@ char *git_oid_to_string(char *out, size_t n, const git_oid *oid)
 	return out;
 }
 
+int git__parse_oid(git_oid *oid, char **buffer_out,
+		const char *buffer_end, const char *header)
+{
+	const size_t sha_len = GIT_OID_HEXSZ;
+	const size_t header_len = strlen(header);
+
+	char *buffer = *buffer_out;
+
+	if (buffer + (header_len + sha_len + 1) > buffer_end)
+		return GIT_EOBJCORRUPTED;
+
+	if (memcmp(buffer, header, header_len) != 0)
+		return GIT_EOBJCORRUPTED;
+
+	if (buffer[header_len + sha_len] != '\n')
+		return GIT_EOBJCORRUPTED;
+
+	if (git_oid_mkstr(oid, buffer + header_len) < 0)
+		return GIT_EOBJCORRUPTED;
+
+	*buffer_out = buffer + (header_len + sha_len + 1);
+
+	return 0;
+}
+
+int git__write_oid(git_odb_source *src, const char *header, const git_oid *oid)
+{
+	char hex_oid[41];
+
+	git_oid_fmt(hex_oid, oid);
+	hex_oid[40] = 0;
+
+	return git__source_printf(src, "%s %s\n", header, hex_oid);
+}

src/person.c

+/*
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2,
+ * as published by the Free Software Foundation.
+ *
+ * In addition to the permissions in the GNU General Public License,
+ * the authors give you unlimited permission to link the compiled
+ * version of this file into combinations with other programs,
+ * and to distribute those combinations without any restriction
+ * coming from the use of this file.  (The General Public License
+ * restrictions do apply in other respects; for example, they cover
+ * modification of the file, and distribution when not linked into
+ * a combined executable.)
+ *
+ * This file is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; see the file COPYING.  If not, write to
+ * the Free Software Foundation, 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#include "common.h"
+#include "person.h"
+#include "repository.h"
+#include "git/common.h"
+
+void git_person__free(git_person *person)
+{
+	if (person == NULL)
+		return;
+
+	free(person->name);
+	free(person->email);
+	free(person);
+}
+
+git_person *git_person__new(const char *name, const char *email, time_t time)
+{
+	git_person *p;
+
+	if ((p = git__malloc(sizeof(git_person))) == NULL)
+		goto cleanup;
+
+	p->name = git__strdup(name);
+	p->email = git__strdup(email);
+	p->time = time;
+
+	if (p->name == NULL || p->email == NULL)
+		goto cleanup;
+
+	return p;
+
+cleanup:
+	git_person__free(p);
+	return NULL;
+}
+
+const char *git_person_name(git_person *person)
+{
+	return person->name;
+}
+
+const char *git_person_email(git_person *person)
+{
+	return person->email;
+}
+
+time_t git_person_time(git_person *person)
+{
+	return person->time;
+}
+
+int git_person__parse(git_person *person, char **buffer_out,
+		const char *buffer_end, const char *header)
+{
+	const size_t header_len = strlen(header);
+
+	int name_length, email_length;
+	char *buffer = *buffer_out;
+	char *line_end, *name_end, *email_end;
+
+	memset(person, 0x0, sizeof(git_person));
+
+	line_end = memchr(buffer, '\n', buffer_end - buffer);
+	if (!line_end)
+		return GIT_EOBJCORRUPTED;
+
+	if (buffer + (header_len + 1) > line_end)
+		return GIT_EOBJCORRUPTED;
+
+	if (memcmp(buffer, header, header_len) != 0)
+		return GIT_EOBJCORRUPTED;
+
+	buffer += header_len;
+
+	/* Parse name */
+	if ((name_end = memchr(buffer, '<', buffer_end - buffer)) == NULL)
+		return GIT_EOBJCORRUPTED;
+
+	name_length = name_end - buffer - 1;
+	person->name = git__malloc(name_length + 1);
+	memcpy(person->name, buffer, name_length);
+	person->name[name_length] = 0;
+	buffer = name_end + 1;
+
+	if (buffer >= line_end)
+		return GIT_EOBJCORRUPTED;
+
+	/* Parse email */
+	if ((email_end = memchr(buffer, '>', buffer_end - buffer)) == NULL)
+		return GIT_EOBJCORRUPTED;
+
+	email_length = email_end - buffer;
+	person->email = git__malloc(email_length + 1);
+	memcpy(person->email, buffer, email_length);
+	person->email[email_length] = 0;
+	buffer = email_end + 1;
+
+	if (buffer >= line_end)
+		return GIT_EOBJCORRUPTED;
+
+	person->time = strtol(buffer, &buffer, 10);
+
+	if (person->time == 0)
+		return GIT_EOBJCORRUPTED;
+
+	*buffer_out = (line_end + 1);
+	return 0;
+}
+
+int git_person__write(git_odb_source *src, const char *header, const git_person *person)
+{
+	return git__source_printf(src, "%s %s <%s> %u\n", header, person->name, person->email, person->time);
+}
+
+

src/person.h

+#ifndef INCLUDE_person_h__
+#define INCLUDE_person_h__
+
+#include "git/common.h"
+#include "repository.h"
+#include <time.h>
+
+/** Parsed representation of a person */
+struct git_person {
+	char *name; /**< Full name */
+	char *email; /**< Email address */
+	time_t time; /**< Time when this person committed the change */
+};
+
+void git_person__free(git_person *person);
+git_person *git_person__new(const char *name, const char *email, time_t time);
+int git_person__parse(git_person *person, char **buffer_out, const char *buffer_end, const char *header);
+int git_person__write(git_odb_source *src, const char *header, const git_person *person);
+
+#endif

src/repository.c

@@ -32,6 +32,8 @@
 static const int default_table_size = 32;
 static const double max_load_factor = 0.65;
 
+static const int OBJECT_BASE_SIZE = 4096;
+
 static const size_t object_sizes[] = {
 	0,
 	sizeof(git_commit),
@@ -171,14 +173,12 @@ int git__source_write(git_odb_source *source, const void *bytes, size_t len)
 
 static void prepare_write(git_object *object)
 {
-	const size_t base_size = 4096; /* 4Kb base size */
-
 	if (object->source.write_ptr != NULL || object->source.open)
 		git_object__source_close(object);
 
 	/* TODO: proper size calculation */
-	object->source.raw.data = git__malloc(base_size);
-	object->source.raw.len = base_size;
+	object->source.raw.data = git__malloc(OBJECT_BASE_SIZE);
+	object->source.raw.len = OBJECT_BASE_SIZE;
 
 	object->source.write_ptr = object->source.raw.data;
 	object->source.written_bytes = 0;
@@ -193,11 +193,9 @@ static int write_back(git_object *object)
 
 	assert(object);
 
-	if (!object->source.open)
-		return GIT_ERROR;
-
+	assert(object->source.open);
 	assert(object->modified);
-	
+
 	object->source.raw.len = object->source.written_bytes;
 
 	git_obj_hash(&new_id, &object->source.raw);
@@ -230,9 +228,6 @@ int git_object__source_open(git_object *object)
 	if (object->source.open)
 		git_object__source_close(object);
 
-	if (object->source.open)
-		return GIT_SUCCESS;
-
 	error = git_odb_read(&object->source.raw, object->repo->db, &object->id);
 	if (error < 0)
 		return error;
@@ -245,7 +240,7 @@ void git_object__source_close(git_object *object)
 {
 	assert(object);
 
-	if (!object->source.open) {
+	if (object->source.open) {
 		git_obj_close(&object->source.raw);
 		object->source.open = 0;
 	}
@@ -294,8 +289,8 @@ void git_object_free(git_object *object)
 {
 	assert(object);
 
+	git_object__source_close(object);
 	git_hashtable_remove(object->repo->objects, &object->id);
-	git_obj_close(&object->source.raw);
 
 	switch (object->source.raw.type) {
 	case GIT_OBJ_COMMIT:
@@ -372,6 +367,7 @@ git_object *git_repository_lookup(git_repository *repo, const git_oid *id, git_o
 {
 	git_object *object = NULL;
 	git_rawobj obj_file;
+	int error = 0;
 
 	assert(repo);
 
@@ -397,43 +393,35 @@ git_object *git_repository_lookup(git_repository *repo, const git_oid *id, git_o
 	/* Initialize parent object */
 	git_oid_cpy(&object->id, id);
 	object->repo = repo;
-	object->source.open = 1;
 	memcpy(&object->source.raw, &obj_file, sizeof(git_rawobj));
+	object->source.open = 1;
 
 	switch (type) {
 
 	case GIT_OBJ_COMMIT:
-		if (git_commit__parse((git_commit *)object) < 0) {
-			free(object);
-			return NULL;
-		}
-
+		error = git_commit__parse((git_commit *)object);
 		break;
 
 	case GIT_OBJ_TREE:
-		if (git_tree__parse((git_tree *)object) < 0) {
-			free(object);
-			return NULL;
-		}
-
+		error = git_tree__parse((git_tree *)object);
 		break;
 
 	case GIT_OBJ_TAG:
-		if (git_tag__parse((git_tag *)object) < 0) {
-			free(object);
-			return NULL;
-		}
-
+		error = git_tag__parse((git_tag *)object);
 		break;
 
+	case GIT_OBJ_BLOB:
 	default:
 		/* blobs get no parsing */
 		break;
 	}
 
-	git_obj_close(&object->source.raw);
-	object->source.open = 0;
+	if (error < 0) {
+		git_object_free(object);
+		return NULL;
+	}
 
+	git_object__source_close(object);
 	git_hashtable_insert(repo->objects, &object->id, object);
 	return object;
 }

src/repository.h

@@ -34,4 +34,7 @@ void git_object__source_close(git_object *object);
 int git__source_printf(git_odb_source *source, const char *format, ...);
 int git__source_write(git_odb_source *source, const void *bytes, size_t len);
 
+int git__parse_oid(git_oid *oid, char **buffer_out, const char *buffer_end, const char *header);
+int git__write_oid(git_odb_source *src, const char *header, const git_oid *oid);
+
 #endif

src/tag.c

@@ -26,15 +26,16 @@
 #include "common.h"
 #include "commit.h"
 #include "tag.h"
+#include "person.h"
 #include "revwalk.h"
 #include "git/odb.h"
 #include "git/repository.h"
 
 void git_tag__free(git_tag *tag)
 {
+	git_person__free(tag->tagger);
 	free(tag->message);
 	free(tag->tag_name);
-	free(tag->tagger);
 	free(tag);
 }
 
@@ -50,6 +51,7 @@ const git_oid *git_tag_id(git_tag *c)
 
 const git_object *git_tag_target(git_tag *t)
 {
+	assert(t);
 	return t->target;
 }
 
@@ -64,6 +66,7 @@ void git_tag_set_target(git_tag *tag, git_object *target)
 
 git_otype git_tag_type(git_tag *t)
 {
+	assert(t);
 	return t->type;
 }
 
@@ -77,6 +80,7 @@ void git_tag_set_type(git_tag *tag, git_otype type)
 
 const char *git_tag_name(git_tag *t)
 {
+	assert(t);
 	return t->tag_name;
 }
 
@@ -98,19 +102,18 @@ const git_person *git_tag_tagger(git_tag *t)
 	return t->tagger;
 }
 
-void git_tag_set_tagger(git_tag *tag, const git_person *tagger)
+void git_tag_set_tagger(git_tag *tag, const char *name, const char *email, time_t time)
 {
-	assert(tag && tagger);
-
+	assert(tag && name && email);
 	tag->object.modified = 1;
-	if (tag->tagger == NULL)
-		tag->tagger = git__malloc(sizeof(git_person));
 
-	memcpy(tag->tagger, tagger, sizeof(git_person));
+	git_person__free(tag->tagger);
+	tag->tagger = git_person__new(name, email, time);
 }
 
 const char *git_tag_message(git_tag *t)
 {
+	assert(t);
 	return t->message;
 }
 
@@ -194,11 +197,11 @@ static int parse_tag_buffer(git_tag *tag, char *buffer, const char *buffer_end)
 	buffer = search + 1;
 
 	if (tag->tagger != NULL)
-		free(tag->tagger);
+		git_person__free(tag->tagger);
 
 	tag->tagger = git__malloc(sizeof(git_person));
 
-	if (git__parse_person(tag->tagger, &buffer, buffer_end, "tagger ") != 0)
+	if (git_person__parse(tag->tagger, &buffer, buffer_end, "tagger ") != 0)
 		return GIT_EOBJCORRUPTED;
 
 	text_len = buffer_end - ++buffer;
@@ -221,7 +224,7 @@ int git_tag__writeback(git_tag *tag, git_odb_source *src)
 	git__write_oid(src, "object", git_object_id(tag->target));
 	git__source_printf(src, "type %s\n", git_obj_type_to_string(tag->type));
 	git__source_printf(src, "tag %s\n", tag->tag_name);
-	git__write_person(src, "tagger", tag->tagger);
+	git_person__write(src, "tagger", tag->tagger);
 
 	if (tag->message != NULL)
 		git__source_printf(src, "\n%s", tag->message);
@@ -232,16 +235,8 @@ int git_tag__writeback(git_tag *tag, git_odb_source *src)
 
 int git_tag__parse(git_tag *tag)
 {
-	int error = 0;
-
-	error = git_object__source_open((git_object *)tag);
-	if (error < 0)
-		return error;
-
-	error = parse_tag_buffer(tag, tag->object.source.raw.data, tag->object.source.raw.data + tag->object.source.raw.len);
-
-	git_object__source_close((git_object *)tag);
-	return error;
+	assert(tag && tag->object.source.open);
+	return parse_tag_buffer(tag, tag->object.source.raw.data, tag->object.source.raw.data + tag->object.source.raw.len);
 }
 
 git_tag *git_tag_lookup(git_repository *repo, const git_oid *id)

src/tree.c

@@ -70,17 +70,26 @@ static void entry_resort(git_tree *tree)
 	qsort(tree->entries, tree->entry_count, sizeof(git_tree_entry *), entry_sort_cmp);
 }
 
-
-
-
-void git_tree__free(git_tree *tree)
+static void free_tree_entries(git_tree *tree)
 {
 	size_t i;
 
-	for (i = 0; i < tree->entry_count; ++i)
+	if (tree == NULL)
+		return;
+
+	for (i = 0; i < tree->entry_count; ++i) {
+		free(tree->entries[i]->filename);
 		free(tree->entries[i]);
+	}
 
 	free(tree->entries);
+}
+
+
+
+void git_tree__free(git_tree *tree)
+{
+	free_tree_entries(tree);
 	free(tree);
 }
 
@@ -111,7 +120,8 @@ void git_tree_entry_set_name(git_tree_entry *entry, const char *name)
 {
 	assert(entry && entry->owner);
 
-	strncpy(entry->filename, name, GIT_TREE_MAX_FILENAME);
+	free(entry->filename);
+	entry->filename = git__strdup(name);
 	entry_resort(entry->owner);
 	entry->owner->object.modified = 1;
 }
@@ -188,7 +198,7 @@ int git_tree_add_entry(git_tree *tree, const git_oid *id, const char *filename,
 
 	memset(entry, 0x0, sizeof(git_tree_entry));
 
-	strncpy(entry->filename, filename, GIT_TREE_MAX_FILENAME);
+	entry->filename = git__strdup(filename);
 	git_oid_cpy(&entry->oid, id);
 	entry->attr = attributes;
 	entry->owner = tree;
@@ -212,6 +222,7 @@ int git_tree_remove_entry_byindex(git_tree *tree, int idx)
 	remove_ptr = tree->entries[idx];
 	tree->entries[idx] = tree->entries[--tree->entry_count];
 
+	free(remove_ptr->filename);
 	free(remove_ptr);
 	entry_resort(tree);
 
@@ -237,6 +248,7 @@ int git_tree_remove_entry_byname(git_tree *tree, const char *filename)
 int git_tree__writeback(git_tree *tree, git_odb_source *src)
 {
 	size_t i;
+	char filemode[8];
 
 	assert(tree && src);
 
@@ -249,38 +261,23 @@ int git_tree__writeback(git_tree *tree, git_odb_source *src)
 		git_tree_entry *entry;
 		entry = tree->entries[i];
 	
-		git__source_printf(src, "%06o %s\0", entry->attr, entry->filename);
+		sprintf(filemode, "%06o ", entry->attr);
+
+		git__source_write(src, filemode, strlen(filemode));
+		git__source_write(src, entry->filename, strlen(entry->filename) + 1);
 		git__source_write(src, entry->oid.id, GIT_OID_RAWSZ);
-	}
+	} 
 
 	return GIT_SUCCESS;
 }
 
 
-int git_tree__parse(git_tree *tree)
+static int tree_parse_buffer(git_tree *tree, char *buffer, char *buffer_end)
 {
 	static const size_t avg_entry_size = 40;
-
 	int error = 0;
-	char *buffer, *buffer_end;
-
-	assert(!tree->object.in_memory);
 
-	if (tree->entries != NULL) {
-		size_t i;
-
-		for (i = 0; i < tree->entry_count; ++i)
-			free(tree->entries[i]);
-
-		free(tree->entries);
-	}
-
-	error = git_object__source_open((git_object *)tree);
-	if (error < 0)
-		return error;
-
-	buffer = tree->object.source.raw.data;
-	buffer_end = buffer + tree->object.source.raw.len;
+	free_tree_entries(tree);
 
 	tree->entry_count = 0;
 	tree->array_size = (tree->object.source.raw.len / avg_entry_size) + 1;
@@ -312,7 +309,12 @@ int git_tree__parse(git_tree *tree)
 			break;
 		}
 
-		strncpy(entry->filename, buffer, GIT_TREE_MAX_FILENAME);
+		if (memchr(buffer, 0, buffer_end - buffer) == NULL) {
+			error = GIT_EOBJCORRUPTED;
+			break;
+		}
+
+		entry->filename = git__strdup(buffer);
 
 		while (buffer < buffer_end && *buffer != 0)
 			buffer++;
@@ -323,6 +325,19 @@ int git_tree__parse(git_tree *tree)
 		buffer += GIT_OID_RAWSZ;
 	}
 
-	git_object__source_close((git_object *)tree);
 	return error;
 }
+
+int git_tree__parse(git_tree *tree)
+{
+	char *buffer, *buffer_end;
+
+	assert(tree && tree->object.source.open);
+	assert(!tree->object.in_memory);
+
+	buffer = tree->object.source.raw.data;
+	buffer_end = buffer + tree->object.source.raw.len;
+
+	return tree_parse_buffer(tree, buffer, buffer_end);
+}
+

src/tree.h

@@ -4,11 +4,9 @@
 #include <git/tree.h>
 #include "repository.h"
 
-#define GIT_TREE_MAX_FILENAME 255
-
 struct git_tree_entry {
 	unsigned int attr;
-	char filename[GIT_TREE_MAX_FILENAME];
+	char *filename;
 	git_oid oid;
 
 	git_tree *owner;

tests/t0401-parse.c

 #include "test_lib.h"
 #include "test_helpers.h"
 #include "commit.h"
+#include "person.h"
 #include <git/odb.h>
 #include <git/commit.h>
 #include <git/revwalk.h>
@@ -127,18 +128,20 @@ BEGIN_TEST(parse_person_test)
 #define TEST_PERSON_PASS(_string, _header, _name, _email, _time) { \
 	char *ptr = _string; \
 	size_t len = strlen(_string);\
-	git_person person; \
-	must_pass(git__parse_person(&person, &ptr, ptr + len, _header));\
-	must_be_true(strncmp(_name, person.name, 63) == 0);\
-	must_be_true(strncmp(_email, person.email, 63) == 0);\
+	git_person person = {NULL, NULL, 0}; \
+	must_pass(git_person__parse(&person, &ptr, ptr + len, _header));\
+	must_be_true(strcmp(_name, person.name) == 0);\
+	must_be_true(strcmp(_email, person.email) == 0);\
 	must_be_true(_time == person.time);\
+	free(person.name); free(person.email);\
 }
 
 #define TEST_PERSON_FAIL(_string, _header) { \
 	char *ptr = _string; \
 	size_t len = strlen(_string);\
-	git_person person; \
-	must_fail(git__parse_person(&person, &ptr, ptr + len, _header));\
+	git_person person = {NULL, NULL, 0}; \
+	must_fail(git_person__parse(&person, &ptr, ptr + len, _header));\
+	free(person.name); free(person.email);\
 }
 
 	TEST_PERSON_PASS(
@@ -213,6 +216,9 @@ BEGIN_TEST(parse_person_test)
 
 END_TEST
 
+/* External declaration for testing the buffer parsing method */
+int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags);
+
 BEGIN_TEST(parse_buffer_test)
 	const int broken_commit_count = sizeof(test_commits_broken) / sizeof(*test_commits_broken);
 	const int working_commit_count = sizeof(test_commits_working) / sizeof(*test_commits_working);
@@ -232,7 +238,7 @@ BEGIN_TEST(parse_buffer_test)
 		memset(commit, 0x0, sizeof(git_commit));
 		commit->object.repo = repo;
 
-		must_fail(git_commit__parse_buffer(
+		must_fail(commit_parse_buffer(
 					commit,
 					test_commits_broken[i],
 					strlen(test_commits_broken[i]),
@@ -248,7 +254,7 @@ BEGIN_TEST(parse_buffer_test)
 		memset(commit, 0x0, sizeof(git_commit));
 		commit->object.repo = repo;
 
-		must_pass(git_commit__parse_buffer(
+		must_pass(commit_parse_buffer(
 					commit,
 					test_commits_working[i],
 					strlen(test_commits_working[i]),
@@ -259,5 +265,6 @@ BEGIN_TEST(parse_buffer_test)
 	}
 
 	git_repository_free(repo);
+	git_odb_close(db);
 
 END_TEST

tests/t0402-details.c

 #include "test_lib.h"
 #include "test_helpers.h"
 #include "commit.h"
+#include "person.h"
 
 #include <git/odb.h>
 #include <git/commit.h>
@@ -28,6 +29,7 @@ BEGIN_TEST(query_details_test)
 	repo = git_repository_alloc(db);
 	must_be_true(repo != NULL);
 
+	
 	for (i = 0; i < commit_count; ++i) {
 		git_oid id;
 		git_commit *commit;

tests/t0403-write.c

@@ -17,6 +17,9 @@ static const char *commit_ids[] = {
 };
 static const char *tree_oid = "1810dff58d8a660512d4832e740f692884338ccd";
 
+#define COMMITTER_NAME "Vicent Marti"
+#define COMMITTER_EMAIL "vicent@github.com"
+
 BEGIN_TEST(writenew_test)
 	git_odb *db;
 	git_repository *repo;
@@ -25,9 +28,6 @@ BEGIN_TEST(writenew_test)
 	git_oid id;
 	/* char hex_oid[41]; */
 
-	git_person author = {"Vicent Marti", "vicent@github.com", 123456789};
-	git_person committer = {"Vicent Marti", "vicent@github.com", 987654321};
-
 	must_pass(git_odb_open(&db, odb_dir));
 
 	repo = git_repository_alloc(db);
@@ -45,8 +45,8 @@ BEGIN_TEST(writenew_test)
 	git_commit_add_parent(commit, parent);
 
 	/* Set other attributes */
-	git_commit_set_committer(commit, &committer);
-	git_commit_set_author(commit, &author);
+	git_commit_set_committer(commit, COMMITTER_NAME, COMMITTER_EMAIL, 123456789);
+	git_commit_set_author(commit, COMMITTER_NAME, COMMITTER_EMAIL, 987654321);
 	git_commit_set_message(commit, 
 			"This commit has been created in memory\n\
 This is a commit created in memory and it will be written back to disk\n");
@@ -73,6 +73,9 @@ This is a commit created in memory and it will be written back to disk\n");
 
 	must_pass(remove_loose_object(odb_dir, (git_object *)commit));
 
+	//git_person_free(&author);
+	//git_person_free(&committer);
+
 	git_repository_free(repo);
 	git_odb_close(db);
 

tests/t0902-modify.c

@@ -38,6 +38,8 @@ BEGIN_TEST(tree_in_memory_add_test)
 	must_pass(git_object_write((git_object *)tree));
 	must_pass(remove_loose_object(odb_dir, (git_object *)tree));
 
+	git_object_free((git_object *)tree);
+
 	git_repository_free(repo);
 	git_odb_close(db);
 END_TEST
@@ -91,6 +93,8 @@ BEGIN_TEST(tree_add_entry_test)
 */
 
 	must_pass(remove_loose_object(odb_dir, (git_object *)tree));
+	
+	git_object_free((git_object *)tree);
 
 	git_repository_free(repo);
 	git_odb_close(db);