[PATCH v3 10/12] bisect: check strbuf_getline_lf return when reading terms

"Johannes Schindelin via GitGitGadget" <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <7db6ac2ab0d346da992b812c8588d90b16aebb49.1786521801.git.gitgitgadget@gmail.com>
From: Johannes Schindelin <[email protected]>

get_terms() in builtin/bisect.c and read_bisect_terms() in
bisect.c both read the BISECT_TERMS file but do not check the
strbuf_getline_lf() return values. If the file is truncated
(e.g., a partial write from a crash or disk-full condition),
strbuf_getline_lf returns EOF and the strbuf remains empty.
strbuf_detach then returns an empty string, and the term names
silently become "" instead of the expected "bad"/"good" or
custom terms.

In get_terms(), check for EOF and return -1 on truncation,
matching the existing -1 return for a missing file.

In read_bisect_terms(), die with a descriptive message when a
line cannot be read, consistent with the die_errno for a
non-ENOENT open failure in the same function. Unlike get_terms(),
read_bisect_terms() returns void and uses die() for all error
paths, so the die is the appropriate error handling here.

Pointed out by Coverity.

Assisted-by: Claude Opus 4.6
Helped-by: Junio C Hamano <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
---
 bisect.c         |  6 ++++--
 builtin/bisect.c | 11 +++++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/bisect.c b/bisect.c
index 94c7028d2a..c2ef5da462 100644
--- a/bisect.c
+++ b/bisect.c
@@ -1019,10 +1019,12 @@ void read_bisect_terms(char **read_bad, char **read_good)
 			die_errno(_("could not read file '%s'"), filename);
 		}
 	} else {
-		strbuf_getline_lf(&str, fp);
+		if (strbuf_getline_lf(&str, fp) == EOF)
+			die(_("could not read bad term from file '%s'"), filename);
 		free(*read_bad);
 		*read_bad = strbuf_detach(&str, NULL);
-		strbuf_getline_lf(&str, fp);
+		if (strbuf_getline_lf(&str, fp) == EOF)
+			die(_("could not read good term from file '%s'"), filename);
 		free(*read_good);
 		*read_good = strbuf_detach(&str, NULL);
 	}
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 798e28f501..69ab7ea248 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -498,9 +498,16 @@ 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;
+		FREE_AND_NULL(terms->term_bad);
+		goto finish;
+	}
 	terms->term_good = strbuf_detach(&str, NULL);
 
 finish:
-- 
gitgitgadget
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.