Re: Hmm.. Try harder to find the common commits in git protocol?

[Junio C Hamano <gitster@pobox.com>]

Linus Torvalds <torvalds@linux-foundation.org> 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.