Merge branch 'js/coverity-unchecked-returns-fix' into jch

A handful of code paths have been corrected to check return values
from functions like curl_easy_duphandle(), deflateInit(), lseek(),
dup(), and strbuf_getline_lf(), resolving several Coverity warnings
about unchecked returns.

* js/coverity-unchecked-returns-fix:
  bisect: handle dup() failure when redirecting stdout
  bisect: check get_terms return at all call sites
  bisect: check strbuf_getline_lf return when reading terms
  transport-helper: warn when export-marks file cannot be finalized
  transport-helper: check dup() return in get_exporter
  compat/pread: check initial lseek for errors
  last-modified: handle repo_parse_commit() failures
  reftable tests: check reftable_table_init_ref_iterator() return
  reftable/block: check deflateInit() return value
  config: propagate launch_editor() failure in show_editor()
  http: die on curl_easy_duphandle failure in get_active_slot

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);
 	}

builtin/bisect.c

@@ -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,14 +493,21 @@ 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;
 	}
 
 	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:
@@ -512,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 +1064,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;
 
@@ -1307,7 +1315,12 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
 
 		fflush(stdout);
 		saved_stdout = dup(1);
-		dup2(temporary_stdout_fd, 1);
+		if (saved_stdout < 0 ||
+		    dup2(temporary_stdout_fd, 1) < 0) {
+			res = error_errno(_("could not duplicate stdout"));
+			close(temporary_stdout_fd);
+			break;
+		}
 
 		res = bisect_state(terms, 1, &new_state);
 
@@ -1383,7 +1396,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 +1431,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 +1444,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 +1459,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 +1499,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,

builtin/config.c

@@ -1313,7 +1313,10 @@ static int show_editor(struct config_location_options *opts)
 		else if (errno != EEXIST)
 			die_errno(_("cannot create configuration file %s"), config_file);
 	}
-	launch_editor(config_file, NULL, NULL);
+	if (launch_editor(config_file, NULL, NULL)) {
+		free(config_file);
+		return -1;
+	}
 	free(config_file);
 
 	return 0;

builtin/last-modified.c

@@ -290,7 +290,8 @@ static void process_parent(struct last_modified *lm,
 {
 	struct bitmap *active_p;
 
-	repo_parse_commit(lm->rev.repo, parent);
+	if (repo_parse_commit(lm->rev.repo, parent))
+		return;
 	active_p = active_paths_for(lm, parent);
 
 	/*
@@ -414,12 +415,14 @@ static int last_modified_run(struct last_modified *lm)
 		 * Otherwise, make sure that 'c' isn't reachable from anything
 		 * in the '--not' queue.
 		 */
-		repo_parse_commit(lm->rev.repo, c);
+		if (repo_parse_commit(lm->rev.repo, c))
+			goto cleanup;
 
 		while ((n = prio_queue_get(&not_queue))) {
 			struct commit_list *np;
 
-			repo_parse_commit(lm->rev.repo, n);
+			if (repo_parse_commit(lm->rev.repo, n))
+				continue;
 
 			for (np = n->parents; np; np = np->next) {
 				if (!(np->item->object.flags & PARENT2)) {

compat/pread.c

@@ -7,6 +7,8 @@ ssize_t git_pread(int fd, void *buf, size_t count, off_t offset)
         ssize_t rc;
 
         current_offset = lseek(fd, 0, SEEK_CUR);
+	if (current_offset < 0)
+		return -1;
 
         if (lseek(fd, offset, SEEK_SET) < 0)
                 return -1;

http.c

@@ -1608,6 +1608,8 @@ struct active_request_slot *get_active_slot(void)
 
 	if (!slot->curl) {
 		slot->curl = curl_easy_duphandle(curl_default);
+		if (!slot->curl)
+			die("curl_easy_duphandle failed");
 		curl_session_count++;
 	}
 

reftable/block.c

@@ -87,7 +87,8 @@ int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,
 		REFTABLE_CALLOC_ARRAY(bw->zstream, 1);
 		if (!bw->zstream)
 			return REFTABLE_OUT_OF_MEMORY_ERROR;
-		deflateInit(bw->zstream, 9);
+		if (deflateInit(bw->zstream, 9) != Z_OK)
+			return REFTABLE_ZLIB_ERROR;
 	}
 
 	return 0;

t/unit-tests/u-reftable-table.c

@@ -32,7 +32,8 @@ void test_reftable_table__seek_once(void)
 	ret = reftable_table_new(&table, &source, "name");
 	cl_assert(!ret);
 
-	reftable_table_init_ref_iterator(table, &it);
+	ret = reftable_table_init_ref_iterator(table, &it);
+	cl_assert_equal_i(ret, 0);
 	ret = reftable_iterator_seek_ref(&it, "");
 	cl_assert(!ret);
 	ret = reftable_iterator_next_ref(&it, &ref);
@@ -74,7 +75,8 @@ void test_reftable_table__reseek(void)
 	ret = reftable_table_new(&table, &source, "name");
 	cl_assert(!ret);
 
-	reftable_table_init_ref_iterator(table, &it);
+	ret = reftable_table_init_ref_iterator(table, &it);
+	cl_assert_equal_i(ret, 0);
 
 	for (size_t i = 0; i < 5; i++) {
 		ret = reftable_iterator_seek_ref(&it, "");

transport-helper.c

@@ -487,6 +487,8 @@ static int get_exporter(struct transport *transport,
 	/* we need to duplicate helper->in because we want to use it after
 	 * fastexport is done with it. */
 	fastexport->out = dup(helper->in);
+	if (fastexport->out < 0)
+		return error_errno(_("could not dup helper output fd"));
 	strvec_push(&fastexport->args, "fast-export");
 	strvec_push(&fastexport->args, "--use-done-feature");
 	strvec_push(&fastexport->args, data->signed_tags ?
@@ -1191,7 +1193,9 @@ static int push_refs_with_export(struct transport *transport,
 
 	if (data->export_marks) {
 		strbuf_addf(&buf, "%s.tmp", data->export_marks);
-		rename(buf.buf, data->export_marks);
+		if (rename(buf.buf, data->export_marks))
+			warning_errno(_("could not rename '%s' to '%s'"),
+				      buf.buf, data->export_marks);
 		strbuf_release(&buf);
 	}