Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 28, 2016, 18:59 UTC
- Message-ID
- <xmqqinr7juqi.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <xmqqmvgjjvk5.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> Looking at the commit you blamed, what happened in this case before
> that change was that
>
> (1) make_cache_entry() would have called refresh_cache_entry() with
> CE_MATCH_REFRESH and returned a NULL;
>
> (2) merge-recursive.c::add_cacheinfo() noticed NULL and did
>
> return error(_("addinfo_cache failed for path '%s'"), path)
>
> But the updated code forgot that refresh_cache_entry() could return
> NULL. So 1335d76e45 ("merge: avoid "safer crlf" during recording of
> merge results", 2016-07-08) was not a faithful rewrite.I'd tentatively queue the two patches fro you on top of the jc/renormalize-merge-kill-safer-crlf topic that ends at 1335d76e45 ("merge: avoid "safer crlf" during recording of merge results", 2016-07-08). The real "fix" became like this with the above analysis. Semantic adjustment "error(" -> "err(o," between the old codebase and the current one will be handled when merging.
Thanks for catching my incorrect refactoring.
-- >8 -- From: Johannes Schindelin <johannes.schindelin@gmx.de> Date: Sat, 26 Nov 2016 13:48:06 +0100 Subject: [PATCH] merge-recursive: handle NULL in add_cacheinfo() correctly
1335d76e45 ("merge: avoid "safer crlf" during recording of merge results", 2016-07-08) tried to split make_cache_entry() call made with CE_MATCH_REFRESH into a call to make_cache_entry() without one, followed by a call to add_cache_entry(), refresh_cache() and another add_cache_entry() as needed. However the conversion was botched in that it forgot that refresh_cache() can return NULL, which was handled correctly in make_cache_entry() but not in the updated code.
This fixes https://github.com/git-for-windows/git/issues/952
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- merge-recursive.c | 2 ++ t/t3501-revert-cherry-pick.sh | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/merge-recursive.c b/merge-recursive.c index de37e5153c..56385d4c01 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -213,6 +213,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1, struct cache_entry *nce; nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING); + if (!nce) + return error(_("addinfo_cache failed for path '%s'"), path); if (nce != ce) ret = add_cache_entry(nce, options); } diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh index 5bef564ff1..22970d2223 100755 --- a/t/t3501-revert-cherry-pick.sh +++ b/t/t3501-revert-cherry-pick.sh @@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' ' test_cmp expect actual ' -test_expect_failure 'cherry-pick works with dirty renamed file' ' +test_expect_success 'cherry-pick works with dirty renamed file' ' test_commit to-rename && git checkout -b unrelated && test_commit unrelated &&
-- 2.11.0-rc3-172-gc8d0e450d3