From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, Markus Klein <markus.klein@reelworx.at>
Subject: Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges
Date: Mon, 28 Nov 2016 10:59:49 -0800 [thread overview]
Message-ID: <xmqqinr7juqi.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqmvgjjvk5.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Mon, 28 Nov 2016 10:42:02 -0800")
Junio C Hamano <gitster@pobox.com> writes:
> 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
prev parent reply other threads:[~2016-11-28 19:02 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-25 16:36 [PATCH 0/2] Fix segmentation fault with cherry-pick Johannes Schindelin
2016-11-25 16:36 ` [PATCH 1/2] cherry-pick: demonstrate a segmentation fault Johannes Schindelin
2016-11-26 12:47 ` Johannes Schindelin
2016-11-25 16:36 ` [PATCH 2/2] Avoid a segmentation fault with renaming merges Johannes Schindelin
2016-11-25 16:41 ` [PATCH 0/2] Fix segmentation fault with cherry-pick Johannes Schindelin
2016-11-26 12:47 ` [PATCH v2 " Johannes Schindelin
2016-11-26 12:48 ` [PATCH v2 1/2] cherry-pick: demonstrate a segmentation fault Johannes Schindelin
2016-11-26 12:48 ` [PATCH v2 2/2] Avoid a segmentation fault with renaming merges Johannes Schindelin
2016-11-26 12:53 ` Johannes Schindelin
2016-11-28 18:42 ` Junio C Hamano
2016-11-28 18:59 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqinr7juqi.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=markus.klein@reelworx.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).