mailmap: handle mailmap blobs without trailing newlines

The read_mailmap_buf function reads each line of the mailmap using strchrnul, like: const char *end = strchrnul(buf, '\n'); unsigned long linelen = end - buf + 1; But that's off-by-one when we actually hit the NUL byte; our line does not have a terminator, and so is only "end - buf" bytes long. As a result, when we subtract the linelen from the total len, we end up with (unsigned long)-1 bytes left in the buffer, and we start reading random junk from memory. We could fix it with: unsigned long linelen = end - buf + !!*end; but let's take a step back for a moment. It's questionable in the first place for a function that takes a buffer and length to be using strchrnul. But it works because we only have one caller (and are only likely to ever have this one), which is handing us data from read_sha1_file. Which means that it's always NUL-terminated. Instead of tightening the assumptions to make the buffer/length pair work for a caller that doesn't actually exist, let's let loosen the assumptions to what the real caller has: a modifiable, NUL-terminated string. This makes the code simpler and shorter (because we don't have to correlate strchrnul with the length calculation), correct (because the code with the off-by-one just goes away), and more efficient (we can drop the extra allocation we needed to create NUL-terminated strings for each line, and just terminate in place). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 27, 2013 at 21:41 UTC f972a1658a30809db113a3c486b1fe95b56633bf
2 files changed +24 -13
mailmap.c
+9 -12
@@ -187,20 +187,17 @@ static int read_mailmap_file(struct string_list *map, const char *filename,
187 return 0;
188 }
189
190 -static void read_mailmap_buf(struct string_list *map,
191 - const char *buf, unsigned long len,
192 - char **repo_abbrev)
190 +static void read_mailmap_string(struct string_list *map, char *buf,
191 + char **repo_abbrev)
192 {
194 - while (len) {
195 - const char *end = strchrnul(buf, '\n');
196 - unsigned long linelen = end - buf + 1;
197 - char *line = xmemdupz(buf, linelen);
193 + while (*buf) {
194 + char *end = strchrnul(buf, '\n');
195
199 - read_mailmap_line(map, line, repo_abbrev);
196 + if (*end)
197 + *end++ = '\0';
198
201 - free(line);
202 - buf += linelen;
203 - len -= linelen;
199 + read_mailmap_line(map, buf, repo_abbrev);
200 + buf = end;
201 }
202 }
203
@@ -224,7 +221,7 @@ static int read_mailmap_blob(struct string_list *map,
221 if (type != OBJ_BLOB)
222 return error("mailmap is not a blob: %s", name);
223
227 - read_mailmap_buf(map, buf, size, repo_abbrev);
224 + read_mailmap_string(map, buf, repo_abbrev);
225
226 free(buf);
227 return 0;
t/t4203-mailmap.sh
+15 -1
@@ -159,7 +159,8 @@ test_expect_success 'setup mailmap blob tests' '
159 Blob Guy <author@example.com>
160 Blob Guy <bugs@company.xx>
161 EOF
162 - git add just-bugs both &&
162 + printf "Tricky Guy <author@example.com>" >no-newline &&
163 + git add just-bugs both no-newline &&
164 git commit -m "my mailmaps" &&
165 echo "Repo Guy <author@example.com>" >.mailmap &&
166 echo "Internal Guy <author@example.com>" >internal.map
@@ -243,6 +244,19 @@ test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '
244 )
245 '
246
247 +test_expect_success 'mailmap.blob can handle blobs without trailing newline' '
248 + cat >expect <<-\EOF &&
249 + Tricky Guy (1):
250 + initial
251 +
252 + nick1 (1):
253 + second
254 +
255 + EOF
256 + git -c mailmap.blob=map:no-newline shortlog HEAD >actual &&
257 + test_cmp expect actual
258 +'
259 +
260 test_expect_success 'cleanup after mailmap.blob tests' '
261 rm -f .mailmap
262 '