{"thread":{"id":"33387","subject":"[PATCH] rerere forget: do not segfault if not all stages are present","startedAt":"2013-04-04T18:41:43Z","lastAt":"2013-04-04T18:41:43Z","messageCount":1,"participants":["Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"213156","messageId":"515DC967.3060108@kdbg.org","threadId":"33387","inReplyTo":null,"subject":"[PATCH] rerere forget: do not segfault if not all stages are present","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-04-04T18:41:43Z","receivedAt":"2013-04-04T18:41:43Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The loop that fills in the buffers that are later passed to the merge\ndriver exits early when not all stages of a path are present in the index.\nBut since the buffer pointers are not initialized in advance, the\nsubsequent accesses are undefined.\n\nInitialize buffer pointers in advance to avoid undefined behavior later.\n\nThat is not sufficient, though, to get correct operation of handle_cache().\nThe function replays a conflicted merge to extract the part inside the\nconflict markers. As written, the loop exits early when a stage is missing.\nConsequently, the buffers for later stages that would be present in the\nindex are not filled in and the merge is replayed with incomplete data.\n\nFix it by investigating all stages of the given path.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n This patch conflicts trivially with js/rerere-forget-protect-against-NUL\n (both add a test case at the end).\n\n rerere.c                  | 15 +++++++--------\n t/t2030-unresolve-info.sh | 13 +++++++++++++\n 2 files changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex a6a5cd5..3d22cd2 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -295,7 +295,7 @@ static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)\n \n static int handle_cache(const char *path, unsigned char *sha1, const char *output)\n {\n-\tmmfile_t mmfile[3];\n+\tmmfile_t mmfile[3] = {{NULL}};\n \tmmbuffer_t result = {NULL, 0};\n \tstruct cache_entry *ce;\n \tint pos, len, i, hunk_no;\n@@ -314,17 +314,16 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \tfor (i = 0; i < 3; i++) {\n \t\tenum object_type type;\n \t\tunsigned long size;\n+\t\tint j;\n \n-\t\tmmfile[i].size = 0;\n-\t\tmmfile[i].ptr = NULL;\n \t\tif (active_nr <= pos)\n \t\t\tbreak;\n \t\tce = active_cache[pos++];\n-\t\tif (ce_namelen(ce) != len || memcmp(ce->name, path, len)\n-\t\t    || ce_stage(ce) != i + 1)\n-\t\t\tbreak;\n-\t\tmmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);\n-\t\tmmfile[i].size = size;\n+\t\tif (ce_namelen(ce) != len || memcmp(ce->name, path, len))\n+\t\t\tcontinue;\n+\t\tj = ce_stage(ce) - 1;\n+\t\tmmfile[j].ptr = read_sha1_file(ce->sha1, &type, &size);\n+\t\tmmfile[j].size = size;\n \t}\n \tfor (i = 0; i < 3; i++) {\n \t\tif (!mmfile[i].ptr && !mmfile[i].size)\ndiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\nindex f262065..c0610b9 100755\n--- a/t/t2030-unresolve-info.sh\n+++ b/t/t2030-unresolve-info.sh\n@@ -50,8 +50,11 @@ test_expect_success setup '\n \ttest_commit second fi/le second &&\n \tgit checkout side &&\n \ttest_commit third fi/le third &&\n+\tgit branch add-add &&\n \tgit checkout another &&\n \ttest_commit fourth fi/le fourth &&\n+\tgit checkout add-add &&\n+\ttest_commit fifth add-differently &&\n \tgit checkout master\n '\n \n@@ -167,4 +170,14 @@ test_expect_success 'rerere and rerere forget (subdirectory)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'rerere forget (add-add conflict)' '\n+\tgit checkout -f master &&\n+\techo master >add-differently &&\n+\tgit add add-differently &&\n+\tgit commit -m \"add differently\" &&\n+\ttest_must_fail git merge fifth &&\n+\tgit rerere forget add-differently 2>actual &&\n+\ttest_i18ngrep \"no remembered\" actual\n+'\n+\n test_done\n-- \n1.8.2.384.g92e0289\n"}]}