blame: fix object casting regression

Commit 1b0d400 refactored the prepare_final() function so that it could be reused in multiple places. Originally, the loop had two outputs: a commit to stuff into sb->final, and the name of the commit from the rev->pending array. After the refactor, that loop is put in its own function with a single return value: the object_array_entry from the rev->pending array. This contains both the name and the object, but with one important difference: the object is the _original_ object found by the revision parser, not the dereferenced commit. If one feeds a tag to "git blame", we end up casting the tag object to a "struct commit", which causes a segfault. Instead, let's return the commit (properly casted) directly from the function, and take the "name" as an optional out-parameter. This does the right thing, and actually simplifies the callers, who no longer need to cast or dereference the object_array_entry themselves. [test case by Max Kirillov <max@max630.net>] Signed-off-by: Jeff King <peff@peff.net>

Jeff King committed Nov 17, 2015 at 18:22 UTC 7cb5f7c44dfea798a5ad99ee5b42fdaf8de4e379
2 files changed +21 -16
builtin/blame.c
+14 -16
@@ -2396,10 +2396,12 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
2396 return commit;
2397 }
2398
2399 -static struct object_array_entry *find_single_final(struct rev_info *revs)
2399 +static struct commit *find_single_final(struct rev_info *revs,
2400 + const char **name_p)
2401 {
2402 int i;
2402 - struct object_array_entry *found = NULL;
2403 + struct commit *found = NULL;
2404 + const char *name = NULL;
2405
2406 for (i = 0; i < revs->pending.nr; i++) {
2407 struct object *obj = revs->pending.objects[i].item;
@@ -2411,22 +2413,20 @@ static struct object_array_entry *find_single_final(struct rev_info *revs)
2413 die("Non commit %s?", revs->pending.objects[i].name);
2414 if (found)
2415 die("More than one commit to dig from %s and %s?",
2414 - revs->pending.objects[i].name,
2415 - found->name);
2416 - found = &(revs->pending.objects[i]);
2416 + revs->pending.objects[i].name, name);
2417 + found = (struct commit *)obj;
2418 + name = revs->pending.objects[i].name;
2419 }
2420 + if (name_p)
2421 + *name_p = name;
2422 return found;
2423 }
2424
2425 static char *prepare_final(struct scoreboard *sb)
2426 {
2423 - struct object_array_entry *found = find_single_final(sb->revs);
2424 - if (found) {
2425 - sb->final = (struct commit *) found->item;
2426 - return xstrdup(found->name);
2427 - } else {
2428 - return NULL;
2429 - }
2427 + const char *name;
2428 + sb->final = find_single_final(sb->revs, &name);
2429 + return xstrdup_or_null(name);
2430 }
2431
2432 static char *prepare_initial(struct scoreboard *sb)
@@ -2712,11 +2712,9 @@ parse_done:
2712 die("Cannot use --contents with final commit object name");
2713
2714 if (reverse && revs.first_parent_only) {
2715 - struct object_array_entry *entry = find_single_final(sb.revs);
2716 - if (!entry)
2715 + final_commit = find_single_final(sb.revs, NULL);
2716 + if (!final_commit)
2717 die("--reverse and --first-parent together require specified latest commit");
2718 - else
2719 - final_commit = (struct commit*) entry->item;
2718 }
2719
2720 /*
t/annotate-tests.sh
+7
@@ -68,6 +68,13 @@ test_expect_success 'blame 1 author' '
68 check_count A 2
69 '
70
71 +test_expect_success 'blame by tag objects' '
72 + git tag -m "test tag" testTag &&
73 + git tag -m "test tag #2" testTag2 testTag &&
74 + check_count -h testTag A 2 &&
75 + check_count -h testTag2 A 2
76 +'
77 +
78 test_expect_success 'setup B lines' '
79 echo "2A quick brown fox jumps over the" >>file &&
80 echo "lazy dog" >>file &&