Merge branch 'jk/no-more-self-assignment' into maint

* jk/no-more-self-assignment:
  match-trees: simplify score_trees() using tree_entry()
  submodule: clarify logic in show_submodule_summary

2 files changed, 34 insertions, 47 deletions

diff --git a/match-trees.c b/match-trees.c
index 26f7ed143..2bb734d51 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -47,6 +47,13 @@ static int score_matches(unsigned mode1, unsigned mode2, const char *path)
 	return score;
 }
 
+static int base_name_entries_compare(const struct name_entry *a,
+				     const struct name_entry *b)
+{
+	return base_name_compare(a->path, tree_entry_len(a), a->mode,
+				 b->path, tree_entry_len(b), b->mode);
+}
+
 /*
  * Inspect two trees, and give a score that tells how similar they are.
  */
@@ -71,54 +78,35 @@ static int score_trees(const unsigned char *hash1, const unsigned char *hash2)
 	if (type != OBJ_TREE)
 		die("%s is not a tree", sha1_to_hex(hash2));
 	init_tree_desc(&two, two_buf, size);
-	while (one.size | two.size) {
-		const unsigned char *elem1 = elem1;
-		const unsigned char *elem2 = elem2;
-		const char *path1 = path1;
-		const char *path2 = path2;
-		unsigned mode1 = mode1;
-		unsigned mode2 = mode2;
+	for (;;) {
+		struct name_entry e1, e2;
+		int got_entry_from_one = tree_entry(&one, &e1);
+		int got_entry_from_two = tree_entry(&two, &e2);
 		int cmp;
 
-		if (one.size)
-			elem1 = tree_entry_extract(&one, &path1, &mode1);
-		if (two.size)
-			elem2 = tree_entry_extract(&two, &path2, &mode2);
-
-		if (!one.size) {
-			/* two has more entries */
-			score += score_missing(mode2, path2);
-			update_tree_entry(&two);
-			continue;
-		}
-		if (!two.size) {
+		if (got_entry_from_one && got_entry_from_two)
+			cmp = base_name_entries_compare(&e1, &e2);
+		else if (got_entry_from_one)
 			/* two lacks this entry */
-			score += score_missing(mode1, path1);
-			update_tree_entry(&one);
-			continue;
-		}
-		cmp = base_name_compare(path1, strlen(path1), mode1,
-					path2, strlen(path2), mode2);
-		if (cmp < 0) {
+			cmp = -1;
+		else if (got_entry_from_two)
+			/* two has more entries */
+			cmp = 1;
+		else
+			break;
+
+		if (cmp < 0)
 			/* path1 does not appear in two */
-			score += score_missing(mode1, path1);
-			update_tree_entry(&one);
-			continue;
-		}
-		else if (cmp > 0) {
+			score += score_missing(e1.mode, e1.path);
+		else if (cmp > 0)
 			/* path2 does not appear in one */
-			score += score_missing(mode2, path2);
-			update_tree_entry(&two);
-			continue;
-		}
-		else if (hashcmp(elem1, elem2))
+			score += score_missing(e2.mode, e2.path);
+		else if (hashcmp(e1.sha1, e2.sha1))
 			/* they are different */
-			score += score_differs(mode1, mode2, path1);
+			score += score_differs(e1.mode, e2.mode, e1.path);
 		else
 			/* same subtree or blob */
-			score += score_matches(mode1, mode2, path1);
-		update_tree_entry(&one);
-		update_tree_entry(&two);
+			score += score_matches(e1.mode, e2.mode, e1.path);
 	}
 	free(one_buf);
 	free(two_buf);
diff --git a/submodule.c b/submodule.c
index 9ba149654..975bc87e4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -261,7 +261,7 @@ void show_submodule_summary(FILE *f, const char *path,
 		const char *del, const char *add, const char *reset)
 {
 	struct rev_info rev;
-	struct commit *left = left, *right = right;
+	struct commit *left = NULL, *right = NULL;
 	const char *message = NULL;
 	struct strbuf sb = STRBUF_INIT;
 	int fast_forward = 0, fast_backward = 0;
@@ -275,10 +275,8 @@ void show_submodule_summary(FILE *f, const char *path,
 	else if (!(left = lookup_commit_reference(one)) ||
 		 !(right = lookup_commit_reference(two)))
 		message = "(commits not present)";
-
-	if (!message &&
-	    prepare_submodule_summary(&rev, path, left, right,
-					&fast_forward, &fast_backward))
+	else if (prepare_submodule_summary(&rev, path, left, right,
+					   &fast_forward, &fast_backward))
 		message = "(revision walker failed)";
 
 	if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
@@ -302,11 +300,12 @@ void show_submodule_summary(FILE *f, const char *path,
 		strbuf_addf(&sb, "%s:%s\n", fast_backward ? " (rewind)" : "", reset);
 	fwrite(sb.buf, sb.len, 1, f);
 
-	if (!message) {
+	if (!message) /* only NULL if we succeeded in setting up the walk */
 		print_submodule_summary(&rev, f, del, add, reset);
+	if (left)
 		clear_commit_marks(left, ~0);
+	if (right)
 		clear_commit_marks(right, ~0);
-	}
 
 	strbuf_release(&sb);
 }