aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2013-03-22 12:19:56 -0400
committerJunio C Hamano <gitster@pobox.com>2013-03-22 14:09:55 -0700
commit837154978e94586027e9684ecfbdfd4178c1c8aa (patch)
tree4234d4588503d07021b41c1448d97b0cd2639f07
parentc9fc4415e2c7d7673cdad34d41114ede3435a395 (diff)
downloadgit-837154978e94586027e9684ecfbdfd4178c1c8aa.tar.gz
git-837154978e94586027e9684ecfbdfd4178c1c8aa.tar.xz
submodule: clarify logic in show_submodule_summary
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 function 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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--submodule.c13
1 files changed, 6 insertions, 7 deletions
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);
}