Pranit Bauva <[email protected]> writes:
> +static int bisect_reset(const char *commit)
> +{
> + struct strbuf branch = STRBUF_INIT;
> +
> + if (!commit) {
> + if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
> + return !printf(_("We are not bisecting.\n"));
> + strbuf_rtrim(&branch);
> + } else {
> + struct object_id oid;
> +
> + if (get_oid_commit(commit, &oid))
> + return error(_("'%s' is not a valid commit"), commit);
> + strbuf_addstr(&branch, commit);
The original checks "test -s BISECT_START" and complains, even when
an explicit commit is given. With this change, when the user is not
bisecting, giving "git bisect reset master" goes ahead---it is
likely that BISECT_HEAD does not exist and we may hit "Could not
check out" error, but if BISECT_HEAD is left behind from a previous
run (which is likely completely unrelated to whatever the user
currently is doing), we'd end up doing quite a random thing, no?
> + }
> +
> + if (!file_exists(git_path_bisect_head())) {
> + struct argv_array argv = ARGV_ARRAY_INIT;
> +
> + argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL);
> + if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
> + error(_("Could not check out original HEAD '%s'. Try "
> + "'git bisect reset <commit>'."), branch.buf);
> + strbuf_release(&branch);
> + argv_array_clear(&argv);
> + return -1;
How does this return value affect the value eventually given to
exit(3), called by somewhere in git.c that called this function?
The call graph would be
common-main.c::main()
-> git.c::cmd_main()
-> handle_builtin()
. exit(run_builtin())
-> run_builtin()
. status = p->fn()
-> cmd_bisect__helper()
. return bisect_reset()
-> bisect_reset()
. return -1
. if (status) return status;
So the -1 is returned throughout the callchain and exit(3) ends up
getting it---which is not quite right. We shouldn't be giving
negative value to exit(3). bisect_clean_state() and other helper
functions may already share the same issue.
> + }
> + argv_array_clear(&argv);
> + }
> +
> + strbuf_release(&branch);
> + return bisect_clean_state();
> +}
