Commit 57cfeab9fe480591e3f56d61ccfbb68f9b500156

Nika Layzell 2018-03-26T15:05:37

mailmap: Switch mailmap parsing to use the git_parse module

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
diff --git a/include/git2/mailmap.h b/include/git2/mailmap.h
index 0c722b5..30cc9cf 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 0574132..b31d2dc 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 c0da98d..caac82c 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 a93ba10..723f852 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 b50633a..8d1bbb7 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 9b06f18..5085984 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 45e9812..9ab085b 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 a81f2f9..fecb882 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));
 }