Re: [PATCH 09/11] bisect: check strbuf_getline_lf return when reading terms

Hi Junio,

On Sun, 19 Jul 2026, Junio C Hamano wrote:

> Junio C Hamano <[email protected]> writes:
> 
> > "Johannes Schindelin via GitGitGadget" <[email protected]>
> > writes:
> > ...
> >> diff --git a/builtin/bisect.c b/builtin/bisect.c
> >> index 798e28f501..fe66d84382 100644
> >> --- a/builtin/bisect.c
> >> +++ b/builtin/bisect.c
> >> @@ -498,9 +498,15 @@ static int get_terms(struct bisect_terms *terms)
> >>  	}
> >>  
> >>  	free_terms(terms);
> >> -	strbuf_getline_lf(&str, fp);
> >> +	if (strbuf_getline_lf(&str, fp) == EOF) {
> >> +		res = -1;
> >> +		goto finish;
> >> +	}
> >>  	terms->term_bad = strbuf_detach(&str, NULL);
> >> -	strbuf_getline_lf(&str, fp);
> >> +	if (strbuf_getline_lf(&str, fp) == EOF) {
> >> +		res = -1;
> >> +		goto finish;
> >> +	}
> >
> > We want to clean-up terms->term_bad when we fail to read the second
> > line after reading the first line successfully, no?
> >
> >>  	terms->term_good = strbuf_detach(&str, NULL);
> >>  
> >>  finish:
> 
> --- >8 ---
> Subject: [PATCH] fixup! bisect: check strbuf_getline_lf return when reading
>  terms
> 
> https://lore.kernel.org/git/[email protected]/
> 
> This fixes the immediate leak introduced by
> 
> https://lore.kernel.org/git/17c382fdf46eada79ce03a7604dd7e0454d8bea4.1784069325.git.gitgitgadget@gmail.com/
> 
> but many callers of get_terms() should all be fixed to check for
> return value.  If it fails to grab the replacement word for "bad",
> both terms->term_bad and terms->term_good are left NULL, since the
> function calls free_terms() early.
> 
> diff --git a/builtin/bisect.c b/builtin/bisect.c
> index fe66d84382..69ab7ea248 100644
> --- a/builtin/bisect.c
> +++ b/builtin/bisect.c
> @@ -505,6 +505,7 @@ static int get_terms(struct bisect_terms *terms)
>  	terms->term_bad = strbuf_detach(&str, NULL);
>  	if (strbuf_getline_lf(&str, fp) == EOF) {
>  		res = -1;
> +		FREE_AND_NULL(terms->term_bad);

Good catch!

Thank you,
Johannes

>  		goto finish;
>  	}
>  	terms->term_good = strbuf_detach(&str, NULL);
>