{"thread":{"id":"20226","subject":"[BUG] Endless loop in git whatchanged --graph -m","startedAt":"2009-07-24T23:45:00Z","lastAt":"2009-07-29T16:32:59Z","messageCount":2,"participants":["Michał Kiedrowicz"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"118689","messageId":"20090725014500.66680dc3@gmail.com","threadId":"20226","inReplyTo":null,"subject":"[BUG] Endless loop in git whatchanged --graph -m","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-07-24T23:45:00Z","receivedAt":"2009-07-24T23:45:00Z","isPatch":false,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Steps to reproduce:\n\n\t(Simple merge of two branches)\n\n\t$ mkdir a && cd a && git init\n\n\t$ echo file1 > file1\n\t$ git add file1 && git commit -m initial\n\n\t$ echo file1 >> file1\n\t$ git commit -a -m \"commit on master\"\n\n\t$ git checkout -b experimental HEAD^\n\t$ echo file2 > file2\n\t$ git add file2 && git commit -m \"commit on experimental\"\n\n\t$ git checkout master && git merge experimental \n\n\t$ git whatchanged --graph -m\n\t\n\nHere is a simple patch, which solves this problem by removing constraint from\ngit_show_commit(). I'm not sure if this is the best solution. Maybe some upper\nlayer shouldn't call git_show_commit()?\n\nSigned-off-by: Michał Kiedrowicz \n---\n\ndiff --git a/Documentation/technical/api-history-graph.txt b/Documentation/technical/api-history-graph.txt\nindex d66e61b..32d961a 100644\n--- a/Documentation/technical/api-history-graph.txt\n+++ b/Documentation/technical/api-history-graph.txt\n@@ -36,11 +36,11 @@ The following utility functions are wrappers around `graph_next_line()` and\n They can all be called with a NULL graph argument, in which case no graph\n output will be printed.\n \n-* `graph_show_commit()` calls `graph_next_line()` until it returns non-zero.\n- This prints all graph lines up to, and including, the line containing this\n- commit. Output is printed to stdout. The last line printed does not contain\n- a terminating newline. This should not be called if the commit line has\n- already been printed, or it will loop forever.\n+* `graph_show_commit()` calls `graph_next_line()` and\n+ `graph_is_commit_finished()` until one of them return non-zero. This prints\n+ all graph lines up to, and including, the line containing this commit.\n+ Output is printed to stdout. The last line printed does not contain a\n+ terminating newline.\n \n * `graph_show_oneline()` calls `graph_next_line()` and prints the result to\n stdout. The line printed does not contain a terminating newline.\ndiff --git a/graph.c b/graph.c\nindex e466770..049cdbc 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -1198,7 +1198,7 @@ void graph_show_commit(struct git_graph *graph)\n if (!graph)\n return;\n \n- while (!shown_commit_line) {\n+ while (!shown_commit_line && !graph_is_commit_finished(graph)) {\n shown_commit_line = graph_next_line(graph, &msgbuf);\n fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);\n if (!shown_commit_line)\n"},{"id":"119077","messageId":"20090729183259.2552912f@gmail.com","threadId":"20226","inReplyTo":"20090725014500.66680dc3@gmail.com","subject":"Re: [BUG] Endless loop in git whatchanged --graph -m","fromName":"Michał Kiedrowicz","fromEmail":"michal.kiedrowicz@gmail.com","sentAt":"2009-07-29T16:32:59Z","receivedAt":"2009-07-29T16:32:59Z","isPatch":false,"sender":{"key":"michal.kiedrowicz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14072847?v=4"},"body":"Michał Kiedrowicz wrote:\n\n> Steps to reproduce:\n> \n> \t(Simple merge of two branches)\n> \n> \t$ mkdir a && cd a && git init\n> \n> \t$ echo file1 > file1\n> \t$ git add file1 && git commit -m initial\n> \n> \t$ echo file1 >> file1\n> \t$ git commit -a -m \"commit on master\"\n> \n> \t$ git checkout -b experimental HEAD^\n> \t$ echo file2 > file2\n> \t$ git add file2 && git commit -m \"commit on experimental\"\n> \n> \t$ git checkout master && git merge experimental \n> \n> \t$ git whatchanged --graph -m\n> \t\n> \n> Here is a simple patch, which solves this problem by removing constraint from\n> git_show_commit(). I'm not sure if this is the best solution. Maybe some upper\n> layer shouldn't call git_show_commit()?\n> \n> Signed-off-by: Michał Kiedrowicz \n> ---\n> \n> diff --git a/Documentation/technical/api-history-graph.txt b/Documentation/technical/api-history-graph.txt\n> index d66e61b..32d961a 100644\n> --- a/Documentation/technical/api-history-graph.txt\n> +++ b/Documentation/technical/api-history-graph.txt\n> @@ -36,11 +36,11 @@ The following utility functions are wrappers around `graph_next_line()` and\n> They can all be called with a NULL graph argument, in which case no graph\n> output will be printed.\n> \n> -* `graph_show_commit()` calls `graph_next_line()` until it returns non-zero.\n> - This prints all graph lines up to, and including, the line containing this\n> - commit. Output is printed to stdout. The last line printed does not contain\n> - a terminating newline. This should not be called if the commit line has\n> - already been printed, or it will loop forever.\n> +* `graph_show_commit()` calls `graph_next_line()` and\n> + `graph_is_commit_finished()` until one of them return non-zero. This prints\n> + all graph lines up to, and including, the line containing this commit.\n> + Output is printed to stdout. The last line printed does not contain a\n> + terminating newline.\n> \n> * `graph_show_oneline()` calls `graph_next_line()` and prints the result to\n> stdout. The line printed does not contain a terminating newline.\n> diff --git a/graph.c b/graph.c\n> index e466770..049cdbc 100644\n> --- a/graph.c\n> +++ b/graph.c\n> @@ -1198,7 +1198,7 @@ void graph_show_commit(struct git_graph *graph)\n> if (!graph)\n> return;\n> \n> - while (!shown_commit_line) {\n> + while (!shown_commit_line && !graph_is_commit_finished(graph)) {\n> shown_commit_line = graph_next_line(graph, &msgbuf);\n> fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);\n> if (!shown_commit_line)\n\nHi, has anyone looked at this?\n\nMichał Kiedrowicz\n"}]}