mirror of
https://github.com/git/git.git
synced 2026-08-07 16:42:02 +00:00
bisect: check get_terms return at all call sites
Six callers of get_terms() silently discard its return value. When get_terms fails (missing or truncated BISECT_TERMS file), the term strings remain NULL or empty, causing confusing downstream behavior: commands like "bisect next" or "bisect run" proceed with empty term strings, producing nonsensical ref names (refs/bisect/ with no suffix) and misleading error messages. Let's not discard the return value, but handle an error with the same message `bisect_terms()` already uses when reading the terms failed. Pointed out by Coverity. There is one slight complication here: One caller _needs_ the return value to indicate an error when the `BISECT_TERMS` file is absent, all the other call sites are totally okay with a "missing" `BISECT_TERMS` file. To address that, extend the function signature of `get_terms()` to indicate which behavior the caller wants. Assisted-by: Claude Opus 4.6 Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
committed by
Junio C Hamano
parent
f99cc3d747
commit
06f493ab48
@@ -485,7 +485,7 @@ static int bisect_next_check(const struct bisect_terms *terms,
|
||||
return decide_next(terms, current_term, !state.nr_good, !state.nr_bad);
|
||||
}
|
||||
|
||||
static int get_terms(struct bisect_terms *terms)
|
||||
static int get_terms(struct bisect_terms *terms, int file_missing_is_ok)
|
||||
{
|
||||
struct strbuf str = STRBUF_INIT;
|
||||
FILE *fp = NULL;
|
||||
@@ -493,7 +493,7 @@ static int get_terms(struct bisect_terms *terms)
|
||||
|
||||
fp = fopen(git_path_bisect_terms(), "r");
|
||||
if (!fp) {
|
||||
res = -1;
|
||||
res = file_missing_is_ok ? 0 : -1;
|
||||
goto finish;
|
||||
}
|
||||
|
||||
@@ -519,7 +519,7 @@ finish:
|
||||
|
||||
static int bisect_terms(struct bisect_terms *terms, const char *option)
|
||||
{
|
||||
if (get_terms(terms))
|
||||
if (get_terms(terms, 0))
|
||||
return error(_("no terms defined"));
|
||||
|
||||
if (!option) {
|
||||
@@ -1057,7 +1057,8 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
|
||||
rev = word_end + strspn(word_end, " \t");
|
||||
*word_end = '\0'; /* NUL-terminate the word */
|
||||
|
||||
get_terms(terms);
|
||||
if (get_terms(terms, 1))
|
||||
return error(_("no terms defined"));
|
||||
if (check_and_set_terms(terms, p))
|
||||
return -1;
|
||||
|
||||
@@ -1383,7 +1384,8 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref
|
||||
if (argc)
|
||||
return error(_("'%s' requires 0 arguments"),
|
||||
"git bisect next");
|
||||
get_terms(&terms);
|
||||
if (get_terms(&terms, 1))
|
||||
return error(_("no terms defined"));
|
||||
res = bisect_next(&terms, prefix);
|
||||
free_terms(&terms);
|
||||
return res;
|
||||
@@ -1417,7 +1419,8 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS
|
||||
struct bisect_terms terms = { 0 };
|
||||
|
||||
set_terms(&terms, "bad", "good");
|
||||
get_terms(&terms);
|
||||
if (get_terms(&terms, 1))
|
||||
return error(_("no terms defined"));
|
||||
res = bisect_skip(&terms, argc, argv);
|
||||
free_terms(&terms);
|
||||
return res;
|
||||
@@ -1429,7 +1432,8 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix
|
||||
int res;
|
||||
struct bisect_terms terms = { 0 };
|
||||
|
||||
get_terms(&terms);
|
||||
if (get_terms(&terms, 1))
|
||||
return error(_("no terms defined"));
|
||||
res = bisect_visualize(&terms, argc, argv);
|
||||
free_terms(&terms);
|
||||
return res;
|
||||
@@ -1443,7 +1447,8 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE
|
||||
|
||||
if (!argc)
|
||||
return error(_("'%s' failed: no command provided."), "git bisect run");
|
||||
get_terms(&terms);
|
||||
if (get_terms(&terms, 1))
|
||||
return error(_("no terms defined"));
|
||||
res = bisect_run(&terms, argc, argv);
|
||||
free_terms(&terms);
|
||||
return res;
|
||||
@@ -1482,7 +1487,8 @@ int cmd_bisect(int argc,
|
||||
usage_with_options(git_bisect_usage, options);
|
||||
|
||||
set_terms(&terms, "bad", "good");
|
||||
get_terms(&terms);
|
||||
if (get_terms(&terms, 1))
|
||||
return error(_("no terms defined"));
|
||||
if (check_and_set_terms(&terms, argv[0]) ||
|
||||
!one_of(argv[0], terms.term_good, terms.term_bad, NULL))
|
||||
usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
|
||||
|
||||
Reference in New Issue
Block a user