[PATCH] diagnose a corrupt MERGE_RR when hitting EOF between TAB and '\0'

Jim Meyering· May 26, 2011, 13:54 UTC · lore

I ran coverity against git and it warned about the truncation inherent in the "buf[i] = fgetc(in);" assignment below. With that hint, it's easy to see that failing fgetc (returning EOF) was not handled at all. The patch below fixes it.

You could probably accomplish the same thing on fewer lines by declaring "c" earlier and assigning to both c and buf[i] inside the for-stmt,

		for (i = 0; i < sizeof(buf) && (buf[i] = c = fgetc(in)); i++)
			if (c < 0)
				die("corrupt MERGE_RR");

But that doesn't placate this particular static analyzer, while the patch below does.

-- >8 --
Subject: [PATCH] diagnose a corrupt MERGE_RR when hitting EOF between TAB and
 '\0'

* rerere.c (read_rr): If we reach EOF after the SHA1-then-TAB,
yet before the NUL that terminates each file name, before we would
fill the file name buffer with \255 bytes resulting from the
repeatedly-failing fgetc (returns EOF/-1) and ultimately complain
about "filename too long", because no NUL was encountered.
---
 rerere.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

Show changes to rerere.c +8 −3

diff --git a/rerere.c b/rerere.c
index dee2cb1..cb8e5ba 100644
--- a/rerere.c
+++ b/rerere.c
@@ -47,8 +47,14 @@ static void read_rr(struct string_list *rr)
 		name = xstrdup(buf);
 		if (fgetc(in) != '\t')
 			die("corrupt MERGE_RR");
-		for (i = 0; i < sizeof(buf) && (buf[i] = fgetc(in)); i++)
-			; /* do nothing */
+		for (i = 0; i < sizeof(buf); i++) {
+			int c = fgetc(in);
+			if (c < 0)
+				die("corrupt MERGE_RR");
+			buf[i] = c;
+			if (c == 0)
+				 break;
+		}
 		if (i == sizeof(buf))
 			die("filename too long");
 		string_list_insert(rr, buf)->util = name;
--
1.7.5.2.660.g9f46c

patchdiagnose a corrupt MERGE_RR when hitting EOF between TAB and '\0'