[PATCH] merge-recursive: Fix copy-paste mistake

[PATCH] merge-recursive: Fix copy-paste mistake

[Stefan Beller]

The following issue was found by scan.coverity.com (ID: 1049510),
and claimed to be likely a copy-paste mistake.

Introduced in 331a1838b (2010-07-02, Try normalizing files
to avoid delete/modify conflicts when merging), which is
quite a long time ago, so I'm rather unsure if it's of any impact
or just went unnoticed.

The line after the changed line has a comparison of 'o.len' to 'a.len',
so we should assume the lengths may be different.

I'd be happy to have a test for this bug(?) attached to
t6031-merge-recursive.sh, but I did not manage to
come up with a test in a reasonable amount of time.

Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
---
 merge-recursive.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 22315c3..d63524c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1555,7 +1555,7 @@ static int blob_unchanged(const unsigned char *o_sha,
 	 * unchanged since their sha1s have already been compared.
 	 */
 	if (renormalize_buffer(path, o.buf, o.len, &o) |
-	    renormalize_buffer(path, a.buf, o.len, &a))
+	    renormalize_buffer(path, a.buf, a.len, &a))
 		ret = (o.len == a.len && !memcmp(o.buf, a.buf, o.len));
 
 error_return:
-- 
2.1.0.238.gce1d3a9

[PATCH] merge-recursive: Remove miss leading comment

[Stefan Beller]

Commented code, which doesn't even compile, is of no use any more?

Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
---

To be applied on top of sb/merge-recursive-copy-paste-fix


 merge-recursive.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index d63524c..8ad4be8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1686,10 +1686,6 @@ static int merge_content(struct merge_options *o,
 static int process_entry(struct merge_options *o,
 			 const char *path, struct stage_data *entry)
 {
-	/*
-	printf("processing entry, clean cache: %s\n", index_only ? "yes": "no");
-	print_index_entry("\tpath: ", entry);
-	*/
 	int clean_merge = 1;
 	int normalize = o->renormalize;
 	unsigned o_mode = entry->stages[1].mode;
-- 
2.1.0.238.gce1d3a9

Re: [PATCH] merge-recursive: Remove miss leading comment

[Junio C Hamano]

Stefan Beller <stefanbeller@gmail.com> writes:

> Commented code, which doesn't even compile, is of no use any more?

Apparently that is meant to help debugging the code.  An alternative
would be to make it usable again, but removal is fine by me as well.

>
> Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
> ---
>
> To be applied on top of sb/merge-recursive-copy-paste-fix
>
>
>  merge-recursive.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index d63524c..8ad4be8 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1686,10 +1686,6 @@ static int merge_content(struct merge_options *o,
>  static int process_entry(struct merge_options *o,
>  			 const char *path, struct stage_data *entry)
>  {
> -	/*
> -	printf("processing entry, clean cache: %s\n", index_only ? "yes": "no");
> -	print_index_entry("\tpath: ", entry);
> -	*/
>  	int clean_merge = 1;
>  	int normalize = o->renormalize;
>  	unsigned o_mode = entry->stages[1].mode;

Re: [PATCH] merge-recursive: Fix copy-paste mistake

[Stefan Beller]

On 21.09.2014 22:49, Stefan Beller wrote:
> I'd be happy to have a test for this bug(?) attached to
> t6031-merge-recursive.sh, but I did not manage to
> come up with a test in a reasonable amount of time.

So I am just sending out my progress on the testing for this
as I may be short on time within the next days/weeks.

If anyone is interested to write a test for this one, 
you may pickup (parts of) the following.

At first a rough script, which makes sure the code in question 
is executed.

--8<--
#!/bin/sh

# for repetitive testing, delete everything:
rm -rf .git file .gitattributes file_with_more_descriptive_name
git init

# run the actual test
git commit --allow-empty -m 'initial'

echo -n "content of file:" '\r\n' > file
echo -n "line 1" '\r\n' >> file
echo -n "line 2" '\r\n' >> file
git add file
git commit -m 'commit file with \r\n line endings'

git checkout -b secondbranch

git rm file
git commit -m "eventually rm file"

git checkout master

echo "content of file:" > file
echo "line 1" >> file
echo "line 2" >> file

echo "file text=auto" > .gitattributes
git add file .gitattributes
git commit -m  "Add text=auto to .gitattributes; Normalize file ending"

git add file
git commit -m 'save file with \r\f line ending'

echo
echo
echo
echo

git merge secondbranch -m "merging branches with (modify/delete); modify caused only by normalisation" -X renormalize

--8<--

Here comes a patch, to be applied to git.git, 
which puts printfs all over the place, visualizing the code flow


>From 1985955b3ab2bad5ac73cbee92d19f63cdbaa3c9 Mon Sep 17 00:00:00 2001
From: Stefan Beller <stefanbeller@gmail.com>
Date: Wed, 24 Sep 2014 14:51:55 +0200
Subject: [PATCH] lots of printfs for debugging [PATCH] merge-recursive: Fix
 copy-paste mistake

---
 merge-recursive.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 8ad4be8..9b09a67 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1541,10 +1541,22 @@ static int blob_unchanged(const unsigned char *o_sha,
 	struct strbuf a = STRBUF_INIT;
 	int ret = 0; /* assume changed for safety */
 
+	printf("in blob_unchanged path=%s\n", path);
+
+
 	if (sha_eq(o_sha, a_sha))
 		return 1;
+	else
+		printf("continue as o_sha and a_sha are different\n");
+
 	if (!renormalize)
 		return 0;
+	else
+		printf("continue as not to renormalize\n");
+
+	printf("testing the patch\n");
+
+
 
 	assert(o_sha && a_sha);
 	if (read_sha1_strbuf(o_sha, &o) || read_sha1_strbuf(a_sha, &a))
@@ -1554,6 +1566,27 @@ static int blob_unchanged(const unsigned char *o_sha,
 	 * performed.  Comparison can be skipped if both files are
 	 * unchanged since their sha1s have already been compared.
 	 */
+
+	printf("o.len %d a.len %d\n", o.len, a.len);
+	printf("o.buf=|%s|\n a.buf=|%s|\n", o.buf, a.buf);
+	struct strbuf dsta = STRBUF_INIT;
+	struct strbuf dsto = STRBUF_INIT;
+
+	renormalize_buffer(path, a.buf, a.len, &dsta);
+	printf("dsta=|%s|\n",dsta.buf);
+	renormalize_buffer(path, a.buf, a.len, &dsta);
+	printf("dsta=|%s|\n",dsta.buf);
+
+
+	renormalize_buffer(path, o.buf, o.len, &dsto);
+	printf("dsto=|%s|\n",dsto.buf);
+	renormalize_buffer(path, o.buf, o.len, &dsto);
+	printf("dsto=|%s|\n",dsto.buf);
+
+
+	printf("dsto.len=%d, dsta.len=%d\n", dsto.len, dsta.len);
+
+
 	if (renormalize_buffer(path, o.buf, o.len, &o) |
 	    renormalize_buffer(path, a.buf, a.len, &a))
 		ret = (o.len == a.len && !memcmp(o.buf, a.buf, o.len));
@@ -1682,10 +1715,24 @@ static int merge_content(struct merge_options *o,
 
 }
 
+
+//~ struct stage_data {
+	//~ struct {
+		//~ unsigned mode;
+		//~ unsigned char sha[20];
+	//~ } stages[4];
+	//~ struct rename_conflict_info *rename_conflict_info;
+	//~ unsigned processed:1;
+//~ };
+
 /* Per entry merge function */
 static int process_entry(struct merge_options *o,
 			 const char *path, struct stage_data *entry)
 {
+
+	printf("processing entry %s has rename_conflict_info %p\n", path, entry->rename_conflict_info);
+	//~ print_index_entry("\tpath: ", entry);
+
 	int clean_merge = 1;
 	int normalize = o->renormalize;
 	unsigned o_mode = entry->stages[1].mode;
@@ -1694,9 +1741,10 @@ static int process_entry(struct merge_options *o,
 	unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);
 	unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
 	unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
-
+	printf("%s %s %s\n", o_sha, a_sha, b_sha);
 	entry->processed = 1;
 	if (entry->rename_conflict_info) {
+		printf("in entry->rename_conflict_info condition\n");
 		struct rename_conflict_info *conflict_info = entry->rename_conflict_info;
 		switch (conflict_info->rename_type) {
 		case RENAME_NORMAL:
@@ -1724,6 +1772,8 @@ static int process_entry(struct merge_options *o,
 			break;
 		}
 	} else if (o_sha && (!a_sha || !b_sha)) {
+		printf("deleted in one, normalize=%d \n", normalize);
+
 		/* Case A: Deleted in one */
 		if ((!a_sha && !b_sha) ||
 		    (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) ||
-- 
2.1.0.238.gce1d3a9