bisect: don't use invalid oid as rev when starting

In 06f5608c14 (bisect--helper: `bisect_start` shell function
partially in C, 2019-01-02), we changed the following shell
code:

-      rev=$(git rev-parse -q --verify "$arg^{commit}") || {
-              test $has_double_dash -eq 1 &&
-              die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
-              break
-      }
-      revs="$revs $rev"

into:

+      char *commit_id = xstrfmt("%s^{commit}", arg);
+      if (get_oid(commit_id, &oid) && has_double_dash)
+              die(_("'%s' does not appear to be a valid "
+                    "revision"), arg);
+
+      string_list_append(&revs, oid_to_hex(&oid));
+      free(commit_id);

In case of an invalid "arg" when "has_double_dash" is false, the old
code would "break" out of the argument loop.

In the new C code though, `oid_to_hex(&oid)` is unconditonally
appended to "revs". This is wrong first because "oid" is junk as
`get_oid(commit_id, &oid)` failed and second because it doesn't break
out of the argument loop.

Not breaking out of the argument loop means that "arg" is then not
treated as a path restriction (which is wrong).

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pull/862/head
Christian Couder 2 years ago committed by Junio C Hamano
parent 7397ca3373
commit 73c6de06af
  1. 13
      builtin/bisect--helper.c
  2. 7
      t/t6030-bisect-porcelain.sh

@ -474,14 +474,13 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
} else if (starts_with(arg, "--") &&
!one_of(arg, "--term-good", "--term-bad", NULL)) {
return error(_("unrecognized option: '%s'"), arg);
} else {
char *commit_id = xstrfmt("%s^{commit}", arg);
if (get_oid(commit_id, &oid) && has_double_dash)
die(_("'%s' does not appear to be a valid "
"revision"), arg);
} else if (!get_oidf(&oid, "%s^{commit}", arg)) {
string_list_append(&revs, oid_to_hex(&oid));
free(commit_id);
} else if (has_double_dash) {
die(_("'%s' does not appear to be a valid "
"revision"), arg);
} else {
break;
}
}
pathspec_pos = i;

@ -82,6 +82,13 @@ test_expect_success 'bisect fails if given any junk instead of revs' '
git bisect bad $HASH4
'
test_expect_success 'bisect start without -- takes unknown arg as pathspec' '
git bisect reset &&
git bisect start foo bar &&
grep foo ".git/BISECT_NAMES" &&
grep bar ".git/BISECT_NAMES"
'
test_expect_success 'bisect reset: back in the master branch' '
git bisect reset &&
echo "* master" > branch.expect &&

Loading…
Cancel
Save