@matheusbsbs / matheus-bagliano / commits / 837154978e

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>

Jeff King committed Mar 22, 2013 at 12:19 UTC 837154978e94586027e9684ecfbdfd4178c1c8aa

1 file changed +6 -7

submodule.c

+6 -7

@@ -261,7 +261,7 @@ void show_submodule_summary(FILE *f, const char *path,

261 const char *del, const char *add, const char *reset)

262 {

263 struct rev_info rev;

264 - struct commit *left = left, *right = right;

264 + struct commit *left = NULL, *right = NULL;

265 const char *message = NULL;

266 struct strbuf sb = STRBUF_INIT;

267 int fast_forward = 0, fast_backward = 0;

@@ -275,10 +275,8 @@ void show_submodule_summary(FILE *f, const char *path,

275 else if (!(left = lookup_commit_reference(one)) ||

276 !(right = lookup_commit_reference(two)))

277 message = "(commits not present)";

278 -

279 - if (!message &&

280 - prepare_submodule_summary(&rev, path, left, right,

281 - &fast_forward, &fast_backward))

278 + else if (prepare_submodule_summary(&rev, path, left, right,

279 + &fast_forward, &fast_backward))

280 message = "(revision walker failed)";

281

282 if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)

@@ -302,11 +300,12 @@ void show_submodule_summary(FILE *f, const char *path,

300 strbuf_addf(&sb, "%s:%s\n", fast_backward ? " (rewind)" : "", reset);

301 fwrite(sb.buf, sb.len, 1, f);

302

305 - if (!message) {

303 + if (!message) /* only NULL if we succeeded in setting up the walk */

304 print_submodule_summary(&rev, f, del, add, reset);

305 + if (left)

306 clear_commit_marks(left, ~0);

307 + if (right)

308 clear_commit_marks(right, ~0);

309 - }

309

310 strbuf_release(&sb);

311 }