{"thread":{"id":"11684","subject":"[PATCH] git-fsck: report missing author/commit line in a commit as an error","startedAt":"2008-01-20T18:59:52Z","lastAt":"2008-01-21T07:00:49Z","messageCount":3,"participants":["Martin Koegler","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"66041","messageId":"12008555922208-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"11684","inReplyTo":null,"subject":"[PATCH] git-fsck: report missing author/commit line in a commit as an error","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-01-20T18:59:52Z","receivedAt":"2008-01-20T18:59:52Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Report a commit date (returned by parse_commit), if it is zero\nas an error (instead by a printf).\n\nA zero date means missing author/commiter line or a totally\ncorrupted commiter line.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n builtin-fsck.c |    7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-fsck.c b/builtin-fsck.c\nindex e4874f6..c4e91bf 100644\n--- a/builtin-fsck.c\n+++ b/builtin-fsck.c\n@@ -360,6 +360,10 @@ static int fsck_commit(struct commit *commit)\n \t\tfprintf(stderr, \"Checking commit %s\\n\",\n \t\t\tsha1_to_hex(commit->object.sha1));\n \n+\tif (!commit->date)\n+\t\treturn objerror(&commit->object, \"invalid author/commiter line in %s\",\n+\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\n \tif (memcmp(buffer, \"tree \", 5))\n \t\treturn objerror(&commit->object, \"invalid format - expected 'tree' line\");\n \tif (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\\n')\n@@ -378,9 +382,6 @@ static int fsck_commit(struct commit *commit)\n \t\treturn objerror(&commit->object, \"could not load commit's tree %s\", tree_sha1);\n \tif (!commit->parents && show_root)\n \t\tprintf(\"root %s\\n\", sha1_to_hex(commit->object.sha1));\n-\tif (!commit->date)\n-\t\tprintf(\"bad commit date in %s\\n\",\n-\t\t       sha1_to_hex(commit->object.sha1));\n \treturn 0;\n }\n \n-- \ngitgui.0.9.1.gdeb16\n"},{"id":"66043","messageId":"alpine.LSU.1.00.0801201918260.5731@racer.site","threadId":"11684","inReplyTo":"12008555922208-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] git-fsck: report missing author/commit line in a commit as an error","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T19:20:54Z","receivedAt":"2008-01-20T19:20:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jan 2008, Martin Koegler wrote:\n\n> +\tif (!commit->date)\n> +\t\treturn objerror(&commit->object, \"invalid author/commiter line in %s\",\n\ns/commiter/committer/\n\nIt makes me wonder, though, if that's correct.  AFAICT it is the committer \ndate, and the rest of the line _might_ be just fine.\n\nWhy not be less intrusive and just change the printf() into a return \nobjerror(), like the commit message suggests?\n\nCiao,\nDscho\n"},{"id":"66090","messageId":"20080121070049.GA18819@auto.tuwien.ac.at","threadId":"11684","inReplyTo":"alpine.LSU.1.00.0801201918260.5731@racer.site","subject":"Re: [PATCH] git-fsck: report missing author/commit line in a commit as an error","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-01-21T07:00:49Z","receivedAt":"2008-01-21T07:00:49Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Sun, Jan 20, 2008 at 07:20:54PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 20 Jan 2008, Martin Koegler wrote:\n> \n> > +\tif (!commit->date)\n> > +\t\treturn objerror(&commit->object, \"invalid author/commiter line in %s\",\n> \n> s/commiter/committer/\n\nThanks. Will change in next version.\n\n> It makes me wonder, though, if that's correct.  AFAICT it is the committer \n> date, and the rest of the line _might_ be just fine.\n>\n> Why not be less intrusive and just change the printf() into a return \n> objerror(), like the commit message suggests?\n\nparse_commit_date returns zero, if\n* the author line is missing\n* the author line is not terminated with \\n\n* the committer line is missing\n* the committer line does not contain >\n* the > is not followed by a number bigger than zero\n* the commiter line is not terminated with \\n\n\nSo, in my option the message \"bad commit date in %s\" is incorrect/misleading.\n\nfsck_commit could be further simplified:\n\n//  If tree is missing, parse_commit should return zero => fsck_commit would not be called \n-        if (memcmp(buffer, \"tree \", 5))\n-                return objerror(&commit->object, \"invalid format - expected 'tree' line\");\n//  If sha1 is missing/incorrect, parse_commit should return zero => fsck_commit would not be called \n-        if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\\n')\n-                return objerror(&commit->object, \"invalid 'tree' line format - bad sha1\");\n         buffer += 46;\n         while (!memcmp(buffer, \"parent \", 7)) {\n//  If sha1 is missing/incorrect, parse_commit should return zero => fsck_commit would not be called \n-                if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\\n')\n-                        return objerror(&commit->object, \"invalid 'parent' line format - bad sha1\");\n                buffer += 48;\n         }\n         if (memcmp(buffer, \"author \", 7))\n                 return objerror(&commit->object, \"invalid format - expected 'author' line\");\n\nI'm not sure, if this 6 lines should be dropped too.\n\nThe memcmp(buffer, \"author \", 7) check is already done by\n(!commit->date), but I prefer to keep it, as further committer/author\nline checks can be based on it.\n\nmfg Martin Kögler\nPS:\nWhile writting this mail, I noticed that parse_object_buffer does not\ncheck the return value of parse_XX_buffer. I'll send a patch, which\ncorrect this.\n"}]}