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
