[PATCH 7/4] submodule: clarify logic in show_submodule_summary

[Jeff King <peff@peff.net>]

There are two uses of the "left" and "right" commit
variables that make it hard to be sure what values they
have (both for the reader, and for gcc, which wrongly
complains that they might be used uninitialized).

The functions starts with a cascading if statement, checking
that the input sha1s exist, and finally working up to
preparing a revision walk. We only prepare the walk if the
cascading conditional did not find any problems, which we
check by seeing whether it set the "message" variable or
not. It's simpler and more obvious to just add a condition
to the end of the cascade.

Later, we check the same "message" variable when deciding
whether to clear commit marks on the left/right commits; if
it is set, we presumably never started the walk. This is
wrong, though; we might have started the walk and munged
commit flags, only to encounter an error afterwards. We
should always clear the flags on left/right if they exist,
whether the walk was successful or not.

Signed-off-by: Jeff King <peff@peff.net>
---
 submodule.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/submodule.c b/submodule.c
index 9ba1496..975bc87 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);
 }
-- 
1.8.2.13.g0f18d3c