Re: [PATCH] general style: replaces memcmp() with proper starts_with()

[Jeff King <peff@peff.net>]

On Wed, Mar 12, 2014 at 05:14:15PM -0400, Jeff King wrote:

> One thing that bugs me about the current code is that the sub-function
> looks one past the end of the length given to it by the caller.
> Switching it to pass "eof - line + 1" resolves that, but is it right?
> 
> The character pointed at by "eof" is either:
> 
>   1. space, if our strchr(line, ' ') found something
> 
>   2. the first character of the next line, if our
>      memchr(line, '\n', eob - line) found something
> 
>   3. Whatever is at eob (end-of-buffer)

This misses a case. We might find no space at all, and eof is NULL. We
never dereference it, so we don't segfault, but it does cause a bogus
giant allocation.

I'm out of time for the day, but here is a test I started on that
demonstrates the failure:

diff --git a/t/t7513-commit-bogus-extra-headers.sh b/t/t7513-commit-bogus-extra-headers.sh
index e69de29..834db08 100755
--- a/t/t7513-commit-bogus-extra-headers.sh
+++ b/t/t7513-commit-bogus-extra-headers.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='check parsing of badly formed commit objects'
+. ./test-lib.sh
+
+EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+
+test_expect_success 'newline right after mergetag header' '
+	test_tick &&
+	cat >commit <<-EOF &&
+	tree $EMPTY_TREE
+	author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	mergetag
+
+	foo
+	EOF
+	commit=$(git hash-object -w -t commit commit) &&
+	cat >expect <<-EOF &&
+	todo...
+	EOF
+	git --no-pager show --show-signature $commit >actual &&
+	test_cmp expect actual
+'
+
+test_done

The "git show" fails with:

  fatal: Out of memory, malloc failed (tried to allocate 18446744073699764927 bytes)

So I think the whole function could use some refactoring to handle
corner cases better. I'll try to take a look tomorrow, but please feel
free if somebody else wants to take a crack at it.

-Peff