Re: [PATCH 8/4] match-trees: drop "x = x" initializations

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Am 22.03.2013 17:21, schrieb Jeff King:
> Of the 8 patches, this is the one I find the least satisfying, if only
> because I do not think gcc's failure is because of complicated control
> flow, and rearranging the code would only hurt readability.

Hmm, let's see if we can help the compiler follow the code without
making it harder for people to understand.  The patch looks a bit
jumbled, but the resulting code is OK in my biased opinion.

-- >8 --
There are two ways we can spot missing entries, i.e. added or removed
files: By reaching the end of one of the trees while the other still
has entries, or in the middle of the two lists with base_name_compare().
Missing files are handled the same in either case, but the code is
duplicated.

Unify the handling by just setting cmp appropriately when running off
a tree instead of handling the case on the spot.  If both trees contain
entries, call base_name_compare() as usual.

This make the code slightly shorter, and also helps gcc 4.6 to
understand that none of the variables in the loop are used without
initialization.  Therefore we can remove the trick to initialize them
using themselves, which was used to squelch false warnings.

[Stolen from Jeff King:]
While we're in the area, let's also update the loop
condition to use logical-OR rather than bitwise-OR. They should
be equivalent in this case, and the use of the latter was
probably a typo.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 match-trees.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/match-trees.c b/match-trees.c
index 26f7ed1..c0c66bb 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -71,34 +71,26 @@ 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;
-		int cmp;
+	while (one.size || two.size) {
+		const unsigned char *elem1, *elem2;
+		const char *path1, *path2;
+		unsigned mode1, mode2;
+		int cmp = 0;
 
 		if (one.size)
 			elem1 = tree_entry_extract(&one, &path1, &mode1);
+		else
+			/* two has more entries */
+			cmp = 1;
 		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) {
+		else
 			/* 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);
+			cmp = -1;
+
+		if (!cmp)
+			cmp = base_name_compare(path1, strlen(path1), mode1,
+						path2, strlen(path2), mode2);
 		if (cmp < 0) {
 			/* path1 does not appear in two */
 			score += score_missing(mode1, path1);
-- 
1.8.2