{"thread":{"id":"44544","subject":"[PATCH 0/2] Fix segmentation fault with cherry-pick","startedAt":"2016-11-25T16:36:26Z","lastAt":"2016-11-28T19:02:03Z","messageCount":11,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"306428","messageId":"cover.1480091758.git.johannes.schindelin@gmx.de","threadId":"44544","inReplyTo":null,"subject":"[PATCH 0/2] Fix segmentation fault with cherry-pick","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-25T16:36:13Z","receivedAt":"2016-11-25T16:36:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The culprit is actually not cherry-pick, but a special code path that\nexpects refresh_cache_entry() not to return NULL. And the fix is to\nteach it to handle NULL there.\n\nThis bug was brought to my attention by Markus Klein via\nhttps://github.com/git-for-windows/git/issues/952.\n\n\nJohannes Schindelin (2):\n cherry-pick: demonstrate a segmentation fault\n Avoid a segmentation fault with renaming merges\n\n merge-recursive.c | 2 ++\n t/t3501-revert-cherry-pick.sh | 12 ++++++++++++\n 2 files changed, 14 insertions(+)\n\n\nbase-commit: e2b2d6a172b76d44cb7b1ddb12ea5bfac9613a44\nPublished-As: https://github.com/dscho/git/releases/tag/cherry-pick-segfault-v1\nFetch-It-Via: git fetch https://github.com/dscho/git cherry-pick-segfault-v1\n\n-- \n2.11.0.rc3.windows.1\n\n"},{"id":"306429","messageId":"89ffd6eaf4e1e121426c84f31dfc9c289f2a948b.1480091758.git.johannes.schindelin@gmx.de","threadId":"44544","inReplyTo":"cover.1480091758.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] cherry-pick: demonstrate a segmentation fault","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-25T16:36:20Z","receivedAt":"2016-11-25T16:36:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In https://github.com/git-for-windows/git/issues/952, a complicated\nscenario was described that leads to a segmentation fault in\ncherry-pick.\n\nIt boils down to a certain code path involving a renamed file that is\ndirty, for which `refresh_cache_entry()` returns `NULL`, and that\n`NULL` not being handled properly.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3501-revert-cherry-pick.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 394f0005a1..d96d391af3 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -141,4 +141,16 @@ test_expect_success 'cherry-pick \"-\" works with arguments' '\n \ttest_cmp expect actual\n '\n \n+test_expect_failure 'cherry-pick fails gracefully with dirty renamed file' '\n+\ttest_commit to-rename &&\n+\tgit checkout -b unrelated &&\n+\ttest_commit unrelated &&\n+\tgit checkout @{-1} &&\n+\tgit mv to-rename.t renamed &&\n+\ttest_tick &&\n+\tgit commit -m renamed &&\n+\techo modified >renamed &&\n+\tgit cherry-pick unrelated\n+'\n+\n test_done\n-- \n2.11.0.rc3.windows.1\n\n\n"},{"id":"306430","messageId":"5b1c217bc490289f1f14bee490533c9e307a2e06.1480091758.git.johannes.schindelin@gmx.de","threadId":"44544","inReplyTo":"cover.1480091758.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/2] Avoid a segmentation fault with renaming merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-25T16:36:26Z","receivedAt":"2016-11-25T16:37:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Under very particular circumstances, merge-recursive's `add_cacheinfo()`\nfunction gets a `NULL` returned from `refresh_cache_entry()` without\nexpecting it, and subsequently passes it to `add_cache_entry()` which\nconsequently crashes.\n\nLet's not crash.\n\nThis fixes https://github.com/git-for-windows/git/issues/952\n\nSigned-off-by: Johannes Schindelin \n---\n merge-recursive.c | 2 ++\n t/t3501-revert-cherry-pick.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 9041c2f149..609061f58a 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,\n \t\tstruct cache_entry *nce;\n \n \t\tnce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);\n+\t\tif (!nce)\n+\t\t\treturn err(o, _(\"addinfo: '%s' is not up-to-date\"), path);\n \t\tif (nce != ce)\n \t\t\tret = add_cache_entry(nce, options);\n \t}\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex d96d391af3..8e21840f11 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick \"-\" works with arguments' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'cherry-pick fails gracefully with dirty renamed file' '\n+test_expect_success 'cherry-pick fails gracefully with dirty renamed file' '\n \ttest_commit to-rename &&\n \tgit checkout -b unrelated &&\n \ttest_commit unrelated &&\n-- \n2.11.0.rc3.windows.1\n"},{"id":"306431","messageId":"alpine.DEB.2.20.1611251739570.117539@virtualbox","threadId":"44544","inReplyTo":"cover.1480091758.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 0/2] Fix segmentation fault with cherry-pick","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-25T16:41:27Z","receivedAt":"2016-11-25T16:48:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 25 Nov 2016, Johannes Schindelin wrote:\n\n> The culprit is actually not cherry-pick, but a special code path that\n> expects refresh_cache_entry() not to return NULL. And the fix is to\n> teach it to handle NULL there.\n> \n> This bug was brought to my attention by Markus Klein via\n> https://github.com/git-for-windows/git/issues/952.\n\nFor the record, I looked at other callers of `refresh_cache_entry()`:\nthere is only `make_cache_entry()`, whose callers all handle NULL return\nvalues except in resolve-undo.c. But that latter caller is okay because it\nspecifically does not allow refreshing (by passing 0 as options), so\nrefresh_cache_entry() cannot return NULL.\n\nCiao,\nDscho\n"},{"id":"306444","messageId":"alpine.DEB.2.20.1611261336240.117539@virtualbox","threadId":"44544","inReplyTo":"89ffd6eaf4e1e121426c84f31dfc9c289f2a948b.1480091758.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/2] cherry-pick: demonstrate a segmentation fault","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-26T12:47:47Z","receivedAt":"2016-11-26T12:48:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 25 Nov 2016, Johannes Schindelin wrote:\n\n> +test_expect_failure 'cherry-pick fails gracefully with dirty renamed file' '\n\nWoops. This title is wrong. It should say instead: 'cherry-pick succeeds\nwith unrelated renamed, dirty file'.\n\n> +\ttest_commit to-rename &&\n> +\tgit checkout -b unrelated &&\n> +\ttest_commit unrelated &&\n> +\tgit checkout @{-1} &&\n> +\tgit mv to-rename.t renamed &&\n> +\ttest_tick &&\n> +\tgit commit -m renamed &&\n> +\techo modified >renamed &&\n> +\tgit cherry-pick unrelated\n\nAnd this actually warns about an ambiguous refname.\n\nWill send out v2 in a moment.\n\nCiao,\nDscho\n"},{"id":"306445","messageId":"cover.1480164459.git.johannes.schindelin@gmx.de","threadId":"44544","inReplyTo":"cover.1480091758.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 0/2] Fix segmentation fault with cherry-pick","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-26T12:47:58Z","receivedAt":"2016-11-26T12:48:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The culprit is actually not cherry-pick, but a special code path that\nexpects refresh_cache_entry() not to return NULL. And the fix is to\nteach it to handle NULL there.\n\nThis bug was brought to my attention by Markus Klein via\nhttps://github.com/git-for-windows/git/issues/952.\n\nChanges since v1:\n\n- changed test title\n\n- avoided ambiguous refname in test\n\n\nJohannes Schindelin (2):\n cherry-pick: demonstrate a segmentation fault\n Avoid a segmentation fault with renaming merges\n\n merge-recursive.c | 2 ++\n t/t3501-revert-cherry-pick.sh | 12 ++++++++++++\n 2 files changed, 14 insertions(+)\n\n\nbase-commit: e2b2d6a172b76d44cb7b1ddb12ea5bfac9613a44\nPublished-As: https://github.com/dscho/git/releases/tag/cherry-pick-segfault-v2\nFetch-It-Via: git fetch https://github.com/dscho/git cherry-pick-segfault-v2\n\nInterdiff vs v1:\n\n diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\n index 8e21840f11..4f2a263b63 100755\n --- a/t/t3501-revert-cherry-pick.sh\n +++ b/t/t3501-revert-cherry-pick.sh\n @@ -141,7 +141,7 @@ test_expect_success 'cherry-pick \"-\" works with arguments' '\n \ttest_cmp expect actual\n '\n \n -test_expect_success 'cherry-pick fails gracefully with dirty renamed file' '\n +test_expect_success 'cherry-pick works with dirty renamed file' '\n \ttest_commit to-rename &&\n \tgit checkout -b unrelated &&\n \ttest_commit unrelated &&\n @@ -150,7 +150,7 @@ test_expect_success 'cherry-pick fails gracefully with dirty renamed file' '\n \ttest_tick &&\n \tgit commit -m renamed &&\n \techo modified >renamed &&\n -\tgit cherry-pick unrelated\n +\tgit cherry-pick refs/heads/unrelated\n '\n \n test_done\n\n-- \n2.11.0.rc3.windows.1\n\n"},{"id":"306446","messageId":"f1a1fbf148e1f0106f47ea0153c4eb0dfa3bd21b.1480164459.git.johannes.schindelin@gmx.de","threadId":"44544","inReplyTo":"cover.1480164459.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 1/2] cherry-pick: demonstrate a segmentation fault","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-26T12:48:02Z","receivedAt":"2016-11-26T12:48:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In https://github.com/git-for-windows/git/issues/952, a complicated\nscenario was described that leads to a segmentation fault in\ncherry-pick.\n\nIt boils down to a certain code path involving a renamed file that is\ndirty, for which `refresh_cache_entry()` returns `NULL`, and that\n`NULL` not being handled properly.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3501-revert-cherry-pick.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 394f0005a1..d7b4251234 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -141,4 +141,16 @@ test_expect_success 'cherry-pick \"-\" works with arguments' '\n \ttest_cmp expect actual\n '\n \n+test_expect_failure 'cherry-pick works with dirty renamed file' '\n+\ttest_commit to-rename &&\n+\tgit checkout -b unrelated &&\n+\ttest_commit unrelated &&\n+\tgit checkout @{-1} &&\n+\tgit mv to-rename.t renamed &&\n+\ttest_tick &&\n+\tgit commit -m renamed &&\n+\techo modified >renamed &&\n+\tgit cherry-pick refs/heads/unrelated\n+'\n+\n test_done\n-- \n2.11.0.rc3.windows.1\n\n\n"},{"id":"306447","messageId":"d1571a25e8f3860a2867b00994d4d6938aa602ec.1480164459.git.johannes.schindelin@gmx.de","threadId":"44544","inReplyTo":"cover.1480164459.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 2/2] Avoid a segmentation fault with renaming merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-26T12:48:06Z","receivedAt":"2016-11-26T12:48:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Under very particular circumstances, merge-recursive's `add_cacheinfo()`\nfunction gets a `NULL` returned from `refresh_cache_entry()` without\nexpecting it, and subsequently passes it to `add_cache_entry()` which\nconsequently crashes.\n\nLet's not crash.\n\nThis fixes https://github.com/git-for-windows/git/issues/952\n\nSigned-off-by: Johannes Schindelin \n---\n merge-recursive.c | 2 ++\n t/t3501-revert-cherry-pick.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 9041c2f149..609061f58a 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,\n \t\tstruct cache_entry *nce;\n \n \t\tnce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);\n+\t\tif (!nce)\n+\t\t\treturn err(o, _(\"addinfo: '%s' is not up-to-date\"), path);\n \t\tif (nce != ce)\n \t\t\tret = add_cache_entry(nce, options);\n \t}\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex d7b4251234..4f2a263b63 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick \"-\" works with arguments' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'cherry-pick works with dirty renamed file' '\n+test_expect_success 'cherry-pick works with dirty renamed file' '\n \ttest_commit to-rename &&\n \tgit checkout -b unrelated &&\n \ttest_commit unrelated &&\n-- \n2.11.0.rc3.windows.1\n"},{"id":"306448","messageId":"alpine.DEB.2.20.1611261348420.117539@virtualbox","threadId":"44544","inReplyTo":"d1571a25e8f3860a2867b00994d4d6938aa602ec.1480164459.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-26T12:53:09Z","receivedAt":"2016-11-26T12:53:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 26 Nov 2016, Johannes Schindelin wrote:\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 9041c2f149..609061f58a 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,\n> \t\tstruct cache_entry *nce;\n> \n> \t\tnce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);\n> +\t\tif (!nce)\n> +\t\t\treturn err(o, _(\"addinfo: '%s' is not up-to-date\"), path);\n> \t\tif (nce != ce)\n> \t\t\tret = add_cache_entry(nce, options);\n> \t}\n\nBTW I was not quite sure why we need to refresh the cache entry here, and\n1335d76e45 (merge: avoid \"safer crlf\" during recording of merge results,\n2016-07-08) has a commit message for which I need some time to wrap my\nhead around.\n\nAlso, an error here may be overkill. Maybe we should simply change the \"if\n(nce != ce)\" to an \"if (nce && nce != ce)\" here, as a locally-modified\nfile will give a nicer message later, anyway.\n\nDunno,\nDscho\n\n"},{"id":"306508","messageId":"xmqqmvgjjvk5.fsf@gitster.mtv.corp.google.com","threadId":"44544","inReplyTo":"alpine.DEB.2.20.1611261348420.117539@virtualbox","subject":"Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-28T18:42:02Z","receivedAt":"2016-11-28T18:42:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Sat, 26 Nov 2016, Johannes Schindelin wrote:\n>\n>> diff --git a/merge-recursive.c b/merge-recursive.c\n>> index 9041c2f149..609061f58a 100644\n>> --- a/merge-recursive.c\n>> +++ b/merge-recursive.c\n>> @@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,\n>> \t\tstruct cache_entry *nce;\n>> \n>> \t\tnce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);\n>> +\t\tif (!nce)\n>> +\t\t\treturn err(o, _(\"addinfo: '%s' is not up-to-date\"), path);\n>> \t\tif (nce != ce)\n>> \t\t\tret = add_cache_entry(nce, options);\n>> \t}\n>\n> BTW I was not quite sure why we need to refresh the cache entry here, and\n> 1335d76e45 (merge: avoid \"safer crlf\" during recording of merge results,\n> 2016-07-08) has a commit message for which I need some time to wrap my\n> head around.\n\nThis callsite used to call make_cache_entry() with CE_MATCH_REFRESH,\nwhich creates a new cache entry, calls refresh_cache_ent, and\nreturns the cache entry\"; the log message attempts to explain why we\navoid passing CE_MATCH_REFRESH and instead first add it as a merged\nentry and then refresh it (and re-add it if ce got changed). We\nused to leave the old (possibly conflicted) entries for the same\npath in the index while refreshing the new cache entry, which has\ncorrectly converted result, and the old entries got in the way,\nattempting a wrong eol conversion and declaring that the new entry\nout-of-date. By adding the correctly converted result as a merged\nentry, which gets rid of the old entries, the refresh operation will\nnot be corrupted by them.\n\n> Also, an error here may be overkill. Maybe we should simply change the \"if\n> (nce != ce)\" to an \"if (nce && nce != ce)\" here, as a locally-modified\n> file will give a nicer message later, anyway.\n\nLooking at the commit you blamed, what happened in this case before\nthat change was that\n\n (1) make_cache_entry() would have called refresh_cache_entry() with\n CE_MATCH_REFRESH and returned a NULL;\n\n (2) merge-recursive.c::add_cacheinfo() noticed NULL and did\n\n return error(_(\"addinfo_cache failed for path '%s'\"), path)\n\nBut the updated code forgot that refresh_cache_entry() could return\nNULL. So 1335d76e45 (\"merge: avoid \"safer crlf\" during recording of\nmerge results\", 2016-07-08) was not a faithful rewrite.\n\nSo I agree that your patch is the right fix; using the old message\nlost by mistake in 1335d76e45 may have made it more clear that this\nis a fix for a misconversion in that commit, though.\n\nIn any case, this does not seem like a new regression (1/2 applied\non v2.10.0 dies the same way), so I am inclined to queue these two\nbut not ship in the upcoming release for now.\n\nThanks.\n\n"},{"id":"306509","messageId":"xmqqinr7juqi.fsf@gitster.mtv.corp.google.com","threadId":"44544","inReplyTo":"xmqqmvgjjvk5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-28T18:59:49Z","receivedAt":"2016-11-28T19:02:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Looking at the commit you blamed, what happened in this case before\n> that change was that\n>\n> (1) make_cache_entry() would have called refresh_cache_entry() with\n> CE_MATCH_REFRESH and returned a NULL;\n>\n> (2) merge-recursive.c::add_cacheinfo() noticed NULL and did\n>\n> return error(_(\"addinfo_cache failed for path '%s'\"), path)\n>\n> But the updated code forgot that refresh_cache_entry() could return\n> NULL. So 1335d76e45 (\"merge: avoid \"safer crlf\" during recording of\n> merge results\", 2016-07-08) was not a faithful rewrite.\n\nI'd tentatively queue the two patches fro you on top of the\njc/renormalize-merge-kill-safer-crlf topic that ends at 1335d76e45\n(\"merge: avoid \"safer crlf\" during recording of merge results\",\n2016-07-08). The real \"fix\" became like this with the above\nanalysis. Semantic adjustment \"error(\" -> \"err(o,\" between the old\ncodebase and the current one will be handled when merging.\n\nThanks for catching my incorrect refactoring.\n\n-- >8 --\nFrom: Johannes Schindelin \nDate: Sat, 26 Nov 2016 13:48:06 +0100\nSubject: [PATCH] merge-recursive: handle NULL in add_cacheinfo() correctly\n\n1335d76e45 (\"merge: avoid \"safer crlf\" during recording of merge\nresults\", 2016-07-08) tried to split make_cache_entry() call made\nwith CE_MATCH_REFRESH into a call to make_cache_entry() without one,\nfollowed by a call to add_cache_entry(), refresh_cache() and another\nadd_cache_entry() as needed. However the conversion was botched in\nthat it forgot that refresh_cache() can return NULL, which was handled\ncorrectly in make_cache_entry() but not in the updated code.\n\nThis fixes https://github.com/git-for-windows/git/issues/952\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Junio C Hamano \n---\n merge-recursive.c | 2 ++\n t/t3501-revert-cherry-pick.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex de37e5153c..56385d4c01 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -213,6 +213,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n \t\tstruct cache_entry *nce;\n \n \t\tnce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);\n+\t\tif (!nce)\n+\t\t\treturn error(_(\"addinfo_cache failed for path '%s'\"), path);\n \t\tif (nce != ce)\n \t\t\tret = add_cache_entry(nce, options);\n \t}\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 5bef564ff1..22970d2223 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick \"-\" works with arguments' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'cherry-pick works with dirty renamed file' '\n+test_expect_success 'cherry-pick works with dirty renamed file' '\n \ttest_commit to-rename &&\n \tgit checkout -b unrelated &&\n \ttest_commit unrelated &&\n-- \n2.11.0-rc3-172-gc8d0e450d3\n\n"}]}