{"thread":{"id":"37613","subject":"[PATCH] merge-recursive: Fix copy-paste mistake","startedAt":"2014-09-21T20:49:46Z","lastAt":"2014-09-24T12:52:51Z","messageCount":4,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"249729","messageId":"1411332586-18275-1-git-send-email-stefanbeller@gmail.com","threadId":"37613","inReplyTo":null,"subject":"[PATCH] merge-recursive: Fix copy-paste mistake","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-09-21T20:49:46Z","receivedAt":"2014-09-21T20:49:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The following issue was found by scan.coverity.com (ID: 1049510),\nand claimed to be likely a copy-paste mistake.\n\nIntroduced in 331a1838b (2010-07-02, Try normalizing files\nto avoid delete/modify conflicts when merging), which is\nquite a long time ago, so I'm rather unsure if it's of any impact\nor just went unnoticed.\n\nThe line after the changed line has a comparison of 'o.len' to 'a.len',\nso we should assume the lengths may be different.\n\nI'd be happy to have a test for this bug(?) attached to\nt6031-merge-recursive.sh, but I did not manage to\ncome up with a test in a reasonable amount of time.\n\nSigned-off-by: Stefan Beller <stefanbeller@gmail.com>\n---\n merge-recursive.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 22315c3..d63524c 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1555,7 +1555,7 @@ static int blob_unchanged(const unsigned char *o_sha,\n \t * unchanged since their sha1s have already been compared.\n \t */\n \tif (renormalize_buffer(path, o.buf, o.len, &o) |\n-\t    renormalize_buffer(path, a.buf, o.len, &a))\n+\t    renormalize_buffer(path, a.buf, a.len, &a))\n \t\tret = (o.len == a.len && !memcmp(o.buf, a.buf, o.len));\n \n error_return:\n-- \n2.1.0.238.gce1d3a9\n"},{"id":"249791","messageId":"1411484150-18025-1-git-send-email-stefanbeller@gmail.com","threadId":"37613","inReplyTo":"1411332586-18275-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH] merge-recursive: Remove miss leading comment","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-09-23T14:55:50Z","receivedAt":"2014-09-23T14:55:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Commented code, which doesn't even compile, is of no use any more?\n\nSigned-off-by: Stefan Beller <stefanbeller@gmail.com>\n---\n\nTo be applied on top of sb/merge-recursive-copy-paste-fix\n\n\n merge-recursive.c | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d63524c..8ad4be8 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1686,10 +1686,6 @@ static int merge_content(struct merge_options *o,\n static int process_entry(struct merge_options *o,\n \t\t\t const char *path, struct stage_data *entry)\n {\n-\t/*\n-\tprintf(\"processing entry, clean cache: %s\\n\", index_only ? \"yes\": \"no\");\n-\tprint_index_entry(\"\\tpath: \", entry);\n-\t*/\n \tint clean_merge = 1;\n \tint normalize = o->renormalize;\n \tunsigned o_mode = entry->stages[1].mode;\n-- \n2.1.0.238.gce1d3a9\n"},{"id":"249801","messageId":"xmqq7g0ujiid.fsf@gitster.dls.corp.google.com","threadId":"37613","inReplyTo":"1411484150-18025-1-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH] merge-recursive: Remove miss leading comment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-23T18:31:38Z","receivedAt":"2014-09-23T18:31:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <stefanbeller@gmail.com> writes:\n\n> Commented code, which doesn't even compile, is of no use any more?\n\nApparently that is meant to help debugging the code.  An alternative\nwould be to make it usable again, but removal is fine by me as well.\n\n>\n> Signed-off-by: Stefan Beller <stefanbeller@gmail.com>\n> ---\n>\n> To be applied on top of sb/merge-recursive-copy-paste-fix\n>\n>\n>  merge-recursive.c | 4 ----\n>  1 file changed, 4 deletions(-)\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index d63524c..8ad4be8 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1686,10 +1686,6 @@ static int merge_content(struct merge_options *o,\n>  static int process_entry(struct merge_options *o,\n>  \t\t\t const char *path, struct stage_data *entry)\n>  {\n> -\t/*\n> -\tprintf(\"processing entry, clean cache: %s\\n\", index_only ? \"yes\": \"no\");\n> -\tprint_index_entry(\"\\tpath: \", entry);\n> -\t*/\n>  \tint clean_merge = 1;\n>  \tint normalize = o->renormalize;\n>  \tunsigned o_mode = entry->stages[1].mode;\n"},{"id":"249828","messageId":"5422BEA3.5050107@gmail.com","threadId":"37613","inReplyTo":"1411332586-18275-1-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH] merge-recursive: Fix copy-paste mistake","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-09-24T12:52:51Z","receivedAt":"2014-09-24T12:52:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 21.09.2014 22:49, Stefan Beller wrote:\n> I'd be happy to have a test for this bug(?) attached to\n> t6031-merge-recursive.sh, but I did not manage to\n> come up with a test in a reasonable amount of time.\n\nSo I am just sending out my progress on the testing for this\nas I may be short on time within the next days/weeks.\n\nIf anyone is interested to write a test for this one, \nyou may pickup (parts of) the following.\n\nAt first a rough script, which makes sure the code in question \nis executed.\n\n--8<--\n#!/bin/sh\n\n# for repetitive testing, delete everything:\nrm -rf .git file .gitattributes file_with_more_descriptive_name\ngit init\n\n# run the actual test\ngit commit --allow-empty -m 'initial'\n\necho -n \"content of file:\" '\\r\\n' > file\necho -n \"line 1\" '\\r\\n' >> file\necho -n \"line 2\" '\\r\\n' >> file\ngit add file\ngit commit -m 'commit file with \\r\\n line endings'\n\ngit checkout -b secondbranch\n\ngit rm file\ngit commit -m \"eventually rm file\"\n\ngit checkout master\n\necho \"content of file:\" > file\necho \"line 1\" >> file\necho \"line 2\" >> file\n\necho \"file text=auto\" > .gitattributes\ngit add file .gitattributes\ngit commit -m  \"Add text=auto to .gitattributes; Normalize file ending\"\n\ngit add file\ngit commit -m 'save file with \\r\\f line ending'\n\necho\necho\necho\necho\n\ngit merge secondbranch -m \"merging branches with (modify/delete); modify caused only by normalisation\" -X renormalize\n\n--8<--\n\nHere comes a patch, to be applied to git.git, \nwhich puts printfs all over the place, visualizing the code flow\n\n\n>From 1985955b3ab2bad5ac73cbee92d19f63cdbaa3c9 Mon Sep 17 00:00:00 2001\nFrom: Stefan Beller <stefanbeller@gmail.com>\nDate: Wed, 24 Sep 2014 14:51:55 +0200\nSubject: [PATCH] lots of printfs for debugging [PATCH] merge-recursive: Fix\n copy-paste mistake\n\n---\n merge-recursive.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 51 insertions(+), 1 deletion(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 8ad4be8..9b09a67 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1541,10 +1541,22 @@ static int blob_unchanged(const unsigned char *o_sha,\n \tstruct strbuf a = STRBUF_INIT;\n \tint ret = 0; /* assume changed for safety */\n \n+\tprintf(\"in blob_unchanged path=%s\\n\", path);\n+\n+\n \tif (sha_eq(o_sha, a_sha))\n \t\treturn 1;\n+\telse\n+\t\tprintf(\"continue as o_sha and a_sha are different\\n\");\n+\n \tif (!renormalize)\n \t\treturn 0;\n+\telse\n+\t\tprintf(\"continue as not to renormalize\\n\");\n+\n+\tprintf(\"testing the patch\\n\");\n+\n+\n \n \tassert(o_sha && a_sha);\n \tif (read_sha1_strbuf(o_sha, &o) || read_sha1_strbuf(a_sha, &a))\n@@ -1554,6 +1566,27 @@ static int blob_unchanged(const unsigned char *o_sha,\n \t * performed.  Comparison can be skipped if both files are\n \t * unchanged since their sha1s have already been compared.\n \t */\n+\n+\tprintf(\"o.len %d a.len %d\\n\", o.len, a.len);\n+\tprintf(\"o.buf=|%s|\\n a.buf=|%s|\\n\", o.buf, a.buf);\n+\tstruct strbuf dsta = STRBUF_INIT;\n+\tstruct strbuf dsto = STRBUF_INIT;\n+\n+\trenormalize_buffer(path, a.buf, a.len, &dsta);\n+\tprintf(\"dsta=|%s|\\n\",dsta.buf);\n+\trenormalize_buffer(path, a.buf, a.len, &dsta);\n+\tprintf(\"dsta=|%s|\\n\",dsta.buf);\n+\n+\n+\trenormalize_buffer(path, o.buf, o.len, &dsto);\n+\tprintf(\"dsto=|%s|\\n\",dsto.buf);\n+\trenormalize_buffer(path, o.buf, o.len, &dsto);\n+\tprintf(\"dsto=|%s|\\n\",dsto.buf);\n+\n+\n+\tprintf(\"dsto.len=%d, dsta.len=%d\\n\", dsto.len, dsta.len);\n+\n+\n \tif (renormalize_buffer(path, o.buf, o.len, &o) |\n \t    renormalize_buffer(path, a.buf, a.len, &a))\n \t\tret = (o.len == a.len && !memcmp(o.buf, a.buf, o.len));\n@@ -1682,10 +1715,24 @@ static int merge_content(struct merge_options *o,\n \n }\n \n+\n+//~ struct stage_data {\n+\t//~ struct {\n+\t\t//~ unsigned mode;\n+\t\t//~ unsigned char sha[20];\n+\t//~ } stages[4];\n+\t//~ struct rename_conflict_info *rename_conflict_info;\n+\t//~ unsigned processed:1;\n+//~ };\n+\n /* Per entry merge function */\n static int process_entry(struct merge_options *o,\n \t\t\t const char *path, struct stage_data *entry)\n {\n+\n+\tprintf(\"processing entry %s has rename_conflict_info %p\\n\", path, entry->rename_conflict_info);\n+\t//~ print_index_entry(\"\\tpath: \", entry);\n+\n \tint clean_merge = 1;\n \tint normalize = o->renormalize;\n \tunsigned o_mode = entry->stages[1].mode;\n@@ -1694,9 +1741,10 @@ static int process_entry(struct merge_options *o,\n \tunsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);\n \tunsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);\n \tunsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);\n-\n+\tprintf(\"%s %s %s\\n\", o_sha, a_sha, b_sha);\n \tentry->processed = 1;\n \tif (entry->rename_conflict_info) {\n+\t\tprintf(\"in entry->rename_conflict_info condition\\n\");\n \t\tstruct rename_conflict_info *conflict_info = entry->rename_conflict_info;\n \t\tswitch (conflict_info->rename_type) {\n \t\tcase RENAME_NORMAL:\n@@ -1724,6 +1772,8 @@ static int process_entry(struct merge_options *o,\n \t\t\tbreak;\n \t\t}\n \t} else if (o_sha && (!a_sha || !b_sha)) {\n+\t\tprintf(\"deleted in one, normalize=%d \\n\", normalize);\n+\n \t\t/* Case A: Deleted in one */\n \t\tif ((!a_sha && !b_sha) ||\n \t\t    (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) ||\n-- \n2.1.0.238.gce1d3a9\n"}]}