Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> Hmm. Trying to debug this, I find the behavior hard to reproduce. But I
> *also* find that there seems to be something wrong in builtin-fetch-pack.
>
> Look at commit f3ec549481827b10609a43bf504517a0e8063a12 ("fetch-pack:
> check parse_commit/object results"), and tell me that the "parents"
> handling isn't totally broken. In particular, get_rev() does:
>
> struct commit_list *parents = NULL;
> ..
> commit = rev_list->item;
> if (!(commit->object.parsed))
> if (!parse_commit(commit))
> parents = commit->parents;
> ..
>
> which means that "parents" will never even get set if the commit was
> already parsed!
>
> And whether it got parsed or not depends on how we got there etc, so this
> may explain the occasionally odd behaviour I saw.
>
> Basically, I don't think that code can be right,...
> This patch should fix that problem, but I wonder why it got rewritten that
> way in the first place?
Sorry, my fault while reviewing the series. The intent of the update was
too focused on not touching possibly bad data (i.e. when parse_commit()
does not return zero to indicate success) and failed to make sure that it
acts identically on good data --- very bad.
--
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
