{"thread":{"id":"37859","subject":"[PATCH] line-log: fix crash when --first-parent is used","startedAt":"2014-10-31T19:43:56Z","lastAt":"2014-11-04T21:41:54Z","messageCount":7,"participants":["Tzvetan Mikov","Junio C Hamano","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"251270","messageId":"1414784636-43155-1-git-send-email-tmikov@gmail.com","threadId":"37859","inReplyTo":null,"subject":"[PATCH] line-log: fix crash when --first-parent is used","fromName":"Tzvetan Mikov","fromEmail":"tmikov@gmail.com","sentAt":"2014-10-31T19:43:56Z","receivedAt":"2014-10-31T19:43:56Z","isPatch":true,"sender":{"key":"tmikov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7785196?v=4"},"body":"line-log tries to access all parents of a commit, but only the first\nparent has been loaded if \"--first-parent\" is specified, resulting\nin a crash.\n\nLimit the number of parents to one if \"--first-parent\" is specified.\n\nReported-by: Eric N. Vander Weele \nSigned-off-by: Tzvetan Mikov \n---\n line-log.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/line-log.c b/line-log.c\nindex 1008e72..86e7274 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1141,6 +1141,9 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm\n \tint i;\n \tint nparents = commit_list_count(commit->parents);\n \n+\tif (nparents > 1 && rev->first_parent_only)\n+\t nparents = 1;\n+\n \tdiffqueues = xmalloc(nparents * sizeof(*diffqueues));\n \tcand = xmalloc(nparents * sizeof(*cand));\n \tparents = xmalloc(nparents * sizeof(*parents));\n-- \n1.9.1\n"},{"id":"251359","messageId":"xmqqoasoq9vh.fsf@gitster.dls.corp.google.com","threadId":"37859","inReplyTo":"1414784636-43155-1-git-send-email-tmikov@gmail.com","subject":"Re: [PATCH] line-log: fix crash when --first-parent is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-03T20:58:42Z","receivedAt":"2014-11-03T20:58:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tzvetan Mikov writes:\n\n> line-log tries to access all parents of a commit, but only the first\n> parent has been loaded if \"--first-parent\" is specified, resulting\n> in a crash.\n>\n> Limit the number of parents to one if \"--first-parent\" is specified.\n>\n> Reported-by: Eric N. Vander Weele \n> Signed-off-by: Tzvetan Mikov \n> ---\n\nThomas, how does this one look?\n\nTzvetan, can we have a test for this one?\n\nThanks.\n\n> line-log.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/line-log.c b/line-log.c\n> index 1008e72..86e7274 100644\n> --- a/line-log.c\n> +++ b/line-log.c\n> @@ -1141,6 +1141,9 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm\n> \tint i;\n> \tint nparents = commit_list_count(commit->parents);\n> \n> +\tif (nparents > 1 && rev->first_parent_only)\n> +\t nparents = 1;\n> +\n> \tdiffqueues = xmalloc(nparents * sizeof(*diffqueues));\n> \tcand = xmalloc(nparents * sizeof(*cand));\n> \tparents = xmalloc(nparents * sizeof(*parents));\n"},{"id":"251366","messageId":"CACt9tMj2GBWC6yRpfKXUk1hnh2ZyBqoV5HDmL0cjfxCiawHT=g@mail.gmail.com","threadId":"37859","inReplyTo":"xmqqoasoq9vh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] line-log: fix crash when --first-parent is used","fromName":"Tzvetan Mikov","fromEmail":"tmikov@gmail.com","sentAt":"2014-11-03T22:09:35Z","receivedAt":"2014-11-03T22:09:35Z","isPatch":true,"sender":{"key":"tmikov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7785196?v=4"},"body":"On Mon, Nov 3, 2014 at 12:58 PM, Junio C Hamano wrote:\n\n>> line-log tries to access all parents of a commit, but only the first\n>> parent has been loaded if \"--first-parent\" is specified, resulting\n>> in a crash.\n>>\n>> Limit the number of parents to one if \"--first-parent\" is specified.\n>>\n\n> [...]\n> Tzvetan, can we have a test for this one?\n\nHere is a sequence of commands to reproduce the crash:\n\ngit init\necho \"1\" > a.txt && git add a.txt && git commit -m \"a\"\ngit checkout -b branch\necho \"2\" > b.txt && git add b.txt && git commit -m \"b\"\ngit checkout master\ngit merge branch --no-ff -m \"merge\"\ngit log --first-parent -L 1,1:a.txt\n\nI am not sure whether you have a requirement for a formal test\nincluded with the patch, but if there is one, I am happy to\nrework it.\n\nThis is a very simple fix for the crash, though possibly\na \"real\" comprehensive fix may be needed. I am not sure.\nThe real cause is that with \"--first-parent\" some commit\nobjects are never loaded from disk but may still be accessed.\n\n(sorry about my formatting - have to use GMail)\n\nregards,\nTzvetan\n"},{"id":"251371","messageId":"54589DBB.7040900@drmicha.warpmail.net","threadId":"37859","inReplyTo":"CACt9tMj2GBWC6yRpfKXUk1hnh2ZyBqoV5HDmL0cjfxCiawHT=g@mail.gmail.com","subject":"Re: [PATCH] line-log: fix crash when --first-parent is used","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-11-04T09:34:51Z","receivedAt":"2014-11-04T09:34:51Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Tzvetan Mikov schrieb am 03.11.2014 um 23:09:\n> On Mon, Nov 3, 2014 at 12:58 PM, Junio C Hamano wrote:\n> \n>>> line-log tries to access all parents of a commit, but only the first\n>>> parent has been loaded if \"--first-parent\" is specified, resulting\n>>> in a crash.\n>>>\n>>> Limit the number of parents to one if \"--first-parent\" is specified.\n>>>\n> \n>> [...]\n>> Tzvetan, can we have a test for this one?\n> \n> Here is a sequence of commands to reproduce the crash:\n> \n> git init\n> echo \"1\" > a.txt && git add a.txt && git commit -m \"a\"\n> git checkout -b branch\n> echo \"2\" > b.txt && git add b.txt && git commit -m \"b\"\n> git checkout master\n> git merge branch --no-ff -m \"merge\"\n> git log --first-parent -L 1,1:a.txt\n> \n> I am not sure whether you have a requirement for a formal test\n> included with the patch, but if there is one, I am happy to\n> rework it.\n> \n> This is a very simple fix for the crash, though possibly\n> a \"real\" comprehensive fix may be needed. I am not sure.\n> The real cause is that with \"--first-parent\" some commit\n> objects are never loaded from disk but may still be accessed.\n> \n> (sorry about my formatting - have to use GMail)\n> \n> regards,\n> Tzvetan\n> \n\nA test would be nice, yes. Look in t/t4211-line-log.sh - you should be\nOK if you add a test at the end which does the following:\n\n\"git checkout parallel-change\" so that you have commits with more than 1\nparent in the history\n\ntest \"git log -L1,1 b.c\" (it core dumps with current git)\n\nThat is, you don't have to set up merges and history, you can use the\nexisting one.\n\nCheers,\nMichael\n"},{"id":"251383","messageId":"1415133217-7824-1-git-send-email-tmikov@gmail.com","threadId":"37859","inReplyTo":"1414784636-43155-1-git-send-email-tmikov@gmail.com","subject":"[PATCH v2] line-log: fix crash when --first-parent is used","fromName":"Tzvetan Mikov","fromEmail":"tmikov@gmail.com","sentAt":"2014-11-04T20:33:37Z","receivedAt":"2014-11-04T20:33:37Z","isPatch":true,"sender":{"key":"tmikov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7785196?v=4"},"body":"line-log tries to access all parents of a commit, but only the first\nparent has been loaded if \"--first-parent\" is specified, resulting\nin a crash.\n\nLimit the number of parents to one if \"--first-parent\" is specified.\n\nReported-by: Eric N. Vander Weele \nSigned-off-by: Tzvetan Mikov \n---\n PATCH v2: Add a test case (thanks Michael J Gruber)\n\n line-log.c | 3 +++\n t/t4211-line-log.sh | 5 +++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/line-log.c b/line-log.c\nindex 1008e72..86e7274 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1141,6 +1141,9 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm\n \tint i;\n \tint nparents = commit_list_count(commit->parents);\n \n+\tif (nparents > 1 && rev->first_parent_only)\n+\t nparents = 1;\n+\n \tdiffqueues = xmalloc(nparents * sizeof(*diffqueues));\n \tcand = xmalloc(nparents * sizeof(*cand));\n \tparents = xmalloc(nparents * sizeof(*parents));\ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex 7369d3c..0901b30 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -94,4 +94,9 @@ test_expect_success '-L ,Y (Y == nlines + 2)' '\n \ttest_must_fail git log -L ,$n:b.c\n '\n \n+test_expect_success '-L with --first-parent and a merge' '\n+\tgit checkout parallel-change &&\n+\tgit log --first-parent -L 1,1:b.c\n+'\n+\n test_done\n-- \n1.9.1\n"},{"id":"251387","messageId":"xmqq8ujqpsk6.fsf@gitster.dls.corp.google.com","threadId":"37859","inReplyTo":"1415133217-7824-1-git-send-email-tmikov@gmail.com","subject":"Re: [PATCH v2] line-log: fix crash when --first-parent is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-04T21:24:57Z","receivedAt":"2014-11-04T21:24:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tzvetan Mikov writes:\n\n> line-log tries to access all parents of a commit, but only the first\n> parent has been loaded if \"--first-parent\" is specified, resulting\n> in a crash.\n>\n> Limit the number of parents to one if \"--first-parent\" is specified.\n>\n> Reported-by: Eric N. Vander Weele \n> Signed-off-by: Tzvetan Mikov \n> ---\n> PATCH v2: Add a test case (thanks Michael J Gruber)\n\nThanks, both. The patch looks good (modulo the indentation of\nnparents assignment, which I'll locally fix up).\n\nWill queue.\n\n> line-log.c | 3 +++\n> t/t4211-line-log.sh | 5 +++++\n> 2 files changed, 8 insertions(+)\n>\n> diff --git a/line-log.c b/line-log.c\n> index 1008e72..86e7274 100644\n> --- a/line-log.c\n> +++ b/line-log.c\n> @@ -1141,6 +1141,9 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm\n> \tint i;\n> \tint nparents = commit_list_count(commit->parents);\n> \n> +\tif (nparents > 1 && rev->first_parent_only)\n> +\t nparents = 1;\n> +\n> \tdiffqueues = xmalloc(nparents * sizeof(*diffqueues));\n> \tcand = xmalloc(nparents * sizeof(*cand));\n> \tparents = xmalloc(nparents * sizeof(*parents));\n> diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\n> index 7369d3c..0901b30 100755\n> --- a/t/t4211-line-log.sh\n> +++ b/t/t4211-line-log.sh\n> @@ -94,4 +94,9 @@ test_expect_success '-L ,Y (Y == nlines + 2)' '\n> \ttest_must_fail git log -L ,$n:b.c\n> '\n> \n> +test_expect_success '-L with --first-parent and a merge' '\n> +\tgit checkout parallel-change &&\n> +\tgit log --first-parent -L 1,1:b.c\n> +'\n> +\n> test_done\n"},{"id":"251388","messageId":"CACt9tMi24QyvdpCk-tmU5U8jbP8eYhaNm4zWu6PqcKsJQ=OsYw@mail.gmail.com","threadId":"37859","inReplyTo":"xmqq8ujqpsk6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] line-log: fix crash when --first-parent is used","fromName":"Tzvetan Mikov","fromEmail":"tmikov@gmail.com","sentAt":"2014-11-04T21:41:54Z","receivedAt":"2014-11-04T21:41:54Z","isPatch":true,"sender":{"key":"tmikov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7785196?v=4"},"body":"On Tue, Nov 4, 2014 at 1:24 PM, Junio C Hamano wrote:\n> Thanks, both. The patch looks good (modulo the indentation of\n> nparents assignment, which I'll locally fix up).\n>\n> Will queue.\n\nAwesome, thanks!\n(I can't believe I missed that tab. Well, at least one of my two\nlines was correct :-) )\n\nregards,\nTzvetan\n"}]}