{"thread":{"id":"27464","subject":"[PATCH] diagnose a corrupt MERGE_RR when hitting EOF between TAB and '\\0'","startedAt":"2011-05-26T13:54:18Z","lastAt":"2011-05-26T13:54:18Z","messageCount":1,"participants":["Jim Meyering"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"168762","messageId":"87boyp7ek5.fsf@rho.meyering.net","threadId":"27464","inReplyTo":null,"subject":"[PATCH] diagnose a corrupt MERGE_RR when hitting EOF between TAB and '\\0'","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2011-05-26T13:54:18Z","receivedAt":"2011-05-26T13:54:18Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"I ran coverity against git and it warned about the truncation\ninherent in the \"buf[i] = fgetc(in);\" assignment below.\nWith that hint, it's easy to see that failing fgetc (returning EOF)\nwas not handled at all.  The patch below fixes it.\n\nYou could probably accomplish the same thing on fewer lines\nby declaring \"c\" earlier and assigning to both c and buf[i]\ninside the for-stmt,\n\n\t\tfor (i = 0; i < sizeof(buf) && (buf[i] = c = fgetc(in)); i++)\n\t\t\tif (c < 0)\n\t\t\t\tdie(\"corrupt MERGE_RR\");\n\nBut that doesn't placate this particular static analyzer, while\nthe patch below does.\n\n-- >8 --\nSubject: [PATCH] diagnose a corrupt MERGE_RR when hitting EOF between TAB and\n '\\0'\n\n* rerere.c (read_rr): If we reach EOF after the SHA1-then-TAB,\nyet before the NUL that terminates each file name, before we would\nfill the file name buffer with \\255 bytes resulting from the\nrepeatedly-failing fgetc (returns EOF/-1) and ultimately complain\nabout \"filename too long\", because no NUL was encountered.\n---\n rerere.c |   10 ++++++++--\n 1 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex dee2cb1..cb8e5ba 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -47,8 +47,14 @@ static void read_rr(struct string_list *rr)\n \t\tname = xstrdup(buf);\n \t\tif (fgetc(in) != '\\t')\n \t\t\tdie(\"corrupt MERGE_RR\");\n-\t\tfor (i = 0; i < sizeof(buf) && (buf[i] = fgetc(in)); i++)\n-\t\t\t; /* do nothing */\n+\t\tfor (i = 0; i < sizeof(buf); i++) {\n+\t\t\tint c = fgetc(in);\n+\t\t\tif (c < 0)\n+\t\t\t\tdie(\"corrupt MERGE_RR\");\n+\t\t\tbuf[i] = c;\n+\t\t\tif (c == 0)\n+\t\t\t\t break;\n+\t\t}\n \t\tif (i == sizeof(buf))\n \t\t\tdie(\"filename too long\");\n \t\tstring_list_insert(rr, buf)->util = name;\n--\n1.7.5.2.660.g9f46c\n"}]}