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>

1 files changed, 68 insertions, 164 deletions

diff --git a/src/commit.c b/src/commit.c
index 90a30e020..a2d7221d1 100644
--- a/src/commit.c
+++ b/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;
 }
 
+