From 05f2dfb965476a59050b7c3446b1281bdcac7051 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 26 Nov 2016 13:48:02 +0100 Subject: [PATCH 1/3] cherry-pick: demonstrate a segmentation fault In https://github.com/git-for-windows/git/issues/952, a complicated scenario was described that leads to a segmentation fault in cherry-pick. It boils down to a certain code path involving a renamed file that is dirty, for which `refresh_cache_entry()` returns `NULL`, and that `NULL` not being handled properly. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3501-revert-cherry-pick.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh index 51f3bbb8af..5bef564ff1 100755 --- a/t/t3501-revert-cherry-pick.sh +++ b/t/t3501-revert-cherry-pick.sh @@ -141,4 +141,16 @@ test_expect_success 'cherry-pick "-" works with arguments' ' test_cmp expect actual ' +test_expect_failure 'cherry-pick works with dirty renamed file' ' + test_commit to-rename && + git checkout -b unrelated && + test_commit unrelated && + git checkout @{-1} && + git mv to-rename.t renamed && + test_tick && + git commit -m renamed && + echo modified >renamed && + git cherry-pick refs/heads/unrelated +' + test_done From 55e9f0e5c9a918c246b7eae1fe2a2e954f6426af Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 26 Nov 2016 13:48:06 +0100 Subject: [PATCH 2/3] 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 in the updated code. This fixes https://github.com/git-for-windows/git/issues/952 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- 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 && From 1c25d2d8ed4c5154c9059918e20e75cda3bede81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torsten=20B=C3=B6gershausen?= Date: Wed, 30 Nov 2016 18:02:32 +0100 Subject: [PATCH 3/3] convert: git cherry-pick -Xrenormalize did not work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Working with a repo that used to be all CRLF. At some point it was changed to all LF, with `text=auto` in .gitattributes. Trying to cherry-pick a commit from before the switchover fails: $ git cherry-pick -Xrenormalize fatal: CRLF would be replaced by LF in [path] Commit 65237284 "unify the "auto" handling of CRLF" introduced a regression: Whenever crlf_action is CRLF_TEXT_XXX and not CRLF_AUTO_XXX, SAFE_CRLF_RENORMALIZE was feed into check_safe_crlf(). This is wrong because here everything else than SAFE_CRLF_WARN is treated as SAFE_CRLF_FAIL. Call check_safe_crlf() only if checksafe is SAFE_CRLF_WARN or SAFE_CRLF_FAIL. Reported-by: Eevee (Lexy Munroe) Signed-off-by: Torsten Bögershausen Signed-off-by: Junio C Hamano --- convert.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/convert.c b/convert.c index 077f5e601e..2f90f363c6 100644 --- a/convert.c +++ b/convert.c @@ -274,15 +274,16 @@ static int crlf_to_git(const char *path, const char *src, size_t len, if (convert_is_binary(len, &stats)) return 0; /* - * If the file in the index has any CR in it, do not convert. - * This is the new safer autocrlf handling. + * If the file in the index has any CR in it, do not + * convert. This is the new safer autocrlf handling, + * unless we want to renormalize in a merge or + * cherry-pick. */ - if (checksafe == SAFE_CRLF_RENORMALIZE) - checksafe = SAFE_CRLF_FALSE; - else if (has_cr_in_index(path)) + if ((checksafe != SAFE_CRLF_RENORMALIZE) && has_cr_in_index(path)) convert_crlf_into_lf = 0; } - if (checksafe && len) { + if ((checksafe == SAFE_CRLF_WARN || + (checksafe == SAFE_CRLF_FAIL)) && len) { struct text_stat new_stats; memcpy(&new_stats, &stats, sizeof(new_stats)); /* simulate "git add" */