git patch-id fails on long lines

In patch-id.c, get_one_patchid uses a fixed 1000-char buffer to read a line.[1]
This causes incorrect results on longer lines.  Pasted below is a git commit
(from git show) that demonstrates the problem.  The result of running git
patch-id on this commit is:

9220f380851be9cab1a760430e3be096dcbee8c6 9b96b6fde8f7df791a1490ae18e1fa75fbab3262
74b8ede07628a574fd586624e0c77a4b6c9967e0 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

The commit:

commit 9b96b6fde8f7df791a1490ae18e1fa75fbab3262
Author: Andrew Pimlott [off-list ref]
Date:   Tue Sep 20 10:53:25 2011 -0700

    2

diff --git a/a b/a
index e69de29..2e6adac 100644
--- a/a
+++ b/a

@@ -0,0 +1 @@
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

diff --git a/b b/b
index e69de29..1425fcc 100644
--- a/b
+++ b/b

@@ -0,0 +1 @@
+b

Andrew

[1] https://github.com/git/git/blob/master/builtin/patch-id.c

On Tue, Sep 20, 2011 at 11:07:42AM -0700, Andrew Pimlott wrote:

In patch-id.c, get_one_patchid uses a fixed 1000-char buffer to read a line.[1]
This causes incorrect results on longer lines.  Pasted below is a git commit
(from git show) that demonstrates the problem.  The result of running git
patch-id on this commit is:

9220f380851be9cab1a760430e3be096dcbee8c6 9b96b6fde8f7df791a1490ae18e1fa75fbab3262
74b8ede07628a574fd586624e0c77a4b6c9967e0 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

Hmm, yeah.  My initial impression "eh, so what, it will just add the
line contents to the sha1 patch-id in two separate hunks". But we
actually treat lines magically based on their beginnings, and it
seems we accidentally think the "aaa" is the start of the next commit
header.

I think this can be trivially converted to use strbuf_getwholeline,
something like this (completely untested):

diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index f821eb3..99ba2ca 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c

@@ -58,11 +58,12 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
 
 static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)
 {
-	static char line[1000];
+	static struct strbuf line_buf = STRBUF_INIT;
 	int patchlen = 0, found_next = 0;
 	int before = -1, after = -1;
 
-	while (fgets(line, sizeof(line), stdin) != NULL) {
+	while (strbuf_getwholeline(&line_buf, stdin, '\n') != EOF) {
+		char *line = line_buf.buf;
 		char *p = line;
 		int len;
 

That just reuses the same heap-allocated buffer over and over, and then
eventually leaks it at the end (but then the program exits anyway). You
could get fancier and actually pass in the strbuf from generate_id_list,
and then release it when we're done.

-Peff