From 109025b4e1c836fb62752f69f24e8f11403760d5 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Mon, 15 Jul 2013 02:54:05 -0400 Subject: [PATCH 1/9] t4203: demonstrate loss of single-character name in mailmap entry A bug in mailmap.c:parse_name_and_email() causes it to overlook the single-character name in "A " and parse it only as "". Demonstrate this problem. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t4203-mailmap.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 842b7549e..27f8f86ea 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -247,6 +247,15 @@ test_expect_success 'cleanup after mailmap.blob tests' ' rm -f .mailmap ' +test_expect_failure 'single-character name' ' + echo " 1 A " >expect && + echo " 1 nick1 " >>expect && + echo "A " >.mailmap && + test_when_finished "rm .mailmap" && + git shortlog -es HEAD >actual && + test_cmp expect actual +' + # Extended mailmap configurations should give us the following output for shortlog cat >expect <<\EOF A U Thor (1): From 8c3811510e2a90f765edbb6dc7f81b0737592c0a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 15 Jul 2013 02:54:06 -0400 Subject: [PATCH 2/9] mailmap: do not lose single-letter names In parse_name_and_email() function, there is this line: *name = (nstart < nend ? nstart : NULL); When the function is given a buffer "A ", nstart scans from the beginning of the buffer, skipping whitespaces (there isn't any, so nstart points at the buffer), while nend starts from one byte before the first '<' and skips whitespaces backwards and stops at the first non-whitespace (i.e. it hits "A" at the beginning of the buffer). nstart == nend in this case for a single-letter name, and an off-by-one error makes it fail to pick up the name, which makes the entry equivalent to without the name. Signed-off-by: Junio C Hamano Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- mailmap.c | 2 +- t/t4203-mailmap.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mailmap.c b/mailmap.c index 2a7b36628..418081e61 100644 --- a/mailmap.c +++ b/mailmap.c @@ -122,7 +122,7 @@ static char *parse_name_and_email(char *buffer, char **name, while (nend > nstart && isspace(*nend)) --nend; - *name = (nstart < nend ? nstart : NULL); + *name = (nstart <= nend ? nstart : NULL); *email = left+1; *(nend+1) = '\0'; *right++ = '\0'; diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 27f8f86ea..858372437 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -247,7 +247,7 @@ test_expect_success 'cleanup after mailmap.blob tests' ' rm -f .mailmap ' -test_expect_failure 'single-character name' ' +test_expect_success 'single-character name' ' echo " 1 A " >expect && echo " 1 nick1 " >>expect && echo "A " >.mailmap && From 3aff56ddbebfdb314f123e9a076403459d6a0767 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Mon, 15 Jul 2013 02:54:07 -0400 Subject: [PATCH 3/9] t4203: demonstrate loss of uppercase characters in canonical email The email addresses read from .mailmap are downcased before being inserted into the mailmap data structure, which undesirably loses information. It is impossible, for instance, to map to . Demonstrate this problem. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t4203-mailmap.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 858372437..ffe6a11ac 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -256,6 +256,15 @@ test_expect_success 'single-character name' ' test_cmp expect actual ' +test_expect_failure 'preserve canonical email case' ' + echo " 1 A U Thor " >expect && + echo " 1 nick1 " >>expect && + echo " " >.mailmap && + test_when_finished "rm .mailmap" && + git shortlog -es HEAD >actual && + test_cmp expect actual +' + # Extended mailmap configurations should give us the following output for shortlog cat >expect <<\EOF A U Thor (1): From 97e751be7928d17c177b2fe65fb9bacf6ee35643 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 15 Jul 2013 02:54:08 -0400 Subject: [PATCH 4/9] mailmap: do not downcase mailmap entries The email addresses in the records read from the .mailmap file are downcased very early, and then used to match against e-mail addresses in the input. Because we do use case insensitive version of string list to manage these entries, there is no need to do this, and worse yet, downcasing the rewritten/canonical e-mail read from the .mailmap file loses information. Stop doing that, and also make the string list used to keep multiple names for an mailmap entry case insensitive (the code that uses the list, lookup_prefix(), expects a case insensitive match). Signed-off-by: Junio C Hamano Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- mailmap.c | 20 ++++++++------------ t/t4203-mailmap.sh | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/mailmap.c b/mailmap.c index 418081e61..a7e92db3d 100644 --- a/mailmap.c +++ b/mailmap.c @@ -51,14 +51,6 @@ static void add_mapping(struct string_list *map, { struct mailmap_entry *me; int index; - char *p; - - if (old_email) - for (p = old_email; *p; p++) - *p = tolower(*p); - if (new_email) - for (p = new_email; *p; p++) - *p = tolower(*p); if (old_email == NULL) { old_email = new_email; @@ -68,13 +60,17 @@ static void add_mapping(struct string_list *map, if ((index = string_list_find_insert_index(map, old_email, 1)) < 0) { /* mailmap entry exists, invert index value */ index = -1 - index; + me = (struct mailmap_entry *)map->items[index].util; } else { /* create mailmap entry */ - struct string_list_item *item = string_list_insert_at_index(map, index, old_email); - item->util = xcalloc(1, sizeof(struct mailmap_entry)); - ((struct mailmap_entry *)item->util)->namemap.strdup_strings = 1; + struct string_list_item *item; + + item = string_list_insert_at_index(map, index, old_email); + me = xcalloc(1, sizeof(struct mailmap_entry)); + me->namemap.strdup_strings = 1; + me->namemap.cmp = strcasecmp; + item->util = me; } - me = (struct mailmap_entry *)map->items[index].util; if (old_name == NULL) { debug_mm("mailmap: adding (simple) entry for %s at index %d\n", old_email, index); diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index ffe6a11ac..c32df80f1 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -256,7 +256,7 @@ test_expect_success 'single-character name' ' test_cmp expect actual ' -test_expect_failure 'preserve canonical email case' ' +test_expect_success 'preserve canonical email case' ' echo " 1 A U Thor " >expect && echo " 1 nick1 " >>expect && echo " " >.mailmap && From c10be0c6acfe671adf0ed1b3387de45d126f8ab7 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Mon, 15 Jul 2013 02:54:09 -0400 Subject: [PATCH 5/9] mailmap: debug: fix out-of-order fprintf() arguments Resolve segmentation fault due to arguments passed in wrong order. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- mailmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mailmap.c b/mailmap.c index a7e92db3d..051635472 100644 --- a/mailmap.c +++ b/mailmap.c @@ -309,7 +309,7 @@ int map_user(struct string_list *map, struct mailmap_entry *me; debug_mm("map_user: map '%.*s' <%.*s>\n", - *name, *namelen, *emaillen, *email); + *namelen, *name, *emaillen, *email); item = lookup_prefix(map, *email, *emaillen); if (item != NULL) { From 0939a242fe95fd2c2a2a4094d827e144382de3d7 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Mon, 15 Jul 2013 02:54:10 -0400 Subject: [PATCH 6/9] mailmap: debug: fix malformed fprintf() format conversion specification Resolve segmentation fault due to size_t variable being consumed by '%s'. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- mailmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mailmap.c b/mailmap.c index 051635472..62d998a37 100644 --- a/mailmap.c +++ b/mailmap.c @@ -337,7 +337,7 @@ int map_user(struct string_list *map, *name = mi->name; *namelen = strlen(*name); } - debug_mm("map_user: to '%.*s' <.*%s>\n", *namelen, *name, + debug_mm("map_user: to '%.*s' <%.*s>\n", *namelen, *name, *emaillen, *email); return 1; } From a8002a5f0ee19e2a513483e763b8b4a7cdcaa48d Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Mon, 15 Jul 2013 02:54:11 -0400 Subject: [PATCH 7/9] mailmap: debug: eliminate -Wformat field precision type warning The compiler complains that '*' in fprintf() format "%.*s" should have type int, but we pass size_t. Fix this. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- mailmap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mailmap.c b/mailmap.c index 62d998a37..4cc6e8128 100644 --- a/mailmap.c +++ b/mailmap.c @@ -309,7 +309,7 @@ int map_user(struct string_list *map, struct mailmap_entry *me; debug_mm("map_user: map '%.*s' <%.*s>\n", - *namelen, *name, *emaillen, *email); + (int)*namelen, *name, (int)*emaillen, *email); item = lookup_prefix(map, *email, *emaillen); if (item != NULL) { @@ -337,8 +337,8 @@ int map_user(struct string_list *map, *name = mi->name; *namelen = strlen(*name); } - debug_mm("map_user: to '%.*s' <%.*s>\n", *namelen, *name, - *emaillen, *email); + debug_mm("map_user: to '%.*s' <%.*s>\n", (int)*namelen, *name, + (int)*emaillen, *email); return 1; } debug_mm("map_user: --\n"); From fbfba7ade0dde36d8911973b1dac248b81ce1375 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Mon, 15 Jul 2013 02:54:12 -0400 Subject: [PATCH 8/9] mailmap: debug: avoid passing NULL to fprintf() '%s' conversion specification POSIX does not state the behavior of '%s' conversion when passed a NULL pointer. Some implementations interpolate literal "(null)"; others may crash. Callers of debug_mm() often pass NULL as indication of either a missing name or email address. Instead, let's always supply a proper string pointer, and make it a bit more descriptive: "(none)" Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- mailmap.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/mailmap.c b/mailmap.c index 4cc6e8128..928e6e5d9 100644 --- a/mailmap.c +++ b/mailmap.c @@ -5,8 +5,10 @@ #define DEBUG_MAILMAP 0 #if DEBUG_MAILMAP #define debug_mm(...) fprintf(stderr, __VA_ARGS__) +#define debug_str(X) ((X) ? (X) : "(none)") #else static inline void debug_mm(const char *format, ...) {} +static inline const char *debug_str(const char *s) { return s; } #endif const char *git_mailmap_file; @@ -29,7 +31,7 @@ struct mailmap_entry { static void free_mailmap_info(void *p, const char *s) { struct mailmap_info *mi = (struct mailmap_info *)p; - debug_mm("mailmap: -- complex: '%s' -> '%s' <%s>\n", s, mi->name, mi->email); + debug_mm("mailmap: -- complex: '%s' -> '%s' <%s>\n", s, debug_str(mi->name), debug_str(mi->email)); free(mi->name); free(mi->email); } @@ -38,7 +40,8 @@ static void free_mailmap_entry(void *p, const char *s) { struct mailmap_entry *me = (struct mailmap_entry *)p; debug_mm("mailmap: removing entries for <%s>, with %d sub-entries\n", s, me->namemap.nr); - debug_mm("mailmap: - simple: '%s' <%s>\n", me->name, me->email); + debug_mm("mailmap: - simple: '%s' <%s>\n", debug_str(me->name), debug_str(me->email)); + free(me->name); free(me->email); @@ -94,7 +97,7 @@ static void add_mapping(struct string_list *map, } debug_mm("mailmap: '%s' <%s> -> '%s' <%s>\n", - old_name, old_email, new_name, new_email); + debug_str(old_name), old_email, debug_str(new_name), debug_str(new_email)); } static char *parse_name_and_email(char *buffer, char **name, @@ -309,7 +312,7 @@ int map_user(struct string_list *map, struct mailmap_entry *me; debug_mm("map_user: map '%.*s' <%.*s>\n", - (int)*namelen, *name, (int)*emaillen, *email); + (int)*namelen, debug_str(*name), (int)*emaillen, debug_str(*email)); item = lookup_prefix(map, *email, *emaillen); if (item != NULL) { @@ -337,8 +340,8 @@ int map_user(struct string_list *map, *name = mi->name; *namelen = strlen(*name); } - debug_mm("map_user: to '%.*s' <%.*s>\n", (int)*namelen, *name, - (int)*emaillen, *email); + debug_mm("map_user: to '%.*s' <%.*s>\n", (int)*namelen, debug_str(*name), + (int)*emaillen, debug_str(*email)); return 1; } debug_mm("map_user: --\n"); From bd23794552a9d344ddda9fac20d7ee9b7607f6b1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 15 Jul 2013 02:54:13 -0400 Subject: [PATCH 9/9] mailmap: style fixes Wrap overlong lines and format the multi-line comments to match our coding style. Signed-off-by: Junio C Hamano Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- mailmap.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/mailmap.c b/mailmap.c index 928e6e5d9..44614fc41 100644 --- a/mailmap.c +++ b/mailmap.c @@ -31,7 +31,8 @@ struct mailmap_entry { static void free_mailmap_info(void *p, const char *s) { struct mailmap_info *mi = (struct mailmap_info *)p; - debug_mm("mailmap: -- complex: '%s' -> '%s' <%s>\n", s, debug_str(mi->name), debug_str(mi->email)); + debug_mm("mailmap: -- complex: '%s' -> '%s' <%s>\n", + s, debug_str(mi->name), debug_str(mi->email)); free(mi->name); free(mi->email); } @@ -39,8 +40,10 @@ static void free_mailmap_info(void *p, const char *s) static void free_mailmap_entry(void *p, const char *s) { struct mailmap_entry *me = (struct mailmap_entry *)p; - debug_mm("mailmap: removing entries for <%s>, with %d sub-entries\n", s, me->namemap.nr); - debug_mm("mailmap: - simple: '%s' <%s>\n", debug_str(me->name), debug_str(me->email)); + debug_mm("mailmap: removing entries for <%s>, with %d sub-entries\n", + s, me->namemap.nr); + debug_mm("mailmap: - simple: '%s' <%s>\n", + debug_str(me->name), debug_str(me->email)); free(me->name); free(me->email); @@ -50,7 +53,8 @@ static void free_mailmap_entry(void *p, const char *s) } static void add_mapping(struct string_list *map, - char *new_name, char *new_email, char *old_name, char *old_email) + char *new_name, char *new_email, + char *old_name, char *old_email) { struct mailmap_entry *me; int index; @@ -76,7 +80,8 @@ static void add_mapping(struct string_list *map, } if (old_name == NULL) { - debug_mm("mailmap: adding (simple) entry for %s at index %d\n", old_email, index); + debug_mm("mailmap: adding (simple) entry for %s at index %d\n", + old_email, index); /* Replace current name and new email for simple entry */ if (new_name) { free(me->name); @@ -88,7 +93,8 @@ static void add_mapping(struct string_list *map, } } else { struct mailmap_info *mi = xcalloc(1, sizeof(struct mailmap_info)); - debug_mm("mailmap: adding (complex) entry for %s at index %d\n", old_email, index); + debug_mm("mailmap: adding (complex) entry for %s at index %d\n", + old_email, index); if (new_name) mi->name = xstrdup(new_name); if (new_email) @@ -97,11 +103,12 @@ static void add_mapping(struct string_list *map, } debug_mm("mailmap: '%s' <%s> -> '%s' <%s>\n", - debug_str(old_name), old_email, debug_str(new_name), debug_str(new_email)); + debug_str(old_name), old_email, + debug_str(new_name), debug_str(new_email)); } static char *parse_name_and_email(char *buffer, char **name, - char **email, int allow_empty_email) + char **email, int allow_empty_email) { char *left, *right, *nstart, *nend; *name = *email = NULL; @@ -305,21 +312,25 @@ static struct string_list_item *lookup_prefix(struct string_list *map, } int map_user(struct string_list *map, - const char **email, size_t *emaillen, - const char **name, size_t *namelen) + const char **email, size_t *emaillen, + const char **name, size_t *namelen) { struct string_list_item *item; struct mailmap_entry *me; debug_mm("map_user: map '%.*s' <%.*s>\n", - (int)*namelen, debug_str(*name), (int)*emaillen, debug_str(*email)); + (int)*namelen, debug_str(*name), + (int)*emaillen, debug_str(*email)); item = lookup_prefix(map, *email, *emaillen); if (item != NULL) { me = (struct mailmap_entry *)item->util; if (me->namemap.nr) { - /* The item has multiple items, so we'll look up on name too */ - /* If the name is not found, we choose the simple entry */ + /* + * The item has multiple items, so we'll look up on + * name too. If the name is not found, we choose the + * simple entry. + */ struct string_list_item *subitem; subitem = lookup_prefix(&me->namemap, *name, *namelen); if (subitem) @@ -340,8 +351,9 @@ int map_user(struct string_list *map, *name = mi->name; *namelen = strlen(*name); } - debug_mm("map_user: to '%.*s' <%.*s>\n", (int)*namelen, debug_str(*name), - (int)*emaillen, debug_str(*email)); + debug_mm("map_user: to '%.*s' <%.*s>\n", + (int)*namelen, debug_str(*name), + (int)*emaillen, debug_str(*email)); return 1; } debug_mm("map_user: --\n");