{"thread":{"id":"62951","subject":"[PATCH RFC] mailmap: fix check-mailmap with full mailmap line","startedAt":"2025-02-13T22:47:30Z","lastAt":"2025-02-15T00:24:45Z","messageCount":3,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"512390","messageId":"20250213-jk-fix-sendemail-mailinfo-v1-1-c0b06c215f21@gmail.com","threadId":"62951","inReplyTo":null,"subject":"[PATCH RFC] mailmap: fix check-mailmap with full mailmap line","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-02-13T22:47:22Z","receivedAt":"2025-02-13T22:47:30Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nI recently had reported to me a crash from a coworker using the recently\nadded sendemail mailmap support:\n\n 3724814 Segmentation fault (core dumped) git check-mailmap \"bugs@company.xx\"\n\nThis appears to happen because of the NULL pointer name passed into\nmap_user(). Fixing this, by assigning the name to be the empty string I\nstill saw somewhat unexpected results.\n\nWith a mailmap file containing:\n\nA B \n\nI get the following unexpected result:\n\n$ git check-mailmap b@domain.com\n\n\nBased on my interpretation of the mailmap documentation, I would have\nexpected this to translate to \"A \".\n\nI checked the map_user implementation, and noticed that it has a block\nto perform a name lookup with lookup_prefix on the subitems under the\nemail. This appears to fail when given the empty string, as it somehow\ndoesn't consider the empty string as a valid prefix for any of the\nnames. It then defaults to using the \"simple\" entry of the map.\n\nThis doesn't make sense, because \"full\" mailmap entries (those with both\nan email and an old name) don't store anything in the\nmailmap_entry->name and mailmap_entry->email. At least, not without a\nshortform entry filling that in.\n\nThis results in the item having a NULL name and email, which triggers\nthe early return and results in failure to update the return parameters.\n\nI tried my hand at fixing this with a new check to select the only entry\nin a mailmap entry with a single subitem. This fixed my test case, but\nresulted in a different test case failure, which I don't quite\nunderstand. The whitespace syntax test now seems to fail expectations by\ntranslating a previously untranslated name.\n\nI suspect this isn't the most elegant solution to the situation, and I\ndon't fully grasp the mailmap code, so help would be appreciated in\nfinding a good solution.\n\nSigned-off-by: Jacob Keller \n---\n builtin/check-mailmap.c | 2 +-\n mailmap.c | 9 ++++++++-\n t/t4203-mailmap.sh | 12 ++++++++++++\n 3 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c\nindex df00b5ee13adb87881b8c1e92cac256e6ad319d1..be2cebe12152e38d3bb8cf12948823c8d710bdda 100644\n--- a/builtin/check-mailmap.c\n+++ b/builtin/check-mailmap.c\n@@ -35,7 +35,7 @@ static void check_mailmap(struct string_list *mailmap, const char *contact)\n \t\tmail = ident.mail_begin;\n \t\tmaillen = ident.mail_end - ident.mail_begin;\n \t} else {\n-\t\tname = NULL;\n+\t\tname = \"\";\n \t\tnamelen = 0;\n \t\tmail = contact;\n \t\tmaillen = strlen(contact);\ndiff --git a/mailmap.c b/mailmap.c\nindex f35d20ed7fd1ef251e65419805fec49a3c30bcbb..7237ceef8a32a66183e4bbbe4ea01c4ea0264cd4 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -297,7 +297,7 @@ int map_user(struct string_list *map,\n \titem = lookup_prefix(map, *email, *emaillen);\n \tif (item) {\n \t\tme = (struct mailmap_entry *)item->util;\n-\t\tif (me->namemap.nr) {\n+\t\tif (me->namemap.nr > 1) {\n \t\t\t/*\n \t\t\t * The item has multiple items, so we'll look up on\n \t\t\t * name too. If the name is not found, we choose the\n@@ -307,6 +307,13 @@ int map_user(struct string_list *map,\n \t\t\tsubitem = lookup_prefix(&me->namemap, *name, *namelen);\n \t\t\tif (subitem)\n \t\t\t\titem = subitem;\n+\t\t} else if (me->name == NULL && me->email == NULL &&\n+\t\t\t me->namemap.nr == 1) {\n+\t\t\t/*\n+\t\t\t * The item has one subitem, but no simple entry. Use\n+\t\t\t * the full entry.\n+\t\t\t */\n+\t\t\titem = &me->namemap.items[0];\n \t\t}\n \t}\n \tif (item) {\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 24214919312777b76e4d3b2b784bcb953583750a..2aab735d2b326caf0d904cb6eee3d602fad96997 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -113,6 +113,18 @@ test_expect_success 'check-mailmap --stdin simple address: no mapping' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'check-mailmap name and address: mapping' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-EOF &&\n+\tBug Reports Bugs \n+\tEOF\n+\tcat >expect <<-EOF &&\n+\tBug Reports \n+\tEOF\n+\tgit check-mailmap \"bugs@company.xx\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'No mailmap' '\n \tcat >expect <<-EOF &&\n \t$GIT_AUTHOR_NAME (1):\n\n---\nbase-commit: 5ffbd7fcf84b313bb07e91246eb9419ebd94a7e7\nchange-id: 20250213-jk-fix-sendemail-mailinfo-32f027b1b9e7\n\nBest regards,\n-- \nJacob Keller \n\n"},{"id":"512391","messageId":"xmqqwmdtmmsl.fsf@gitster.g","threadId":"62951","inReplyTo":"20250213-jk-fix-sendemail-mailinfo-v1-1-c0b06c215f21@gmail.com","subject":"Re: [PATCH RFC] mailmap: fix check-mailmap with full mailmap line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-13T23:45:30Z","receivedAt":"2025-02-13T23:45:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> I recently had reported to me a crash from a coworker using the recently\n> added sendemail mailmap support:\n>\n> 3724814 Segmentation fault (core dumped) git check-mailmap \"bugs@company.xx\"\n\nThanks for relaying the report.\n \nI can easily reproduce your segfault with our own mailmap, by\npicking at random an entry with both name and e-mail listed as\nthe mapping source, e.g.\n\n $ git check-mailmap ksaitoh560@gmail.com\n\n> With a mailmap file containing:\n>\n> A B \n>\n> I get the following unexpected result:\n>\n> $ git check-mailmap b@domain.com\n> \n>\n> Based on my interpretation of the mailmap documentation, I would have\n> expected this to translate to \"A \".\n\nAfter reading \"git help mailmap\" twice, my interpretation is\ndifferent (disclaimer: I haven't read the implementation of the\nmailmap code lately, and the last time I read any part of it is\nprobably at least a few years ago if not before).\n\nUnlike \"please map anybody with this e-mail address to 'A '\"\nentry, which is spelled \"A \", the \"fully spelled\" form limits\nthe damage to those that match both name and e-mail, in order to\navoid \"D \" from getting modified, while rewriting \"B \" to \"A\n\". So I would not expect a request with no name to be mapped at\nall.\n\nAnd the command emits the e-mail intact when it does not find any\nmatch, \"b@domain.com\" being answered by \"\" is quite\nexpected from my point of view.\n\nThanks.\n"},{"id":"512455","messageId":"CA+P7+xr4EaTqSw0vqpJz17iF9gMFqhAt_6rvTsv+49mrWYntDw@mail.gmail.com","threadId":"62951","inReplyTo":"xmqqwmdtmmsl.fsf@gitster.g","subject":"Re: [PATCH RFC] mailmap: fix check-mailmap with full mailmap line","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2025-02-15T00:24:46Z","receivedAt":"2025-02-15T00:24:45Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Feb 13, 2025 at 3:45 PM Junio C Hamano wrote:\n>\n> Jacob Keller writes:\n>\n> > I recently had reported to me a crash from a coworker using the recently\n> > added sendemail mailmap support:\n> >\n> > 3724814 Segmentation fault (core dumped) git check-mailmap \"bugs@company.xx\"\n>\n> Thanks for relaying the report.\n>\n> I can easily reproduce your segfault with our own mailmap, by\n> picking at random an entry with both name and e-mail listed as\n> the mapping source, e.g.\n>\n> $ git check-mailmap ksaitoh560@gmail.com\n>\n> > With a mailmap file containing:\n> >\n> > A B \n> >\n> > I get the following unexpected result:\n> >\n> > $ git check-mailmap b@domain.com\n> > \n> >\n> > Based on my interpretation of the mailmap documentation, I would have\n> > expected this to translate to \"A \".\n>\n> After reading \"git help mailmap\" twice, my interpretation is\n> different (disclaimer: I haven't read the implementation of the\n> mailmap code lately, and the last time I read any part of it is\n> probably at least a few years ago if not before).\n>\n> Unlike \"please map anybody with this e-mail address to 'A '\"\n> entry, which is spelled \"A \", the \"fully spelled\" form limits\n> the damage to those that match both name and e-mail, in order to\n> avoid \"D \" from getting modified, while rewriting \"B \" to \"A\n> \". So I would not expect a request with no name to be mapped at\n> all.\n>\n> And the command emits the e-mail intact when it does not find any\n> match, \"b@domain.com\" being answered by \"\" is quite\n> expected from my point of view.\n>\n\nRe-reading the manual, that is a fair interpretation. I can share that\nwith my coworker and he can adapt his mailmap to match this\nexpectation I think.\n\nIn that case, I think the simple fix is to just replace the NULL with\na \"\" to resolve the segmentation fault and add a suitable test case\nfor that?\n\nThanks,\nJake\n\n> Thanks.\n"}]}