mailmap: Switch mailmap parsing to use the git_parse module

8 files changed, 206 insertions, 315 deletions

diff --git a/include/git2/mailmap.h b/include/git2/mailmap.h
index 0c722b549..30cc9cf69 100644
--- a/include/git2/mailmap.h
+++ b/include/git2/mailmap.h
@@ -9,6 +9,7 @@
 
 #include "common.h"
 #include "types.h"
+#include "buffer.h"
 
 /**
  * @file git2/mailmap.h
@@ -35,22 +36,20 @@ typedef struct git_mailmap_entry {
 #define GIT_MAILMAP_ENTRY_INIT {GIT_MAILMAP_ENTRY_VERSION}
 
 /**
- * Create a mailmap object by parsing a mailmap file.
+ * Create a mailmap object by parsing a mailmap file. Malformed entries in the
+ * mailmap are ignored.
  *
  * The mailmap must be freed with 'git_mailmap_free'.
  *
  * @param out pointer to store the mailmap
- * @param data raw data buffer to parse
- * @param size size of the raw data buffer
- * @return 0 on success
+ * @param buffer buffer to parse the mailmap from
+ * @return 0 on success, otherwise an error code
  */
-GIT_EXTERN(int) git_mailmap_parse(
-	git_mailmap **out,
-	const char *data,
-	size_t size);
+GIT_EXTERN(int) git_mailmap_from_buffer(git_mailmap **out, git_buf *buffer);
 
 /**
- * Create a mailmap object from the given repository.
+ * Create a mailmap object from the given repository. Malformed entries in the
+ * mailmap are ignored.
  *
  * If the repository is not bare, the repository's working directory root will
  * be checked for the '.mailmap' file to be parsed.
@@ -62,20 +61,23 @@ GIT_EXTERN(int) git_mailmap_parse(
  *
  * @param out pointer to store the mailmap
  * @param repo repository to find the .mailmap in
- * @return 0 on success; GIT_ENOTFOUND if .mailmap could not be found.
+ * @return 0 on success; GIT_ENOTFOUND if .mailmap could not be found,
+ *         otherwise an error code.
  */
-GIT_EXTERN(int) git_mailmap_from_repo(
-	git_mailmap **out,
-	git_repository *repo);
+GIT_EXTERN(int) git_mailmap_from_repo(git_mailmap **out, git_repository *repo);
 
 /**
- * Free a mailmap created by 'git_mailmap_parse' or 'git_mailmap_from_repo'.
+ * Free a mailmap created by 'git_mailmap_from_buffer' or
+ * 'git_mailmap_from_repo'.
  */
 GIT_EXTERN(void) git_mailmap_free(git_mailmap *mailmap);
 
 /**
  * Resolve a name and email to the corresponding real name and email.
  *
+ * The lifetime of the string results is tied to the `mailmap`, `name`, and
+ * `email` parameters.
+ *
  * @param name_out either 'name', or the real name to use.
  *             You should NOT free this value.
  * @param email_out either 'email' or the real email to use,
@@ -85,11 +87,8 @@ GIT_EXTERN(void) git_mailmap_free(git_mailmap *mailmap);
  * @param email the email to resolve.
  */
 GIT_EXTERN(void) git_mailmap_resolve(
-	const char **name_out,
-	const char **email_out,
-	const git_mailmap *mailmap,
-	const char *name,
-	const char *email);
+	const char **name_out, const char **email_out,
+	const git_mailmap *mailmap, const char *name, const char *email);
 
 /**
  * Get the number of mailmap entries in this mailmap.
@@ -104,8 +103,7 @@ GIT_EXTERN(size_t) git_mailmap_entry_count(const git_mailmap *mailmap);
  * @return the mailmap entry at index, or NULL if it cannot be found.
  */
 GIT_EXTERN(const git_mailmap_entry *) git_mailmap_entry_byindex(
-	const git_mailmap *mailmap,
-	size_t idx);
+	const git_mailmap *mailmap, size_t idx);
 
 /**
  * Lookup a mailmap entry by name/email pair.
@@ -118,9 +116,7 @@ GIT_EXTERN(const git_mailmap_entry *) git_mailmap_entry_byindex(
  * @return the corresponding mailmap entry, or NULL if it cannot be found.
  */
 GIT_EXTERN(const git_mailmap_entry *) git_mailmap_entry_lookup(
-	const git_mailmap *mailmap,
-	const char *name,
-	const char *email);
+	const git_mailmap *mailmap, const char *name, const char *email);
 
 /** @} */
 GIT_END_DECL
diff --git a/src/blame.h b/src/blame.h
index 057413274..b31d2dc20 100644
--- a/src/blame.h
+++ b/src/blame.h
@@ -67,6 +67,7 @@ typedef struct git_blame__entry {
 struct git_blame {
 	char *path;
 	git_repository *repository;
+	git_mailmap *mailmap;
 	git_blame_options options;
 
 	git_vector hunks;
@@ -84,8 +85,6 @@ struct git_blame {
 	int num_lines;
 	const char *final_buf;
 	git_off_t final_buf_size;
-
-	git_mailmap *mailmap;
 };
 
 git_blame *git_blame__alloc(
diff --git a/src/mailmap.c b/src/mailmap.c
index c0da98d48..caac82c09 100644
--- a/src/mailmap.c
+++ b/src/mailmap.c
@@ -9,216 +9,180 @@
 
 #include "blob.h"
 #include "commit.h"
+#include "parse.h"
 #include "git2/common.h"
 #include "git2/repository.h"
 #include "git2/revparse.h"
 #include "git2/sys/commit.h"
 
-/**
- * Helper type and methods for the mailmap parser
- */
-typedef struct char_range {
-	const char *p;
-	size_t len;
-} char_range;
+#define MAILMAP_FILE ".mailmap"
 
-static const char_range NULL_RANGE = {0};
+struct git_mailmap {
+	git_vector entries;
+};
 
-/* Split a range at the first instance of 'c'. Returns whether 'c' was found */
-static bool range_split(
-	char_range range,
-	char c,
-	char_range *before,
-	char_range *after)
+/* Check if we're at the end of line, w/ comments */
+static bool is_eol(git_parse_ctx *ctx)
 {
-	const char *off;
-
-	*before = *after = NULL_RANGE;
-	before->p = range.p;
-	off = memchr(range.p, c, range.len);
-	if (!off) {
-		before->len = range.len;
-		return false;
-	}
-
-	before->len = off - range.p;
-	after->p = off + 1;
-	after->len = (range.p + range.len) - after->p;
-	return true;
+	char c;
+	return git_parse_peek(&c, ctx, GIT_PARSE_PEEK_SKIP_WHITESPACE) < 0 || c == '#';
 }
 
-/* Trim whitespace from the beginning and end of the range */
-static void range_trim(char_range *range) {
-	while (range->len > 0 && git__isspace(range->p[0])) {
-		++range->p;
-		--range->len;
-	}
-	while (range->len > 0 && git__isspace(range->p[range->len - 1]))
-		--range->len;
+static int advance_until(
+	const char **start, size_t *len, git_parse_ctx *ctx, char needle)
+{
+	*start = ctx->line;
+	while (ctx->line_len > 0 && *ctx->line != '#' && *ctx->line != needle)
+		git_parse_advance_chars(ctx, 1);
+
+	if (ctx->line_len == 0 || *ctx->line == '#')
+		return -1; /* end of line */
+
+	*len = ctx->line - *start;
+	git_parse_advance_chars(ctx, 1); /* advance past needle */
+	return 0;
 }
 
-/**
- * If `buf` is not NULL, copies range into it with a '\0', and bumps buf.
- * If `size` is not NULL, adds the number of bytes to be written to it.
- * returns a pointer to the copied string, or NULL.
+/* Parse a single entry from a mailmap file.
+ *
+ * The output git_bufs will be non-owning, and should be copied before being
+ * persisted.
  */
-static const char *range_copyz(char **buf, size_t *size, char_range src)
+static int parse_mailmap_entry(
+	git_buf *real_name, git_buf *real_email,
+	git_buf *replace_name, git_buf *replace_email,
+	git_parse_ctx *ctx)
 {
-	char *s = NULL;
-	if (src.p == NULL)
-		return NULL;
+	const char *start;
+	size_t len;
 
-	if (size)
-		*size += src.len + 1;
+	git_buf_clear(real_name);
+	git_buf_clear(real_email);
+	git_buf_clear(replace_name);
+	git_buf_clear(replace_email);
 
-	if (buf) {
-		s = *buf;
-		memcpy(s, src.p, src.len);
-		s[src.len] = '\0';
-		*buf += src.len + 1;
-	}
-	return s;
-}
+	/* Parse the real name */
+	git_parse_advance_ws(ctx);
+	if (advance_until(&start, &len, ctx, '<') < 0)
+		return -1;
 
-struct git_mailmap {
-	git_vector entries;
-};
+	git_buf_attach_notowned(real_name, start, len);
+	git_buf_rtrim(real_name);
 
-/**
- * Parse a single entry out of a mailmap file.
- * Advances the `file` range past the parsed entry.
- */
-static int git_mailmap_parse_single(
-	char_range *file,
-	bool *found,
-	char_range *real_name,
-	char_range *real_email,
-	char_range *replace_name,
-	char_range *replace_email)
-{
-	char_range line, comment, name_a, email_a, name_b, email_b;
-	bool two_emails = false;
-
-	*found = false;
-	*real_name = NULL_RANGE;
-	*real_email = NULL_RANGE;
-	*replace_name = NULL_RANGE;
-	*replace_email = NULL_RANGE;
-
-	while (file->len > 0) {
-		/* Get the line, and remove any comments */
-		range_split(*file, '\n', &line, file);
-		range_split(line, '#', &line, &comment);
-
-		/* Skip blank lines */
-		range_trim(&line);
-		if (line.len == 0)
-			continue;
+	/* If this is the last email in the line, this is the email to replace,
+	 * otherwise, it's the real email. */
+	if (advance_until(&start, &len, ctx, '>') < 0)
+		return -1;
 
-		/* Get the first name and email */
-		if (!range_split(line, '<', &name_a, &line))
-			return -1; /* garbage in line */
-		if (!range_split(line, '>', &email_a, &line))
-			return -1; /* unfinished <> pair */
-
-		/* Get an optional second name and/or email */
-		two_emails = range_split(line, '<', &name_b, &line);
-		if (two_emails && !range_split(line, '>', &email_b, &line))
-			return -1; /* unfinished <> pair */
-
-		/* Trim whitespace from around names */
-		range_trim(&name_a);
-		range_trim(&name_b);
-
-		*found = true;
-		if (name_a.len > 0)
-			*real_name = name_a;
-
-		if (two_emails) {
-			if (email_a.len > 0)
-				*real_email = email_a;
-			*replace_email = email_b;
-
-			if (name_b.len > 0)
-				*replace_name = name_b;
-		} else {
-			*replace_email = email_a;
-		}
-		break;
+	/* If we aren't at the end of the line, parse a second name and email */
+	if (!is_eol(ctx)) {
+		git_buf_attach_notowned(real_email, start, len);
+
+		git_parse_advance_ws(ctx);
+		if (advance_until(&start, &len, ctx, '<') < 0)
+			return -1;
+		git_buf_attach_notowned(replace_name, start, len);
+		git_buf_rtrim(replace_name);
+
+		if (advance_until(&start, &len, ctx, '>') < 0)
+			return -1;
 	}
 
+	git_buf_attach_notowned(replace_email, start, len);
+
+	if (!is_eol(ctx))
+		return -1;
+
 	return 0;
 }
 
-int git_mailmap_parse(
-	git_mailmap **mailmap,
-	const char *data,
-	size_t size)
+int git_mailmap_from_buffer(git_mailmap **out, git_buf *buf)
 {
-	char_range file = { data, size };
-	git_mailmap_entry* entry = NULL;
-	int error = 0;
+	int error;
+	git_mailmap *mm;
+	size_t entry_size;
+	char *entry_data;
+	git_mailmap_entry *entry = NULL;
+	git_parse_ctx ctx;
+
+	/* Scratch buffers containing the real parsed names & emails */
+	git_buf real_name = GIT_BUF_INIT;
+	git_buf real_email = GIT_BUF_INIT;
+	git_buf replace_name = GIT_BUF_INIT;
+	git_buf replace_email = GIT_BUF_INIT;
+
+	if (git_buf_contains_nul(buf))
+		return -1;
 
-	if (memchr(data, '\0', size) != NULL)
-		return -1; /* data may not contain '\0's */
+	git_parse_ctx_init(&ctx, buf->ptr, buf->size);
 
-	*mailmap = git__calloc(1, sizeof(git_mailmap));
-	if (!*mailmap)
-		return -1;
+	/* Create our mailmap object */
+	mm = git__calloc(1, sizeof(git_mailmap));
+	GITERR_CHECK_ALLOC(mm);
 
-	/* XXX: Is it worth it to precompute the size? */
-	error = git_vector_init(&(*mailmap)->entries, 0, NULL);
+	error = git_vector_init(&mm->entries, 0, NULL);
 	if (error < 0)
 		goto cleanup;
 
-	while (file.len > 0) {
-		bool found = false;
-		char_range real_name, real_email, replace_name, replace_email;
-		size_t size = 0;
-		char *buf = NULL;
-
-		error = git_mailmap_parse_single(
-			&file, &found,
-			&real_name, &real_email,
-			&replace_name, &replace_email);
-		if (error < 0 || !found) {
-			error = 0;
+	/* Run the parser */
+	while (ctx.remain_len > 0) {
+		error = parse_mailmap_entry(
+			&real_name, &real_email, &replace_name, &replace_email, &ctx);
+		if (error < 0) {
+			error = 0; /* Skip lines which don't contain a valid entry */
+			git_parse_advance_line(&ctx);
 			continue;
 		}
 
-		/* Compute how much space we'll need to store our entry */
-		size = sizeof(git_mailmap_entry);
-		range_copyz(NULL, &size, real_name);
-		range_copyz(NULL, &size, real_email);
-		range_copyz(NULL, &size, replace_name);
-		range_copyz(NULL, &size, replace_email);
+		GITERR_CHECK_ALLOC_ADD5(
+			&entry_size, sizeof(git_mailmap_entry) + 4 /* 4x'\0' */,
+			real_name.size, real_email.size,
+			replace_name.size, replace_email.size);
+		entry = git__calloc(1, entry_size);
+		GITERR_CHECK_ALLOC(entry);
 
-		entry = git__malloc(size);
-		if (!entry) {
-			error = -1;
-			goto cleanup;
-		}
 		entry->version = GIT_MAILMAP_ENTRY_VERSION;
 
-		buf = (char*)(entry + 1);
-		entry->real_name = range_copyz(&buf, NULL, real_name);
-		entry->real_email = range_copyz(&buf, NULL, real_email);
-		entry->replace_name = range_copyz(&buf, NULL, replace_name);
-		entry->replace_email = range_copyz(&buf, NULL, replace_email);
-		assert(buf == ((char*)entry) + size);
+		/* Copy strings into the buffer following entry */
+		entry_data = (char *)(entry + 1);
+		if (real_name.size > 0) {
+			memcpy(entry_data, real_name.ptr, real_name.size);
+			entry->real_name = entry_data;
+			entry_data += real_name.size + 1; /* advance past null from calloc */
+		}
+		if (real_email.size > 0) {
+			memcpy(entry_data, real_email.ptr, real_email.size);
+			entry->real_email = entry_data;
+			entry_data += real_email.size + 1;
+		}
+		if (replace_name.size > 0) {
+			memcpy(entry_data, replace_name.ptr, replace_name.size);
+			entry->replace_name = entry_data;
+			entry_data += replace_name.size + 1;
+		}
+		/* replace_email is always non-null */
+		memcpy(entry_data, replace_email.ptr, replace_email.size);
+		entry->replace_email = entry_data;
 
-		error = git_vector_insert(&(*mailmap)->entries, entry);
+		error = git_vector_insert(&mm->entries, entry);
 		if (error < 0)
 			goto cleanup;
 		entry = NULL;
 	}
 
+	/* fill in *out, and make sure we don't free our mailmap */
+	*out = mm;
+	mm = NULL;
+
 cleanup:
 	git__free(entry);
-	if (error < 0) {
-		git_mailmap_free(*mailmap);
-		*mailmap = NULL;
-	}
+	git_mailmap_free(mm);
+
+	/* We never allocate data in these buffers, but better safe than sorry */
+	git_buf_free(&real_name);
+	git_buf_free(&real_email);
+	git_buf_free(&replace_name);
+	git_buf_free(&replace_email);
 	return error;
 }
 
@@ -232,11 +196,9 @@ void git_mailmap_free(git_mailmap *mailmap)
 }
 
 void git_mailmap_resolve(
-	const char **name_out,
-	const char **email_out,
+	const char **name_out, const char **email_out,
 	const git_mailmap *mailmap,
-	const char *name,
-	const char *email)
+	const char *name, const char *email)
 {
 	const git_mailmap_entry *entry = NULL;
 	assert(name && email);
@@ -257,9 +219,7 @@ void git_mailmap_resolve(
 }
 
 const git_mailmap_entry *git_mailmap_entry_lookup(
-	const git_mailmap *mailmap,
-	const char *name,
-	const char *email)
+	const git_mailmap *mailmap, const char *name, const char *email)
 {
 	size_t i;
 	git_mailmap_entry *entry;
@@ -293,15 +253,12 @@ size_t git_mailmap_entry_count(const git_mailmap *mailmap)
 	return 0;
 }
 
-static int git_mailmap_from_bare_repo(
-	git_mailmap **mailmap,
-	git_repository *repo)
+static int mailmap_from_bare_repo(git_mailmap **mailmap, git_repository *repo)
 {
 	git_reference *head = NULL;
 	git_object *tree = NULL;
 	git_blob *blob = NULL;
-	const char *content = NULL;
-	git_off_t size = 0;
+	git_buf content = GIT_BUF_INIT;
 	int error;
 
 	assert(git_repository_is_bare(repo));
@@ -316,31 +273,28 @@ static int git_mailmap_from_bare_repo(
 		goto cleanup;
 
 	error = git_object_lookup_bypath(
-		(git_object **) &blob,
-		tree,
-		".mailmap",
-		GIT_OBJ_BLOB);
+		(git_object **) &blob, tree, MAILMAP_FILE, GIT_OBJ_BLOB);
 	if (error < 0)
 		goto cleanup;
 
-	content = git_blob_rawcontent(blob);
-	size = git_blob_rawsize(blob);
+	error = git_blob_filtered_content(&content, blob, MAILMAP_FILE, false);
+	if (error < 0)
+		goto cleanup;
 
-	error = git_mailmap_parse(mailmap, content, size);
+	error = git_mailmap_from_buffer(mailmap, &content);
 	if (error < 0)
 		goto cleanup;
 
 cleanup:
-	git_reference_free(head);
-	git_object_free(tree);
+	git_buf_free(&content);
 	git_blob_free(blob);
+	git_object_free(tree);
+	git_reference_free(head);
 
 	return error;
 }
 
-static int git_mailmap_from_workdir_repo(
-	git_mailmap **mailmap,
-	git_repository *repo)
+static int mailmap_from_workdir_repo(git_mailmap **mailmap, git_repository *repo)
 {
 	git_buf path = GIT_BUF_INIT;
 	git_buf data = GIT_BUF_INIT;
@@ -349,7 +303,7 @@ static int git_mailmap_from_workdir_repo(
 	assert(!git_repository_is_bare(repo));
 
 	/* In non-bare repositories, .mailmap should be read from the workdir */
-	error = git_buf_joinpath(&path, git_repository_workdir(repo), ".mailmap");
+	error = git_buf_joinpath(&path, git_repository_workdir(repo), MAILMAP_FILE);
 	if (error < 0)
 		goto cleanup;
 
@@ -357,7 +311,7 @@ static int git_mailmap_from_workdir_repo(
 	if (error < 0)
 		goto cleanup;
 
-	error = git_mailmap_parse(mailmap, data.ptr, data.size);
+	error = git_mailmap_from_buffer(mailmap, &data);
 	if (error < 0)
 		goto cleanup;
 
@@ -375,7 +329,7 @@ int git_mailmap_from_repo(git_mailmap **mailmap, git_repository *repo)
 	*mailmap = NULL;
 
 	if (git_repository_is_bare(repo))
-		return git_mailmap_from_bare_repo(mailmap, repo);
+		return mailmap_from_bare_repo(mailmap, repo);
 	else
-		return git_mailmap_from_workdir_repo(mailmap, repo);
+		return mailmap_from_workdir_repo(mailmap, repo);
 }
diff --git a/src/signature.c b/src/signature.c
index a93ba1019..723f8527a 100644
--- a/src/signature.c
+++ b/src/signature.c
@@ -99,27 +99,8 @@ int git_signature_new(git_signature **sig_out, const char *name, const char *ema
 
 int git_signature_dup(git_signature **dest, const git_signature *source)
 {
-	git_signature *signature;
-
-	if (source == NULL)
-		return 0;
-
-	signature = git__calloc(1, sizeof(git_signature));
-	GITERR_CHECK_ALLOC(signature);
-
-	signature->name = git__strdup(source->name);
-	GITERR_CHECK_ALLOC(signature->name);
-
-	signature->email = git__strdup(source->email);
-	GITERR_CHECK_ALLOC(signature->email);
-
-	signature->when.time = source->when.time;
-	signature->when.offset = source->when.offset;
-	signature->when.sign = source->when.sign;
-
-	*dest = signature;
-
-	return 0;
+	/* If mailmap is NULL, this method just copies the signature */
+	return git_signature_with_mailmap(dest, source, NULL);
 }
 
 int git_signature_with_mailmap(
@@ -132,32 +113,26 @@ int git_signature_with_mailmap(
 	const char *email = NULL;
 
 	if (source == NULL)
-		goto on_error;
+		return 0;
 
 	git_mailmap_resolve(&name, &email, mailmap, source->name, source->email);
 
 	signature = git__calloc(1, sizeof(git_signature));
-	if (!signature)
-		goto on_error;
+	GITERR_CHECK_ALLOC(signature);
 
 	signature->name = git__strdup(name);
-	if (!signature->name)
-		goto on_error;
+	GITERR_CHECK_ALLOC(signature->name);
 
 	signature->email = git__strdup(email);
-	if (!signature->email)
-		goto on_error;
+	GITERR_CHECK_ALLOC(signature->email);
 
 	signature->when.time = source->when.time;
 	signature->when.offset = source->when.offset;
 	signature->when.sign = source->when.sign;
 
 	*dest = signature;
-	return 0;
 
-on_error:
-	git_signature_free(signature);
-	return -1;
+	return 0;
 }
 
 int git_signature__pdup(git_signature **dest, const git_signature *source, git_pool *pool)
diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c
index b50633a6c..8d1bbb731 100644
--- a/tests/clar_libgit2.c
+++ b/tests/clar_libgit2.c
@@ -227,8 +227,7 @@ git_repository *cl_git_sandbox_reopen(void)
 		_cl_repo = NULL;
 
 		cl_git_pass(git_repository_open(
-			&_cl_repo,
-			cl_fixture_basename(_cl_sandbox)));
+			&_cl_repo, cl_fixture_basename(_cl_sandbox)));
 	}
 
 	return _cl_repo;
diff --git a/tests/mailmap/basic.c b/tests/mailmap/basic.c
index 9b06f184d..508598440 100644
--- a/tests/mailmap/basic.c
+++ b/tests/mailmap/basic.c
@@ -15,7 +15,10 @@ const char TEST_MAILMAP[] =
 
 void test_mailmap_basic__initialize(void)
 {
-	cl_git_pass(git_mailmap_parse(&mailmap, TEST_MAILMAP, sizeof(TEST_MAILMAP) - 1));
+	git_buf buf = GIT_BUF_INIT;
+	git_buf_attach_notowned(&buf, TEST_MAILMAP, sizeof(TEST_MAILMAP) - 1);
+
+	cl_git_pass(git_mailmap_from_buffer(&mailmap, &buf));
 }
 
 void test_mailmap_basic__cleanup(void)
@@ -53,9 +56,7 @@ void test_mailmap_basic__lookup_not_found(void)
 void test_mailmap_basic__lookup(void)
 {
 	const git_mailmap_entry *entry = git_mailmap_entry_lookup(
-		mailmap,
-		"Typoed the name once",
-		"foo@baz.com");
+		mailmap, "Typoed the name once", "foo@baz.com");
 	cl_assert(entry);
 	cl_assert(!git__strcmp(entry->real_name, "Foo bar"));
 }
@@ -65,11 +66,7 @@ void test_mailmap_basic__empty_email_query(void)
 	const char *name;
 	const char *email;
 	git_mailmap_resolve(
-		&name,
-		&email,
-		mailmap,
-		"Author name",
-		"otheremail@foo.com");
+		&name, &email, mailmap, "Author name", "otheremail@foo.com");
 	cl_assert(!git__strcmp(name, "Author name"));
 	cl_assert(!git__strcmp(email, "email@foo.com"));
 }
@@ -79,20 +76,13 @@ void test_mailmap_basic__name_matching(void)
 	const char *name;
 	const char *email;
 	git_mailmap_resolve(
-		&name,
-		&email,
-		mailmap,
-		"Other Name",
-		"yetanotheremail@foo.com");
+		&name, &email, mailmap, "Other Name", "yetanotheremail@foo.com");
 	cl_assert(!git__strcmp(name, "Other Name"));
 	cl_assert(!git__strcmp(email, "email@foo.com"));
 
 	git_mailmap_resolve(
-		&name,
-		&email,
-		mailmap,
-		"Other Name That Doesn't Match",
-		"yetanotheremail@foo.com");
+		&name, &email, mailmap,
+		"Other Name That Doesn't Match", "yetanotheremail@foo.com");
 	cl_assert(!git__strcmp(name, "Other Name That Doesn't Match"));
 	cl_assert(!git__strcmp(email, "yetanotheremail@foo.com"));
 }
diff --git a/tests/mailmap/blame.c b/tests/mailmap/blame.c
index 45e9812a8..9ab085bd3 100644
--- a/tests/mailmap/blame.c
+++ b/tests/mailmap/blame.c
@@ -15,11 +15,8 @@ void test_mailmap_blame__initialize(void)
 
 void test_mailmap_blame__cleanup(void)
 {
-	cl_git_sandbox_cleanup();
-	g_repo = NULL;
-
 	git_blame_free(g_blame);
-	g_blame = NULL;
+	cl_git_sandbox_cleanup();
 }
 
 void test_mailmap_blame__hunks(void)
@@ -32,7 +29,7 @@ void test_mailmap_blame__hunks(void)
 
 	opts.flags |= GIT_BLAME_USE_MAILMAP;
 
-	cl_check_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
+	cl_git_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
 	if (!g_blame)
 		return;
 
@@ -54,7 +51,7 @@ void test_mailmap_blame__hunks_no_mailmap(void)
 
 	g_repo = cl_git_sandbox_init("mailmap");
 
-	cl_check_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
+	cl_git_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
 	if (!g_blame)
 		return;
 
diff --git a/tests/mailmap/parsing.c b/tests/mailmap/parsing.c
index a81f2f9bf..fecb882cd 100644
--- a/tests/mailmap/parsing.c
+++ b/tests/mailmap/parsing.c
@@ -14,18 +14,15 @@ void test_mailmap_parsing__initialize(void)
 
 void test_mailmap_parsing__cleanup(void)
 {
-	cl_git_sandbox_cleanup();
-	g_repo = NULL;
-
 	git_mailmap_free(g_mailmap);
-	g_mailmap = NULL;
+	cl_git_sandbox_cleanup();
 }
 
 static void check_mailmap_entries(
 	const git_mailmap *mailmap, const mailmap_entry *entries, size_t entries_size)
 {
 	const git_mailmap_entry *parsed = NULL;
-	size_t idx = 0;
+	size_t idx;
 
 	/* Check that the parsed entries match */
 	cl_assert_equal_sz(entries_size, git_mailmap_entry_count(mailmap));
@@ -43,7 +40,7 @@ static void check_mailmap_resolve(
 {
 	const char *resolved_name = NULL;
 	const char *resolved_email = NULL;
-	size_t idx = 0;
+	size_t idx;
 
 	/* Check that the resolver behaves correctly */
 	for (idx = 0; idx < resolved_size; ++idx) {
@@ -60,23 +57,17 @@ static void check_mailmap_resolve(
 
 void test_mailmap_parsing__string(void)
 {
-	cl_check_pass(git_mailmap_parse(
-		&g_mailmap,
-		string_mailmap,
-		strlen(string_mailmap)));
+	git_buf buf = GIT_BUF_INIT;
+	git_buf_attach_notowned(&buf, string_mailmap, strlen(string_mailmap));
+	cl_git_pass(git_mailmap_from_buffer(&g_mailmap, &buf));
 
 	/* We should have parsed all of the entries */
-	check_mailmap_entries(
-		g_mailmap,
-		entries, ARRAY_SIZE(entries));
+	check_mailmap_entries(g_mailmap, entries, ARRAY_SIZE(entries));
 
 	/* Check that resolving the entries works */
+	check_mailmap_resolve(g_mailmap, resolved, ARRAY_SIZE(resolved));
 	check_mailmap_resolve(
-		g_mailmap,
-		resolved, ARRAY_SIZE(resolved));
-	check_mailmap_resolve(
-		g_mailmap,
-		resolved_untracked, ARRAY_SIZE(resolved_untracked));
+		g_mailmap, resolved_untracked, ARRAY_SIZE(resolved_untracked));
 }
 
 void test_mailmap_parsing__fromrepo(void)
@@ -84,40 +75,30 @@ void test_mailmap_parsing__fromrepo(void)
 	g_repo = cl_git_sandbox_init("mailmap");
 	cl_check(!git_repository_is_bare(g_repo));
 
-	cl_check_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
+	cl_git_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
 
 	/* We should have parsed all of the entries */
-	check_mailmap_entries(
-		g_mailmap,
-		entries, ARRAY_SIZE(entries));
+	check_mailmap_entries(g_mailmap, entries, ARRAY_SIZE(entries));
 
 	/* Check that resolving the entries works */
+	check_mailmap_resolve(g_mailmap, resolved, ARRAY_SIZE(resolved));
 	check_mailmap_resolve(
-		g_mailmap,
-		resolved, ARRAY_SIZE(resolved));
-	check_mailmap_resolve(
-		g_mailmap,
-		resolved_untracked, ARRAY_SIZE(resolved_untracked));
+		g_mailmap, resolved_untracked, ARRAY_SIZE(resolved_untracked));
 }
 
 void test_mailmap_parsing__frombare(void)
 {
 	g_repo = cl_git_sandbox_init("mailmap/.gitted");
-	cl_check_pass(git_repository_set_bare(g_repo));
+	cl_git_pass(git_repository_set_bare(g_repo));
 	cl_check(git_repository_is_bare(g_repo));
 
-	cl_check_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
+	cl_git_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
 
 	/* We should have parsed all of the entries, except for the untracked one */
-	check_mailmap_entries(
-		g_mailmap,
-		entries, ARRAY_SIZE(entries) - 1);
+	check_mailmap_entries(g_mailmap, entries, ARRAY_SIZE(entries) - 1);
 
 	/* Check that resolving the entries works */
+	check_mailmap_resolve(g_mailmap, resolved, ARRAY_SIZE(resolved));
 	check_mailmap_resolve(
-		g_mailmap,
-		resolved, ARRAY_SIZE(resolved));
-	check_mailmap_resolve(
-		g_mailmap,
-		resolved_bare, ARRAY_SIZE(resolved_bare));
+		g_mailmap, resolved_bare, ARRAY_SIZE(resolved_bare));
 }