On Sun, Jan 20, 2008 at 07:20:54PM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Sun, 20 Jan 2008, Martin Koegler wrote:
>
> > + if (!commit->date)
> > + return objerror(&commit->object, "invalid author/commiter line in %s",
>
> s/commiter/committer/
Thanks. Will change in next version.
> It makes me wonder, though, if that's correct. AFAICT it is the committer
> date, and the rest of the line _might_ be just fine.
>
> Why not be less intrusive and just change the printf() into a return
> objerror(), like the commit message suggests?
parse_commit_date returns zero, if
* the author line is missing
* the author line is not terminated with \n
* the committer line is missing
* the committer line does not contain >
* the > is not followed by a number bigger than zero
* the commiter line is not terminated with \n
So, in my option the message "bad commit date in %s" is incorrect/misleading.
fsck_commit could be further simplified:
// If tree is missing, parse_commit should return zero => fsck_commit would not be called
- if (memcmp(buffer, "tree ", 5))
- return objerror(&commit->object, "invalid format - expected 'tree' line");
// If sha1 is missing/incorrect, parse_commit should return zero => fsck_commit would not be called
- if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
- return objerror(&commit->object, "invalid 'tree' line format - bad sha1");
buffer += 46;
while (!memcmp(buffer, "parent ", 7)) {
// If sha1 is missing/incorrect, parse_commit should return zero => fsck_commit would not be called
- if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
- return objerror(&commit->object, "invalid 'parent' line format - bad sha1");
buffer += 48;
}
if (memcmp(buffer, "author ", 7))
return objerror(&commit->object, "invalid format - expected 'author' line");
I'm not sure, if this 6 lines should be dropped too.
The memcmp(buffer, "author ", 7) check is already done by
(!commit->date), but I prefer to keep it, as further committer/author
line checks can be based on it.
mfg Martin Kögler
PS:
While writting this mail, I noticed that parse_object_buffer does not
check the return value of parse_XX_buffer. I'll send a patch, which
correct this.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
