| Message ID | 20250221-jk-fix-sendemail-mailinfo-v2-1-9aca7dc05dbb@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mailmap: fix check-mailmap with full mailmap line | expand |
Jacob Keller <jacob.e.keller@intel.com> writes: > From: Jacob Keller <jacob.keller@gmail.com> > > I recently had reported to me a crash from a coworker using the recently > added sendemail mailmap support: > > 3724814 Segmentation fault (core dumped) git check-mailmap "bugs@company.xx" > > This appears to happen because of the NULL pointer name passed into > map_user(). Fix this by passing "" instead of NULL so that we have a > valid pointer. > > Signed-off-by: Jacob Keller <jacob.keller@gmail.com> > --- > Changes in v2: > - Just fix the NULL pointer dereference, leave mailmap resolution as-is > - Link to v1: https://lore.kernel.org/r/20250213-jk-fix-sendemail-mailinfo-v1-1-c0b06c215f21@gmail.com > --- > builtin/check-mailmap.c | 2 +- > t/t4203-mailmap.sh | 12 ++++++++++++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c > index df00b5ee13adb87881b8c1e92cac256e6ad319d1..be2cebe12152e38d3bb8cf12948823c8d710bdda 100644 > --- a/builtin/check-mailmap.c > +++ b/builtin/check-mailmap.c > @@ -35,7 +35,7 @@ static void check_mailmap(struct string_list *mailmap, const char *contact) > mail = ident.mail_begin; > maillen = ident.mail_end - ident.mail_begin; > } else { > - name = NULL; > + name = ""; > namelen = 0; > mail = contact; > maillen = strlen(contact); OK. I audited all users of map_user(), and everybody else (including the other side of the if/else we see here) follows the pattern to point name and mail into an "ident" instance, which would never be NULL. The way an "ident" instance signals that there is no name is to have the name_begin and name_end members point at the same byte. So this change obviously is a good thing to do. The callee, map_user() does assume both email and name pointers point at valid strings, passes them to lookup_prefix(), and have them receive what a mailmap_info instance records. We may want to document the calling convention a bit better, but that is obviously outside the scope of this immediate fix. Thanks. Will queue. > diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh > index 24214919312777b76e4d3b2b784bcb953583750a..4a6242ff99b59ea1a46eb14ca812c94e5e620162 100755 > --- a/t/t4203-mailmap.sh > +++ b/t/t4203-mailmap.sh > @@ -113,6 +113,18 @@ test_expect_success 'check-mailmap --stdin simple address: no mapping' ' > test_cmp expect actual > ' > > +test_expect_success 'check-mailmap name and address: mapping' ' > + test_when_finished "rm .mailmap" && > + cat >.mailmap <<-EOF && > + Bug Reports <bugs-new@company.xx> Bugs <bugs@company.xx> > + EOF > + cat >expect <<-EOF && > + <bugs@company.xx> > + EOF > + git check-mailmap "bugs@company.xx" >actual && > + test_cmp expect actual > +' > + > test_expect_success 'No mailmap' ' > cat >expect <<-EOF && > $GIT_AUTHOR_NAME (1): > > --- > base-commit: b838bf1938926a7a900166136d995d86f8a00e24 > change-id: 20250213-jk-fix-sendemail-mailinfo-32f027b1b9e7 > > Best regards,
diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c index df00b5ee13adb87881b8c1e92cac256e6ad319d1..be2cebe12152e38d3bb8cf12948823c8d710bdda 100644 --- a/builtin/check-mailmap.c +++ b/builtin/check-mailmap.c @@ -35,7 +35,7 @@ static void check_mailmap(struct string_list *mailmap, const char *contact) mail = ident.mail_begin; maillen = ident.mail_end - ident.mail_begin; } else { - name = NULL; + name = ""; namelen = 0; mail = contact; maillen = strlen(contact); diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 24214919312777b76e4d3b2b784bcb953583750a..4a6242ff99b59ea1a46eb14ca812c94e5e620162 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -113,6 +113,18 @@ test_expect_success 'check-mailmap --stdin simple address: no mapping' ' test_cmp expect actual ' +test_expect_success 'check-mailmap name and address: mapping' ' + test_when_finished "rm .mailmap" && + cat >.mailmap <<-EOF && + Bug Reports <bugs-new@company.xx> Bugs <bugs@company.xx> + EOF + cat >expect <<-EOF && + <bugs@company.xx> + EOF + git check-mailmap "bugs@company.xx" >actual && + test_cmp expect actual +' + test_expect_success 'No mailmap' ' cat >expect <<-EOF && $GIT_AUTHOR_NAME (1):