Re: [PATCH] graph: avoid infinite loop in graph_show_commit()

Nguyen Thai Ngoc Duy [off-list ref] writes:

On Sun, Sep 23, 2012 at 6:55 PM, Michal Kiedrowicz
[off-list ref] wrote:

quoted

Nguyễn Thái Ngọc Duy <pclouds <at> gmail.com> writes:

quoted

The loop can be triggered with "git diff-tree --graph commit" where
the commit is a non-merge. It goes like this

Isn't this the same issue as in
http://article.gmane.org/gmane.comp.version-control.git/123979
? (with slightly different fix)

I don't know. I'm not familiar enough with graph.c to tell. Maybe Adam
can have a look?

Has either of you tried the patch with the problematic case the
other patch tries to solve?  Michal's old patch does smell like it
is going in the better direction in that it stops looping when we
know we would only be showing the padding, which is a sign that we
are done with showing the commit.

But I didn't look at it too closely.  I'd prefer to see the
assert(0) turned into die("BUG: internal error") at the end of
graph_next_line() to catch these cases.  Also I am not sure if
assignment of the return value from graph_next_line() to
shown_comit_line in the loop is correct (shouldn't it be OR'ing it
in, so that "we have shown the information on this commit" is not
lost when the function adds things after showing the commit???)