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

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> 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/xmqqh5m1qcfh.fsf@gitster.g/

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);
 		goto finish;
 	}
 	terms->term_good = strbuf_detach(&str, NULL);