Re: [PATCH 10/11] bisect: check get_terms return at all call sites

[Patrick Steinhardt <ps@pks.im>]

On Tue, Jul 14, 2026 at 10:48:43PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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.
> 
> Add checks at each call site so that a failed get_terms produces a
> clear "no terms defined" error, matching the pattern already used
> in bisect_terms() at line 512. The check tests the term pointers
> rather than the return value because some callers (bisect skip,
> legacy bad/good) call set_terms before get_terms, and the
> set_terms values should survive a get_terms failure.

Hm. Are there any callers that accept the case where either `term->bad`
or `term->good` are `NULL`? If not, should we maybe adapt the function
itself to return an error if so and then have all callers only ever
check for the return value of `get_term()` instead of also having to
check the result? That might also allow us to deduplicate the error
messages.

Patrick