John Keeping <j...@keeping.me.uk> writes:
> On Sun, Feb 10, 2013 at 11:30:39AM -0800, Junio C Hamano wrote:
>> Is it correct to say that this essentially re-does 656197ad3805
>> (graph.c: infinite loop in git whatchanged --graph -m, 2009-07-25)
>> in a slightly different way, in that Michał's original fix also
>> protected against the case where graph->state is flipped to
>> GRAPH_PADDING by graph_next_line() that returns false, but with your
>> fixup, the code knows it never happens (i.e. when graph_next_line()
>> returns false, graph->state is always in the GRAPH_PADDING state),
>> and the only thing we need to be careful about is when graph->state
>> is already in the PADDING state upon entry to this function?
> Yes, although I wonder if we can end up in POST_MERGE or COLLAPSING
> state here as well. The check in the loop guards against that because
> those will eventually end up as PADDING.
> As far as I can see, this is okay because we have called
> graph_show_remainder() at the end of outputting a commit, even when we
> end up outputting the same (merge) commit more than once. But someone
> more familiar with the graph code might want to comment here.
More importantly, that kind of thought process needs to be
documented in the log message; that will help people to diagnose the
cause of the problem if they later find that this patch made an
incorrect assumption while simplifying the code.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html