{"thread":{"id":"65998","subject":"[PATCH 00/11] coverity: fix unchecked returns","startedAt":"2026-07-14T22:48:47Z","lastAt":"2026-08-13T06:31:51Z","messageCount":60,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Patrick Steinhardt","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"548173","messageId":"pull.2179.git.1784069325.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":null,"subject":"[PATCH 00/11] coverity: fix unchecked returns","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:33Z","receivedAt":"2026-07-14T22:48:47Z","isPatch":true,"body":"This is the next batch of fixes in response to issues reported by Coverity.\n\nJohannes Schindelin (11):\n http: die on curl_easy_duphandle failure in get_active_slot\n config: propagate launch_editor() failure in show_editor()\n reftable/block: check deflateInit() return value\n reftable tests: check reftable_table_init_ref_iterator() return\n last-modified: handle repo_parse_commit() failures\n compat/pread: check initial lseek for errors\n transport-helper: check dup() return in get_exporter\n transport-helper: warn when export-marks file cannot be finalized\n bisect: check strbuf_getline_lf return when reading terms\n bisect: check get_terms return at all call sites\n bisect: handle dup() failure when redirecting stdout\n\n bisect.c | 6 ++++--\n builtin/bisect.c | 27 +++++++++++++++++++++++++--\n builtin/config.c | 5 ++++-\n builtin/last-modified.c | 9 ++++++---\n compat/pread.c | 2 ++\n http.c | 2 ++\n reftable/block.c | 3 ++-\n t/unit-tests/u-reftable-table.c | 6 ++++--\n transport-helper.c | 6 +++++-\n 9 files changed, 54 insertions(+), 12 deletions(-)\n\n\nbase-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2179%2Fdscho%2Fcoverity-fixes-unchecked-returns-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2179/dscho/coverity-fixes-unchecked-returns-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2179\n-- \ngitgitgadget\n"},{"id":"548174","messageId":"e653255de19decfe45d4ef8d3277aaf69c44c391.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 01/11] http: die on curl_easy_duphandle failure in get_active_slot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:34Z","receivedAt":"2026-07-14T22:48:50Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_active_slot() duplicates the default curl handle via\ncurl_easy_duphandle() to create a per-slot session handle. The\nreturn value is stored directly in slot->curl without checking\nfor NULL. curl_easy_duphandle() can return NULL when memory\nallocation fails internally, and the libcurl documentation\nexplicitly states this possibility.\n\nWhen this happens, slot->curl is NULL and the very next operation\n(curl_easy_setopt on line 1632 for CURLOPT_COOKIEFILE) passes\nNULL as the curl handle, which is undefined behavior in libcurl\nand typically crashes.\n\nEvery HTTP operation in git goes through get_active_slot(), so\nthis affects all remote-https, remote-http, and HTTP-based\noperations (clone, fetch, push over HTTP, bundle-uri downloads).\n\nAdd a NULL check and die() with a clear message. There is no\nreasonable recovery from a failed handle duplication: the process\nis out of memory and cannot perform any HTTP operation.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n http.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex b4e7b8d00b..8f1d6d1f56 100644\n--- a/http.c\n+++ b/http.c\n@@ -1608,6 +1608,8 @@ struct active_request_slot *get_active_slot(void)\n \n \tif (!slot->curl) {\n \t\tslot->curl = curl_easy_duphandle(curl_default);\n+\t\tif (!slot->curl)\n+\t\t\tdie(\"curl_easy_duphandle failed\");\n \t\tcurl_session_count++;\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"548175","messageId":"0692704d45060a62579b50dd7a2f07da04f435c8.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 02/11] config: propagate launch_editor() failure in show_editor()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:35Z","receivedAt":"2026-07-14T22:48:52Z","isPatch":true,"body":"From: Johannes Schindelin \n\nshow_editor() calls launch_editor() to open the user's editor on\nthe configuration file, but discards the return value and\nunconditionally returns 0 (success). When the editor fails to\nlaunch (e.g., $EDITOR is not found, or the editor exits with a\nnonzero status), the caller receives no indication that anything\nwent wrong.\n\nThis affects \"git config edit\" and \"git config --edit\": the\ncommand silently succeeds even when the editor could not be\nstarted. In contrast, other editor-launching paths in git (such\nas \"git commit\" and \"git rebase --edit-todo\") properly propagate\neditor failures and exit with an error.\n\nCheck the return value and propagate the failure by returning -1.\nThe two callers (cmd_config_edit at line 1315 and the legacy\ncmd_config at line 1478) both propagate this return to\nhandle_builtin, which translates negative returns into an error\nexit.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/config.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 8d8ec0beea..1307fdb0d6 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -1313,7 +1313,10 @@ static int show_editor(struct config_location_options *opts)\n \t\telse if (errno != EEXIST)\n \t\t\tdie_errno(_(\"cannot create configuration file %s\"), config_file);\n \t}\n-\tlaunch_editor(config_file, NULL, NULL);\n+\tif (launch_editor(config_file, NULL, NULL)) {\n+\t\tfree(config_file);\n+\t\treturn -1;\n+\t}\n \tfree(config_file);\n \n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"548176","messageId":"9bf7e737c740d8a80467ee3b38df9c86bbf7a566.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 03/11] reftable/block: check deflateInit() return value","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:36Z","receivedAt":"2026-07-14T22:48:54Z","isPatch":true,"body":"From: Johannes Schindelin \n\nblock_writer_init() allocates a z_stream and calls deflateInit()\nto prepare it for compressing log records. The return value of\ndeflateInit() is silently discarded. If zlib initialization fails\n(e.g., Z_MEM_ERROR when the system is under memory pressure), the\nz_stream is left in an undefined state.\n\nSubsequent deflate() calls in block_writer_finish() then operate\non this uninitialized stream. Depending on the zlib\nimplementation, this can produce silently corrupted compressed\ndata (which would be written to the reftable file and discovered\nonly when a later reader fails to inflate) or crash outright.\n\nThe function already uses REFTABLE_ZLIB_ERROR for deflate()\nfailures later in the code path (lines 171, 199), so returning\nthe same error code for deflateInit() failure is consistent.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n reftable/block.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/block.c b/reftable/block.c\nindex 920b3f4486..ec81fd0493 100644\n--- a/reftable/block.c\n+++ b/reftable/block.c\n@@ -87,7 +87,8 @@ int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,\n \t\tREFTABLE_CALLOC_ARRAY(bw->zstream, 1);\n \t\tif (!bw->zstream)\n \t\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n-\t\tdeflateInit(bw->zstream, 9);\n+\t\tif (deflateInit(bw->zstream, 9) != Z_OK)\n+\t\t\treturn REFTABLE_ZLIB_ERROR;\n \t}\n \n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"548177","messageId":"711671c3abac64d9bb0872a69d45df4f103afc66.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 04/11] reftable tests: check reftable_table_init_ref_iterator() return","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:37Z","receivedAt":"2026-07-14T22:48:56Z","isPatch":true,"body":"From: Johannes Schindelin \n\ntest_reftable_table__seek_once() and test_reftable_table__reseek()\nboth call reftable_table_init_ref_iterator() without checking its\nreturn value. This function returns an int error code (0 on\nsuccess, negative on failure). Every other reftable function call\nin these same tests checks the return via cl_assert_equal_i() or\ncl_assert(), making this omission inconsistent.\n\nIf the iterator initialization ever fails (e.g., due to a memory\nallocation failure in the reftable internals), the test would\nproceed to seek and read with an uninitialized iterator, producing\nmisleading test results or crashes rather than a clear assertion\nfailure.\n\nCheck the return value via cl_assert_equal_i(ret, 0), consistent\nwith the surrounding code.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n t/unit-tests/u-reftable-table.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c\nindex fae478ee04..6f444f8cf9 100644\n--- a/t/unit-tests/u-reftable-table.c\n+++ b/t/unit-tests/u-reftable-table.c\n@@ -29,7 +29,8 @@ void test_reftable_table__seek_once(void)\n \tret = reftable_table_new(&table, &source, \"name\");\n \tcl_assert(!ret);\n \n-\treftable_table_init_ref_iterator(table, &it);\n+\tret = reftable_table_init_ref_iterator(table, &it);\n+\tcl_assert_equal_i(ret, 0);\n \tret = reftable_iterator_seek_ref(&it, \"\");\n \tcl_assert(!ret);\n \tret = reftable_iterator_next_ref(&it, &ref);\n@@ -71,7 +72,8 @@ void test_reftable_table__reseek(void)\n \tret = reftable_table_new(&table, &source, \"name\");\n \tcl_assert(!ret);\n \n-\treftable_table_init_ref_iterator(table, &it);\n+\tret = reftable_table_init_ref_iterator(table, &it);\n+\tcl_assert_equal_i(ret, 0);\n \n \tfor (size_t i = 0; i < 5; i++) {\n \t\tret = reftable_iterator_seek_ref(&it, \"\");\n-- \ngitgitgadget\n\n"},{"id":"548178","messageId":"f728be4dacb0b9781ef6589a0d2c48009aa31e9e.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 05/11] last-modified: handle repo_parse_commit() failures","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:38Z","receivedAt":"2026-07-14T22:48:57Z","isPatch":true,"body":"From: Johannes Schindelin \n\nlast_modified_run() and process_parent() call repo_parse_commit()\nwithout checking the return value at three sites. When a commit\nobject is corrupt or unavailable (e.g., a shallow clone boundary\nor a missing object in a partial clone), the parse fails and the\ncommit's internal fields (parents, tree, date) are not populated.\n\nThe consequences depend on which call site fails:\n\nAt line 417 (the main walk loop), c->parents stays NULL after a\nfailed parse. The parent-walking loop at line 440 simply does not\nexecute, silently treating the unparsable commit as a root commit.\nThis produces incorrect \"last modified\" results: paths changed in\nancestors beyond the corrupt commit are attributed to the wrong\ncommit or not reported at all.\n\nAt line 423 (the --not exclusion walk), n->parents stays NULL,\ncausing the exclusion walk to stop prematurely. Commits that\nshould be excluded from the output may be incorrectly included.\n\nAt line 293 (process_parent), the parent's tree and parents are\nunavailable, so diff operations against it produce wrong results\nand the parent's own ancestors are never enqueued for walking.\n\nSkip unparsable commits by checking the return value and\ncontinuing to the next iteration (or returning early in\nprocess_parent). This matches the defensive pattern used in other\nrevision walkers such as limit_list() and get_revision_internal().\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/last-modified.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 5478182f2e..fe012b0c2e 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -290,7 +290,8 @@ static void process_parent(struct last_modified *lm,\n {\n \tstruct bitmap *active_p;\n \n-\trepo_parse_commit(lm->rev.repo, parent);\n+\tif (repo_parse_commit(lm->rev.repo, parent))\n+\t\treturn;\n \tactive_p = active_paths_for(lm, parent);\n \n \t/*\n@@ -414,12 +415,14 @@ static int last_modified_run(struct last_modified *lm)\n \t\t * Otherwise, make sure that 'c' isn't reachable from anything\n \t\t * in the '--not' queue.\n \t\t */\n-\t\trepo_parse_commit(lm->rev.repo, c);\n+\t\tif (repo_parse_commit(lm->rev.repo, c))\n+\t\t\tcontinue;\n \n \t\twhile ((n = prio_queue_get(¬_queue))) {\n \t\t\tstruct commit_list *np;\n \n-\t\t\trepo_parse_commit(lm->rev.repo, n);\n+\t\t\tif (repo_parse_commit(lm->rev.repo, n))\n+\t\t\t\tcontinue;\n \n \t\t\tfor (np = n->parents; np; np = np->next) {\n \t\t\t\tif (!(np->item->object.flags & PARENT2)) {\n-- \ngitgitgadget\n\n"},{"id":"548179","messageId":"b31e0326e7c4f97753c80077c8f0927504f40370.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 06/11] compat/pread: check initial lseek for errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:39Z","receivedAt":"2026-07-14T22:48:59Z","isPatch":true,"body":"From: Johannes Schindelin \n\ngit_pread() saves the current file offset via lseek(fd, 0,\nSEEK_CUR) and later restores it. If the initial lseek fails\n(e.g., the fd is a pipe or otherwise non-seekable),\ncurrent_offset is -1. This negative value is later passed to\nlseek(fd, -1, SEEK_SET) at line 16, which sets the file position\nto an unintended location (or fails with EINVAL on some\nplatforms).\n\nCheck the initial lseek return value and return -1 immediately\nif it fails, consistent with the error handling for the other\nlseek calls in the same function.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n compat/pread.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/compat/pread.c b/compat/pread.c\nindex 484e6d4c71..ac7d058cb8 100644\n--- a/compat/pread.c\n+++ b/compat/pread.c\n@@ -7,6 +7,8 @@ ssize_t git_pread(int fd, void *buf, size_t count, off_t offset)\n ssize_t rc;\n \n current_offset = lseek(fd, 0, SEEK_CUR);\n+\tif (current_offset < 0)\n+\t\treturn -1;\n \n if (lseek(fd, offset, SEEK_SET) < 0)\n return -1;\n-- \ngitgitgadget\n\n"},{"id":"548180","messageId":"1792042098cd50ba164b90e5ce62430037661343.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 07/11] transport-helper: check dup() return in get_exporter","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:40Z","receivedAt":"2026-07-14T22:49:01Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_exporter() duplicates helper->in via dup() and stores the\nresult in fastexport->out. If dup() fails (fd exhaustion), it\nreturns -1. The child_process machinery interprets out = -1 as\n\"create a pipe for stdout\", which would silently change the\nfast-export process's output wiring: instead of sending data\nback through the helper's input fd, it would write to a new pipe\nthat nobody reads from.\n\nCheck the return value and report the error before proceeding.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n transport-helper.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 80f90eb7ba..31883b244e 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -487,6 +487,8 @@ static int get_exporter(struct transport *transport,\n \t/* we need to duplicate helper->in because we want to use it after\n \t * fastexport is done with it. */\n \tfastexport->out = dup(helper->in);\n+\tif (fastexport->out < 0)\n+\t\treturn error_errno(_(\"could not dup helper output fd\"));\n \tstrvec_push(&fastexport->args, \"fast-export\");\n \tstrvec_push(&fastexport->args, \"--use-done-feature\");\n \tstrvec_push(&fastexport->args, data->signed_tags ?\n-- \ngitgitgadget\n\n"},{"id":"548181","messageId":"13ddcce053921d3fc8f97deb0dd884ae4667abd3.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 08/11] transport-helper: warn when export-marks file cannot be finalized","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:41Z","receivedAt":"2026-07-14T22:49:03Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen push_refs_with_export() finalizes a successful push, it writes\nthe fast-export marks file to a .tmp sibling and rename()s it into\nplace. The return value of rename() is currently ignored. If the\nrename fails (permission denied, full disk, or an antivirus product\nlocking the destination on Windows), the .tmp file is left behind\nand the existing export_marks file remains stale; the next\nfast-export operation that resumes from it then silently operates on\ninconsistent bookkeeping.\n\nThe push itself succeeded by that point, so promoting this to a\nfatal error would be inappropriate. Emit warning_errno() naming both\npaths so the user can recover manually, and keep returning 0.\n\nFlagged by Coverity as CID 1427723 (\"Unchecked return value\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n transport-helper.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 31883b244e..ed0543f1ad 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -1184,7 +1184,9 @@ static int push_refs_with_export(struct transport *transport,\n \n \tif (data->export_marks) {\n \t\tstrbuf_addf(&buf, \"%s.tmp\", data->export_marks);\n-\t\trename(buf.buf, data->export_marks);\n+\t\tif (rename(buf.buf, data->export_marks))\n+\t\t\twarning_errno(_(\"could not rename '%s' to '%s'\"),\n+\t\t\t\t buf.buf, data->export_marks);\n \t\tstrbuf_release(&buf);\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"548182","messageId":"17c382fdf46eada79ce03a7604dd7e0454d8bea4.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 09/11] bisect: check strbuf_getline_lf return when reading terms","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:42Z","receivedAt":"2026-07-14T22:49:06Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_terms() in builtin/bisect.c and read_bisect_terms() in\nbisect.c both read the BISECT_TERMS file but do not check the\nstrbuf_getline_lf() return values. If the file is truncated\n(e.g., a partial write from a crash or disk-full condition),\nstrbuf_getline_lf returns EOF and the strbuf remains empty.\nstrbuf_detach then returns an empty string, and the term names\nsilently become \"\" instead of the expected \"bad\"/\"good\" or\ncustom terms.\n\nIn get_terms(), check for EOF and return -1 on truncation,\nmatching the existing -1 return for a missing file.\n\nIn read_bisect_terms(), die with a descriptive message when a\nline cannot be read, consistent with the die_errno for a\nnon-ENOENT open failure in the same function. Unlike get_terms(),\nread_bisect_terms() returns void and uses die() for all error\npaths, so the die is the appropriate error handling here.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n bisect.c | 6 ++++--\n builtin/bisect.c | 10 ++++++++--\n 2 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 94c7028d2a..c2ef5da462 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1019,10 +1019,12 @@ void read_bisect_terms(char **read_bad, char **read_good)\n \t\t\tdie_errno(_(\"could not read file '%s'\"), filename);\n \t\t}\n \t} else {\n-\t\tstrbuf_getline_lf(&str, fp);\n+\t\tif (strbuf_getline_lf(&str, fp) == EOF)\n+\t\t\tdie(_(\"could not read bad term from file '%s'\"), filename);\n \t\tfree(*read_bad);\n \t\t*read_bad = strbuf_detach(&str, NULL);\n-\t\tstrbuf_getline_lf(&str, fp);\n+\t\tif (strbuf_getline_lf(&str, fp) == EOF)\n+\t\t\tdie(_(\"could not read good term from file '%s'\"), filename);\n \t\tfree(*read_good);\n \t\t*read_good = strbuf_detach(&str, NULL);\n \t}\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 798e28f501..fe66d84382 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -498,9 +498,15 @@ static int get_terms(struct bisect_terms *terms)\n \t}\n \n \tfree_terms(terms);\n-\tstrbuf_getline_lf(&str, fp);\n+\tif (strbuf_getline_lf(&str, fp) == EOF) {\n+\t\tres = -1;\n+\t\tgoto finish;\n+\t}\n \tterms->term_bad = strbuf_detach(&str, NULL);\n-\tstrbuf_getline_lf(&str, fp);\n+\tif (strbuf_getline_lf(&str, fp) == EOF) {\n+\t\tres = -1;\n+\t\tgoto finish;\n+\t}\n \tterms->term_good = strbuf_detach(&str, NULL);\n \n finish:\n-- \ngitgitgadget\n\n"},{"id":"548183","messageId":"c0827a79476d02f2b09ded919b44860e3743fbe0.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 10/11] bisect: check get_terms return at all call sites","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:43Z","receivedAt":"2026-07-14T22:49:10Z","isPatch":true,"body":"From: Johannes Schindelin \n\nSix callers of get_terms() silently discard its return value. When\nget_terms fails (missing or truncated BISECT_TERMS file), the term\nstrings remain NULL or empty, causing confusing downstream\nbehavior: commands like \"bisect next\" or \"bisect run\" proceed with\nempty term strings, producing nonsensical ref names (refs/bisect/\nwith no suffix) and misleading error messages.\n\nAdd checks at each call site so that a failed get_terms produces a\nclear \"no terms defined\" error, matching the pattern already used\nin bisect_terms() at line 512. The check tests the term pointers\nrather than the return value because some callers (bisect skip,\nlegacy bad/good) call set_terms before get_terms, and the\nset_terms values should survive a get_terms failure.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex fe66d84382..15a2a30f89 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1057,6 +1057,8 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n \t*word_end = '\\0'; /* NUL-terminate the word */\n \n \tget_terms(terms);\n+\tif (!terms->term_bad || !terms->term_good)\n+\t\treturn error(_(\"no terms defined\"));\n \tif (check_and_set_terms(terms, p))\n \t\treturn -1;\n \n@@ -1383,6 +1385,8 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref\n \t\treturn error(_(\"'%s' requires 0 arguments\"),\n \t\t\t \"git bisect next\");\n \tget_terms(&terms);\n+\tif (!terms.term_bad || !terms.term_good)\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_next(&terms, prefix);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1417,6 +1421,8 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS\n \n \tset_terms(&terms, \"bad\", \"good\");\n \tget_terms(&terms);\n+\tif (!terms.term_bad || !terms.term_good)\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_skip(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1429,6 +1435,8 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix\n \tstruct bisect_terms terms = { 0 };\n \n \tget_terms(&terms);\n+\tif (!terms.term_bad || !terms.term_good)\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_visualize(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1443,6 +1451,8 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE\n \tif (!argc)\n \t\treturn error(_(\"'%s' failed: no command provided.\"), \"git bisect run\");\n \tget_terms(&terms);\n+\tif (!terms.term_bad || !terms.term_good)\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_run(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1482,6 +1492,8 @@ int cmd_bisect(int argc,\n \n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tget_terms(&terms);\n+\t\tif (!terms.term_bad || !terms.term_good)\n+\t\t\treturn error(_(\"no terms defined\"));\n \t\tif (check_and_set_terms(&terms, argv[0]) ||\n \t\t !one_of(argv[0], terms.term_good, terms.term_bad, NULL))\n \t\t\tusage_msg_optf(_(\"unknown command: '%s'\"), git_bisect_usage,\n-- \ngitgitgadget\n\n"},{"id":"548184","messageId":"2da452e39cbe1bd53da9d76fa7f7615c1a453634.1784069325.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH 11/11] bisect: handle dup() failure when redirecting stdout","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-14T22:48:44Z","receivedAt":"2026-07-14T22:49:13Z","isPatch":true,"body":"From: Johannes Schindelin \n\nTo capture the output of each verdict command, bisect_run()\ntemporarily redirects stdout to a temporary file via the classic\ndup(1) / dup2() pair, restoring it afterwards. The return value of\ndup(1) is not checked, however. When it fails, the saved descriptor\nis -1, which is then passed to close() (the issue Coverity flags),\nand the matching dup2() that is meant to restore stdout also fails,\nleaving the process with stdout still pointing at the temporary file\nfor the remainder of the run.\n\nTreat a failed dup(1) as a fatal error for this bisect step: close\nthe temporary file descriptor, report the error via error_errno(),\nand break out of the loop so the existing cleanup path handles the\nrest, just as on other failure paths in this function.\n\nReported by Coverity as CID 1508242 (\"Improper use of negative\nvalue\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 15a2a30f89..801daf8c78 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1308,6 +1308,11 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n \n \t\tfflush(stdout);\n \t\tsaved_stdout = dup(1);\n+\t\tif (saved_stdout < 0) {\n+\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n+\t\t\tclose(temporary_stdout_fd);\n+\t\t\tbreak;\n+\t\t}\n \t\tdup2(temporary_stdout_fd, 1);\n \n \t\tres = bisect_state(terms, 1, &new_state);\n-- \ngitgitgadget\n"},{"id":"548189","messageId":"xmqqldbdqciy.fsf@gitster.g","threadId":"65998","inReplyTo":"f728be4dacb0b9781ef6589a0d2c48009aa31e9e.1784069325.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 05/11] last-modified: handle repo_parse_commit() failures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-15T01:15:01Z","receivedAt":"2026-07-15T01:15:07Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Skip unparsable commits by checking the return value and\n> continuing to the next iteration (or returning early in\n> process_parent). This matches the defensive pattern used in other\n> revision walkers such as limit_list() and get_revision_internal().\n> ...\n> @@ -414,12 +415,14 @@ static int last_modified_run(struct last_modified *lm)\n> \t\t * Otherwise, make sure that 'c' isn't reachable from anything\n> \t\t * in the '--not' queue.\n> \t\t */\n> -\t\trepo_parse_commit(lm->rev.repo, c);\n> +\t\tif (repo_parse_commit(lm->rev.repo, c))\n> +\t\t\tcontinue;\n\nShouldn't this be\n\n\t\t\tgoto cleanup;\n\ninstead? 'n' pulled out of not_queue may be unparseable and when we\nignore it, don't we still want to clean up the active_paths slab for\ncommit 'c'?\n\n> \t\twhile ((n = prio_queue_get(¬_queue))) {\n> \t\t\tstruct commit_list *np;\n> \n> -\t\t\trepo_parse_commit(lm->rev.repo, n);\n> +\t\t\tif (repo_parse_commit(lm->rev.repo, n))\n> +\t\t\t\tcontinue;\n> \n> \t\t\tfor (np = n->parents; np; np = np->next) {\n> \t\t\t\tif (!(np->item->object.flags & PARENT2)) {\n"},{"id":"548190","messageId":"xmqqh5m1qcfh.fsf@gitster.g","threadId":"65998","inReplyTo":"17c382fdf46eada79ce03a7604dd7e0454d8bea4.1784069325.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 09/11] bisect: check strbuf_getline_lf return when reading terms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-15T01:17:06Z","receivedAt":"2026-07-15T01:17:08Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index 798e28f501..fe66d84382 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -498,9 +498,15 @@ static int get_terms(struct bisect_terms *terms)\n> \t}\n> \n> \tfree_terms(terms);\n> -\tstrbuf_getline_lf(&str, fp);\n> +\tif (strbuf_getline_lf(&str, fp) == EOF) {\n> +\t\tres = -1;\n> +\t\tgoto finish;\n> +\t}\n> \tterms->term_bad = strbuf_detach(&str, NULL);\n> -\tstrbuf_getline_lf(&str, fp);\n> +\tif (strbuf_getline_lf(&str, fp) == EOF) {\n> +\t\tres = -1;\n> +\t\tgoto finish;\n> +\t}\n\nWe want to clean-up terms->term_bad when we fail to read the second\nline after reading the first line successfully, no?\n\n> \tterms->term_good = strbuf_detach(&str, NULL);\n> \n> finish:\n"},{"id":"548227","messageId":"alcvip2czKFiiIhV@pks.im","threadId":"65998","inReplyTo":"0692704d45060a62579b50dd7a2f07da04f435c8.1784069325.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/11] config: propagate launch_editor() failure in show_editor()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-15T06:58:18Z","receivedAt":"2026-07-15T06:58:24Z","isPatch":true,"body":"On Tue, Jul 14, 2026 at 10:48:35PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/builtin/config.c b/builtin/config.c\n> index 8d8ec0beea..1307fdb0d6 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -1313,7 +1313,10 @@ static int show_editor(struct config_location_options *opts)\n> \t\telse if (errno != EEXIST)\n> \t\t\tdie_errno(_(\"cannot create configuration file %s\"), config_file);\n> \t}\n> -\tlaunch_editor(config_file, NULL, NULL);\n> +\tif (launch_editor(config_file, NULL, NULL)) {\n> +\t\tfree(config_file);\n> +\t\treturn -1;\n> +\t}\n\nAll error paths in `launch_editor()` already print an error message, so\nwe indeed don't have to do anything but bubble up the error here.\n\nPatrick\n"},{"id":"548228","messageId":"alcvjynWsHZKXD84@pks.im","threadId":"65998","inReplyTo":"b31e0326e7c4f97753c80077c8f0927504f40370.1784069325.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 06/11] compat/pread: check initial lseek for errors","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-15T06:58:23Z","receivedAt":"2026-07-15T06:58:28Z","isPatch":true,"body":"On Tue, Jul 14, 2026 at 10:48:39PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/compat/pread.c b/compat/pread.c\n> index 484e6d4c71..ac7d058cb8 100644\n> --- a/compat/pread.c\n> +++ b/compat/pread.c\n> @@ -7,6 +7,8 @@ ssize_t git_pread(int fd, void *buf, size_t count, off_t offset)\n> ssize_t rc;\n> \n> current_offset = lseek(fd, 0, SEEK_CUR);\n> +\tif (current_offset < 0)\n> +\t\treturn -1;\n> \n> if (lseek(fd, offset, SEEK_SET) < 0)\n> return -1;\n\nHeh, funny. I wanted to complain about misindentation here, but your new\ncode is actually indented correctly. It's everything else in this file\nthat is indented with spaces.\n\nPatrick\n"},{"id":"548229","messageId":"alcvlJABsStbRtw8@pks.im","threadId":"65998","inReplyTo":"1792042098cd50ba164b90e5ce62430037661343.1784069325.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 07/11] transport-helper: check dup() return in get_exporter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-15T06:58:28Z","receivedAt":"2026-07-15T06:58:33Z","isPatch":true,"body":"On Tue, Jul 14, 2026 at 10:48:40PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 80f90eb7ba..31883b244e 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -487,6 +487,8 @@ static int get_exporter(struct transport *transport,\n> \t/* we need to duplicate helper->in because we want to use it after\n> \t * fastexport is done with it. */\n> \tfastexport->out = dup(helper->in);\n> +\tif (fastexport->out < 0)\n> +\t\treturn error_errno(_(\"could not dup helper output fd\"));\n> \tstrvec_push(&fastexport->args, \"fast-export\");\n> \tstrvec_push(&fastexport->args, \"--use-done-feature\");\n> \tstrvec_push(&fastexport->args, data->signed_tags ?\n\nMakes sense. The only caller already knows to die in case it sees a\nnon-zero return value.\n\nPatrick\n"},{"id":"548230","messageId":"alcvmX3b6y92KE4y@pks.im","threadId":"65998","inReplyTo":"c0827a79476d02f2b09ded919b44860e3743fbe0.1784069325.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 10/11] bisect: check get_terms return at all call sites","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-15T06:58:33Z","receivedAt":"2026-07-15T06:58:38Z","isPatch":true,"body":"On Tue, Jul 14, 2026 at 10:48:43PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Six callers of get_terms() silently discard its return value. When\n> get_terms fails (missing or truncated BISECT_TERMS file), the term\n> strings remain NULL or empty, causing confusing downstream\n> behavior: commands like \"bisect next\" or \"bisect run\" proceed with\n> empty term strings, producing nonsensical ref names (refs/bisect/\n> with no suffix) and misleading error messages.\n> \n> Add checks at each call site so that a failed get_terms produces a\n> clear \"no terms defined\" error, matching the pattern already used\n> in bisect_terms() at line 512. The check tests the term pointers\n> rather than the return value because some callers (bisect skip,\n> legacy bad/good) call set_terms before get_terms, and the\n> set_terms values should survive a get_terms failure.\n\nHm. Are there any callers that accept the case where either `term->bad`\nor `term->good` are `NULL`? If not, should we maybe adapt the function\nitself to return an error if so and then have all callers only ever\ncheck for the return value of `get_term()` instead of also having to\ncheck the result? That might also allow us to deduplicate the error\nmessages.\n\nPatrick\n"},{"id":"548231","messageId":"alcvnm0xiOv5W0w_@pks.im","threadId":"65998","inReplyTo":"2da452e39cbe1bd53da9d76fa7f7615c1a453634.1784069325.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 11/11] bisect: handle dup() failure when redirecting stdout","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-15T06:58:38Z","receivedAt":"2026-07-15T06:58:43Z","isPatch":true,"body":"On Tue, Jul 14, 2026 at 10:48:44PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index 15a2a30f89..801daf8c78 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -1308,6 +1308,11 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n> \n> \t\tfflush(stdout);\n> \t\tsaved_stdout = dup(1);\n> +\t\tif (saved_stdout < 0) {\n> +\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n> +\t\t\tclose(temporary_stdout_fd);\n> +\t\t\tbreak;\n> +\t\t}\n> \t\tdup2(temporary_stdout_fd, 1);\n\nShouldn't we also verify the return value of `dup2()` while at it?\n\nPatrick\n"},{"id":"548648","messageId":"xmqqh5lui6wg.fsf@gitster.g","threadId":"65998","inReplyTo":"xmqqldbdqciy.fsf@gitster.g","subject":"Re: [PATCH 05/11] last-modified: handle repo_parse_commit() failures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-20T05:09:35Z","receivedAt":"2026-07-20T05:09:39Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> ...\n>> -\t\trepo_parse_commit(lm->rev.repo, c);\n>> +\t\tif (repo_parse_commit(lm->rev.repo, c))\n>> +\t\t\tcontinue;\n>\n> Shouldn't this be\n>\n> \t\t\tgoto cleanup;\n>\n> instead? 'n' pulled out of not_queue may be unparseable and when we\n> ignore it, don't we still want to clean up the active_paths slab for\n> commit 'c'?\n\n--- >8 ---\nSubject: [PATCH] fixup! last-modified: handle repo_parse_commit() failures\n\nhttps://lore.kernel.org/git/xmqqldbdqciy.fsf@gitster.g/\n\n'n' pulled out of not_queue may be unparseable and when we ignore\nit, we still want to clean up the active_paths slab for commit 'c'.\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex fe012b0c2e..3846244dfc 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -416,7 +416,7 @@ static int last_modified_run(struct last_modified *lm)\n \t\t * in the '--not' queue.\n \t\t */\n \t\tif (repo_parse_commit(lm->rev.repo, c))\n-\t\t\tcontinue;\n+\t\t\tgoto cleanup;\n \n \t\twhile ((n = prio_queue_get(¬_queue))) {\n \t\t\tstruct commit_list *np;\n"},{"id":"548649","messageId":"xmqqcxwii6wd.fsf@gitster.g","threadId":"65998","inReplyTo":"xmqqh5m1qcfh.fsf@gitster.g","subject":"Re: [PATCH 09/11] bisect: check strbuf_getline_lf return when reading terms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-20T05:09:38Z","receivedAt":"2026-07-20T05:09:41Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> ...\n>> diff --git a/builtin/bisect.c b/builtin/bisect.c\n>> index 798e28f501..fe66d84382 100644\n>> --- a/builtin/bisect.c\n>> +++ b/builtin/bisect.c\n>> @@ -498,9 +498,15 @@ static int get_terms(struct bisect_terms *terms)\n>> \t}\n>> \n>> \tfree_terms(terms);\n>> -\tstrbuf_getline_lf(&str, fp);\n>> +\tif (strbuf_getline_lf(&str, fp) == EOF) {\n>> +\t\tres = -1;\n>> +\t\tgoto finish;\n>> +\t}\n>> \tterms->term_bad = strbuf_detach(&str, NULL);\n>> -\tstrbuf_getline_lf(&str, fp);\n>> +\tif (strbuf_getline_lf(&str, fp) == EOF) {\n>> +\t\tres = -1;\n>> +\t\tgoto finish;\n>> +\t}\n>\n> We want to clean-up terms->term_bad when we fail to read the second\n> line after reading the first line successfully, no?\n>\n>> \tterms->term_good = strbuf_detach(&str, NULL);\n>> \n>> finish:\n\n--- >8 ---\nSubject: [PATCH] fixup! bisect: check strbuf_getline_lf return when reading\n terms\n\nhttps://lore.kernel.org/git/xmqqh5m1qcfh.fsf@gitster.g/\n\nThis fixes the immediate leak introduced by\n\nhttps://lore.kernel.org/git/17c382fdf46eada79ce03a7604dd7e0454d8bea4.1784069325.git.gitgitgadget@gmail.com/\n\nbut many callers of get_terms() should all be fixed to check for\nreturn value. If it fails to grab the replacement word for \"bad\",\nboth terms->term_bad and terms->term_good are left NULL, since the\nfunction calls free_terms() early.\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex fe66d84382..69ab7ea248 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -505,6 +505,7 @@ static int get_terms(struct bisect_terms *terms)\n \tterms->term_bad = strbuf_detach(&str, NULL);\n \tif (strbuf_getline_lf(&str, fp) == EOF) {\n \t\tres = -1;\n+\t\tFREE_AND_NULL(terms->term_bad);\n \t\tgoto finish;\n \t}\n \tterms->term_good = strbuf_detach(&str, NULL);\n"},{"id":"549723","messageId":"7e111d67-1e43-8a4c-d4a8-7ddd923e8083@gmx.de","threadId":"65998","inReplyTo":"xmqqh5lui6wg.fsf@gitster.g","subject":"Re: [PATCH 05/11] last-modified: handle repo_parse_commit() failures","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-08-05T14:27:39Z","receivedAt":"2026-08-05T14:27:47Z","isPatch":true,"body":"Hi Junio,\n\nOn Sun, 19 Jul 2026, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > \"Johannes Schindelin via GitGitGadget\" \n> > writes:\n> > ...\n> >> -\t\trepo_parse_commit(lm->rev.repo, c);\n> >> +\t\tif (repo_parse_commit(lm->rev.repo, c))\n> >> +\t\t\tcontinue;\n> >\n> > Shouldn't this be\n> >\n> > \t\t\tgoto cleanup;\n> >\n> > instead? 'n' pulled out of not_queue may be unparseable and when we\n> > ignore it, don't we still want to clean up the active_paths slab for\n> > commit 'c'?\n\nCorrect.\n\nThanks,\nJohannes\n\n> \n> --- >8 ---\n> Subject: [PATCH] fixup! last-modified: handle repo_parse_commit() failures\n> \n> https://lore.kernel.org/git/xmqqldbdqciy.fsf@gitster.g/\n> \n> 'n' pulled out of not_queue may be unparseable and when we ignore\n> it, we still want to clean up the active_paths slab for commit 'c'.\n> \n> diff --git a/builtin/last-modified.c b/builtin/last-modified.c\n> index fe012b0c2e..3846244dfc 100644\n> --- a/builtin/last-modified.c\n> +++ b/builtin/last-modified.c\n> @@ -416,7 +416,7 @@ static int last_modified_run(struct last_modified *lm)\n> \t\t * in the '--not' queue.\n> \t\t */\n> \t\tif (repo_parse_commit(lm->rev.repo, c))\n> -\t\t\tcontinue;\n> +\t\t\tgoto cleanup;\n> \n> \t\twhile ((n = prio_queue_get(¬_queue))) {\n> \t\t\tstruct commit_list *np;\n> \n"},{"id":"549724","messageId":"11e705f9-d64f-f8c8-3967-d3289e47eb91@gmx.de","threadId":"65998","inReplyTo":"alcvjynWsHZKXD84@pks.im","subject":"Re: [PATCH 06/11] compat/pread: check initial lseek for errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-08-05T14:29:26Z","receivedAt":"2026-08-05T14:29:29Z","isPatch":true,"body":"Hi Patrick,\n\nOn Wed, 15 Jul 2026, Patrick Steinhardt wrote:\n\n> On Tue, Jul 14, 2026 at 10:48:39PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > diff --git a/compat/pread.c b/compat/pread.c\n> > index 484e6d4c71..ac7d058cb8 100644\n> > --- a/compat/pread.c\n> > +++ b/compat/pread.c\n> > @@ -7,6 +7,8 @@ ssize_t git_pread(int fd, void *buf, size_t count, off_t offset)\n> > ssize_t rc;\n> > \n> > current_offset = lseek(fd, 0, SEEK_CUR);\n> > +\tif (current_offset < 0)\n> > +\t\treturn -1;\n> > \n> > if (lseek(fd, offset, SEEK_SET) < 0)\n> > return -1;\n> \n> Heh, funny. I wanted to complain about misindentation here, but your new\n> code is actually indented correctly. It's everything else in this file\n> that is indented with spaces.\n\nHeh. I did notice something odd going on, thinking that Opus ignored my\nclear instructions about tab-indentation once again when I replaced the\nspaces by tabs...\n\nCiao,\nJohannes\n"},{"id":"549725","messageId":"282a9a4e-46bc-a790-e801-970d3a52468b@gmx.de","threadId":"65998","inReplyTo":"xmqqcxwii6wd.fsf@gitster.g","subject":"Re: [PATCH 09/11] bisect: check strbuf_getline_lf return when reading terms","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-08-05T14:33:20Z","receivedAt":"2026-08-05T14:33:24Z","isPatch":true,"body":"Hi Junio,\n\nOn Sun, 19 Jul 2026, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > \"Johannes Schindelin via GitGitGadget\" \n> > writes:\n> > ...\n> >> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> >> index 798e28f501..fe66d84382 100644\n> >> --- a/builtin/bisect.c\n> >> +++ b/builtin/bisect.c\n> >> @@ -498,9 +498,15 @@ static int get_terms(struct bisect_terms *terms)\n> >> \t}\n> >> \n> >> \tfree_terms(terms);\n> >> -\tstrbuf_getline_lf(&str, fp);\n> >> +\tif (strbuf_getline_lf(&str, fp) == EOF) {\n> >> +\t\tres = -1;\n> >> +\t\tgoto finish;\n> >> +\t}\n> >> \tterms->term_bad = strbuf_detach(&str, NULL);\n> >> -\tstrbuf_getline_lf(&str, fp);\n> >> +\tif (strbuf_getline_lf(&str, fp) == EOF) {\n> >> +\t\tres = -1;\n> >> +\t\tgoto finish;\n> >> +\t}\n> >\n> > We want to clean-up terms->term_bad when we fail to read the second\n> > line after reading the first line successfully, no?\n> >\n> >> \tterms->term_good = strbuf_detach(&str, NULL);\n> >> \n> >> finish:\n> \n> --- >8 ---\n> Subject: [PATCH] fixup! bisect: check strbuf_getline_lf return when reading\n> terms\n> \n> https://lore.kernel.org/git/xmqqh5m1qcfh.fsf@gitster.g/\n> \n> This fixes the immediate leak introduced by\n> \n> https://lore.kernel.org/git/17c382fdf46eada79ce03a7604dd7e0454d8bea4.1784069325.git.gitgitgadget@gmail.com/\n> \n> but many callers of get_terms() should all be fixed to check for\n> return value. If it fails to grab the replacement word for \"bad\",\n> both terms->term_bad and terms->term_good are left NULL, since the\n> function calls free_terms() early.\n> \n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index fe66d84382..69ab7ea248 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -505,6 +505,7 @@ static int get_terms(struct bisect_terms *terms)\n> \tterms->term_bad = strbuf_detach(&str, NULL);\n> \tif (strbuf_getline_lf(&str, fp) == EOF) {\n> \t\tres = -1;\n> +\t\tFREE_AND_NULL(terms->term_bad);\n\nGood catch!\n\nThank you,\nJohannes\n\n> \t\tgoto finish;\n> \t}\n> \tterms->term_good = strbuf_detach(&str, NULL);\n> \n"},{"id":"549726","messageId":"fa29b166-39e0-ad33-50bf-2a1241fa6971@gmx.de","threadId":"65998","inReplyTo":"alcvmX3b6y92KE4y@pks.im","subject":"Re: [PATCH 10/11] bisect: check get_terms return at all call sites","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-08-05T14:57:01Z","receivedAt":"2026-08-05T14:57:04Z","isPatch":true,"body":"Hi Patrick,\n\nOn Wed, 15 Jul 2026, Patrick Steinhardt wrote:\n\n> On Tue, Jul 14, 2026 at 10:48:43PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> > \n> > Six callers of get_terms() silently discard its return value. When\n> > get_terms fails (missing or truncated BISECT_TERMS file), the term\n> > strings remain NULL or empty, causing confusing downstream\n> > behavior: commands like \"bisect next\" or \"bisect run\" proceed with\n> > empty term strings, producing nonsensical ref names (refs/bisect/\n> > with no suffix) and misleading error messages.\n> > \n> > Add checks at each call site so that a failed get_terms produces a\n> > clear \"no terms defined\" error, matching the pattern already used\n> > in bisect_terms() at line 512. The check tests the term pointers\n> > rather than the return value because some callers (bisect skip,\n> > legacy bad/good) call set_terms before get_terms, and the\n> > set_terms values should survive a get_terms failure.\n> \n> Hm. Are there any callers that accept the case where either `term->bad`\n> or `term->good` are `NULL`?\n\nAs far as I can tell, no, the case where either `term->bad` or\n`term->good` are `NULL` is not permissible.\n\n> If not, should we maybe adapt the function itself to return an error if\n> so and then have all callers only ever check for the return value of\n> `get_term()` instead of also having to check the result? That might also\n> allow us to deduplicate the error messages.\n\nIt's a good point that we should not look at `term->bad` and `term->good`,\nbut at the return value of `get_term()` instead. That's incidentally what\n`bisect_terms()` does, and we should do the same here (including the same,\nalready-translated error message).\n\nThanks,\nJohannes\n\n"},{"id":"549747","messageId":"8f0e066f-c995-e47b-a3a4-709821255671@gmx.de","threadId":"65998","inReplyTo":"alcvnm0xiOv5W0w_@pks.im","subject":"Re: [PATCH 11/11] bisect: handle dup() failure when redirecting stdout","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-08-05T16:44:53Z","receivedAt":"2026-08-05T16:44:56Z","isPatch":true,"body":"Hi Patrick,\n\nOn Wed, 15 Jul 2026, Patrick Steinhardt wrote:\n\n> On Tue, Jul 14, 2026 at 10:48:44PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > diff --git a/builtin/bisect.c b/builtin/bisect.c\n> > index 15a2a30f89..801daf8c78 100644\n> > --- a/builtin/bisect.c\n> > +++ b/builtin/bisect.c\n> > @@ -1308,6 +1308,11 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n> > \n> > \t\tfflush(stdout);\n> > \t\tsaved_stdout = dup(1);\n> > +\t\tif (saved_stdout < 0) {\n> > +\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n> > +\t\t\tclose(temporary_stdout_fd);\n> > +\t\t\tbreak;\n> > +\t\t}\n> > \t\tdup2(temporary_stdout_fd, 1);\n> \n> Shouldn't we also verify the return value of `dup2()` while at it?\n\nTrue. I wonder why Coverity didn't complain... funny. I changed it to also\ncheck the return value of `dup2()`.\n\nThank you for your review!\nJohannes\n"},{"id":"549753","messageId":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH v2 00/11] coverity: fix unchecked returns","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:49Z","receivedAt":"2026-08-05T18:31:03Z","isPatch":true,"body":"This is the next batch of fixes in response to issues reported by Coverity.\n\nChanges since v1:\n\n * The last-modified patch is now more careful to clean up a commit slab\n when parsing the commit failed.\n * When the \"good\" bisect term was read successfully, but not the \"bad\" one,\n the \"good\" one is now cleaned up.\n * Instead of detecting failed get_terms() calls indirectly, the return\n value is now checked.\n * Failures when bisect_run() calls dup2() are now handled properly, too.\n\nJohannes Schindelin (11):\n http: die on curl_easy_duphandle failure in get_active_slot\n config: propagate launch_editor() failure in show_editor()\n reftable/block: check deflateInit() return value\n reftable tests: check reftable_table_init_ref_iterator() return\n last-modified: handle repo_parse_commit() failures\n compat/pread: check initial lseek for errors\n transport-helper: check dup() return in get_exporter\n transport-helper: warn when export-marks file cannot be finalized\n bisect: check strbuf_getline_lf return when reading terms\n bisect: check get_terms return at all call sites\n bisect: handle dup() failure when redirecting stdout\n\n bisect.c | 6 +++--\n builtin/bisect.c | 42 +++++++++++++++++++++++----------\n builtin/config.c | 5 +++-\n builtin/last-modified.c | 9 ++++---\n compat/pread.c | 2 ++\n http.c | 2 ++\n reftable/block.c | 3 ++-\n t/unit-tests/u-reftable-table.c | 6 +++--\n transport-helper.c | 6 ++++-\n 9 files changed, 59 insertions(+), 22 deletions(-)\n\n\nbase-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2179%2Fdscho%2Fcoverity-fixes-unchecked-returns-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2179/dscho/coverity-fixes-unchecked-returns-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2179\n\nRange-diff vs v1:\n\n 1: e653255de1 = 1: e653255de1 http: die on curl_easy_duphandle failure in get_active_slot\n 2: 0692704d45 = 2: 0692704d45 config: propagate launch_editor() failure in show_editor()\n 3: 9bf7e737c7 = 3: 9bf7e737c7 reftable/block: check deflateInit() return value\n 4: 711671c3ab = 4: 711671c3ab reftable tests: check reftable_table_init_ref_iterator() return\n 5: f728be4dac ! 5: 72a74c76be last-modified: handle repo_parse_commit() failures\n @@ Commit message\n Pointed out by Coverity.\n \n Assisted-by: Claude Opus 4.6\n + Helped-by: Junio C Hamano \n Signed-off-by: Johannes Schindelin \n \n ## builtin/last-modified.c ##\n @@ builtin/last-modified.c: static int last_modified_run(struct last_modified *lm)\n \t\t */\n -\t\trepo_parse_commit(lm->rev.repo, c);\n +\t\tif (repo_parse_commit(lm->rev.repo, c))\n -+\t\t\tcontinue;\n ++\t\t\tgoto cleanup;\n \n \t\twhile ((n = prio_queue_get(¬_queue))) {\n \t\t\tstruct commit_list *np;\n 6: b31e0326e7 = 6: f0b1e13979 compat/pread: check initial lseek for errors\n 7: 1792042098 = 7: 0facb9e8ca transport-helper: check dup() return in get_exporter\n 8: 13ddcce053 = 8: 2b0e4f32fd transport-helper: warn when export-marks file cannot be finalized\n 9: 17c382fdf4 ! 9: 7f2b963103 bisect: check strbuf_getline_lf return when reading terms\n @@ Commit message\n Pointed out by Coverity.\n \n Assisted-by: Claude Opus 4.6\n + Helped-by: Junio C Hamano \n Signed-off-by: Johannes Schindelin \n \n ## bisect.c ##\n @@ builtin/bisect.c: static int get_terms(struct bisect_terms *terms)\n -\tstrbuf_getline_lf(&str, fp);\n +\tif (strbuf_getline_lf(&str, fp) == EOF) {\n +\t\tres = -1;\n ++\t\tFREE_AND_NULL(terms->term_bad);\n +\t\tgoto finish;\n +\t}\n \tterms->term_good = strbuf_detach(&str, NULL);\n 10: c0827a7947 ! 10: 9a9103096a bisect: check get_terms return at all call sites\n @@ Commit message\n empty term strings, producing nonsensical ref names (refs/bisect/\n with no suffix) and misleading error messages.\n \n - Add checks at each call site so that a failed get_terms produces a\n - clear \"no terms defined\" error, matching the pattern already used\n - in bisect_terms() at line 512. The check tests the term pointers\n - rather than the return value because some callers (bisect skip,\n - legacy bad/good) call set_terms before get_terms, and the\n - set_terms values should survive a get_terms failure.\n + Let's not discard the return value, but handle an error with the same\n + message `bisect_terms()` already uses when reading the terms failed.\n \n Pointed out by Coverity.\n \n + There is one slight complication here: One caller _needs_ the return\n + value to indicate an error when the `BISECT_TERMS` file is absent, all\n + the other call sites are totally okay with a \"missing\" `BISECT_TERMS`\n + file. To address that, extend the function signature of `get_terms()` to\n + indicate which behavior the caller wants.\n +\n Assisted-by: Claude Opus 4.6\n + Helped-by: Patrick Steinhardt \n Signed-off-by: Johannes Schindelin \n \n ## builtin/bisect.c ##\n +@@ builtin/bisect.c: static int bisect_next_check(const struct bisect_terms *terms,\n + \treturn decide_next(terms, current_term, !state.nr_good, !state.nr_bad);\n + }\n + \n +-static int get_terms(struct bisect_terms *terms)\n ++static int get_terms(struct bisect_terms *terms, int file_missing_is_ok)\n + {\n + \tstruct strbuf str = STRBUF_INIT;\n + \tFILE *fp = NULL;\n +@@ builtin/bisect.c: static int get_terms(struct bisect_terms *terms)\n + \n + \tfp = fopen(git_path_bisect_terms(), \"r\");\n + \tif (!fp) {\n +-\t\tres = -1;\n ++\t\tres = file_missing_is_ok ? 0 : -1;\n + \t\tgoto finish;\n + \t}\n + \n +@@ builtin/bisect.c: finish:\n + \n + static int bisect_terms(struct bisect_terms *terms, const char *option)\n + {\n +-\tif (get_terms(terms))\n ++\tif (get_terms(terms, 0))\n + \t\treturn error(_(\"no terms defined\"));\n + \n + \tif (!option) {\n @@ builtin/bisect.c: static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n + \trev = word_end + strspn(word_end, \" \\t\");\n \t*word_end = '\\0'; /* NUL-terminate the word */\n \n - \tget_terms(terms);\n -+\tif (!terms->term_bad || !terms->term_good)\n +-\tget_terms(terms);\n ++\tif (get_terms(terms, 1))\n +\t\treturn error(_(\"no terms defined\"));\n \tif (check_and_set_terms(terms, p))\n \t\treturn -1;\n \n @@ builtin/bisect.c: static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref\n + \tif (argc)\n \t\treturn error(_(\"'%s' requires 0 arguments\"),\n \t\t\t \"git bisect next\");\n - \tget_terms(&terms);\n -+\tif (!terms.term_bad || !terms.term_good)\n +-\tget_terms(&terms);\n ++\tif (get_terms(&terms, 1))\n +\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_next(&terms, prefix);\n \tfree_terms(&terms);\n \treturn res;\n @@ builtin/bisect.c: static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS\n + \tstruct bisect_terms terms = { 0 };\n \n \tset_terms(&terms, \"bad\", \"good\");\n - \tget_terms(&terms);\n -+\tif (!terms.term_bad || !terms.term_good)\n +-\tget_terms(&terms);\n ++\tif (get_terms(&terms, 1))\n +\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_skip(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n @@ builtin/bisect.c: static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix\n + \tint res;\n \tstruct bisect_terms terms = { 0 };\n \n - \tget_terms(&terms);\n -+\tif (!terms.term_bad || !terms.term_good)\n +-\tget_terms(&terms);\n ++\tif (get_terms(&terms, 1))\n +\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_visualize(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n @@ builtin/bisect.c: static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE\n + \n \tif (!argc)\n \t\treturn error(_(\"'%s' failed: no command provided.\"), \"git bisect run\");\n - \tget_terms(&terms);\n -+\tif (!terms.term_bad || !terms.term_good)\n +-\tget_terms(&terms);\n ++\tif (get_terms(&terms, 1))\n +\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_run(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n @@ builtin/bisect.c: int cmd_bisect(int argc,\n + \t\t\tusage_with_options(git_bisect_usage, options);\n \n \t\tset_terms(&terms, \"bad\", \"good\");\n - \t\tget_terms(&terms);\n -+\t\tif (!terms.term_bad || !terms.term_good)\n +-\t\tget_terms(&terms);\n ++\t\tif (get_terms(&terms, 1))\n +\t\t\treturn error(_(\"no terms defined\"));\n \t\tif (check_and_set_terms(&terms, argv[0]) ||\n \t\t !one_of(argv[0], terms.term_good, terms.term_bad, NULL))\n 11: 2da452e39c ! 11: 829cd82177 bisect: handle dup() failure when redirecting stdout\n @@ Commit message\n leaving the process with stdout still pointing at the temporary file\n for the remainder of the run.\n \n - Treat a failed dup(1) as a fatal error for this bisect step: close\n - the temporary file descriptor, report the error via error_errno(),\n - and break out of the loop so the existing cleanup path handles the\n - rest, just as on other failure paths in this function.\n + Treat a failed dup(1) or dup2(..., 1) as a fatal error for this bisect\n + step: close the temporary file descriptor, report the error via\n + error_errno(), and break out of the loop so the existing cleanup path\n + handles the rest, just as on other failure paths in this function.\n \n Reported by Coverity as CID 1508242 (\"Improper use of negative\n value\").\n \n Assisted-by: Opus 4.7\n + Helped-by: Patrick Steinhardt \n Signed-off-by: Johannes Schindelin \n \n ## builtin/bisect.c ##\n @@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co\n \n \t\tfflush(stdout);\n \t\tsaved_stdout = dup(1);\n -+\t\tif (saved_stdout < 0) {\n +-\t\tdup2(temporary_stdout_fd, 1);\n ++\t\tif (saved_stdout < 0 ||\n ++\t\t dup2(temporary_stdout_fd, 1) < 0) {\n +\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n +\t\t\tclose(temporary_stdout_fd);\n +\t\t\tbreak;\n +\t\t}\n - \t\tdup2(temporary_stdout_fd, 1);\n \n \t\tres = bisect_state(terms, 1, &new_state);\n + \n\n-- \ngitgitgadget\n"},{"id":"549754","messageId":"e653255de19decfe45d4ef8d3277aaf69c44c391.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 01/11] http: die on curl_easy_duphandle failure in get_active_slot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:50Z","receivedAt":"2026-08-05T18:31:05Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_active_slot() duplicates the default curl handle via\ncurl_easy_duphandle() to create a per-slot session handle. The\nreturn value is stored directly in slot->curl without checking\nfor NULL. curl_easy_duphandle() can return NULL when memory\nallocation fails internally, and the libcurl documentation\nexplicitly states this possibility.\n\nWhen this happens, slot->curl is NULL and the very next operation\n(curl_easy_setopt on line 1632 for CURLOPT_COOKIEFILE) passes\nNULL as the curl handle, which is undefined behavior in libcurl\nand typically crashes.\n\nEvery HTTP operation in git goes through get_active_slot(), so\nthis affects all remote-https, remote-http, and HTTP-based\noperations (clone, fetch, push over HTTP, bundle-uri downloads).\n\nAdd a NULL check and die() with a clear message. There is no\nreasonable recovery from a failed handle duplication: the process\nis out of memory and cannot perform any HTTP operation.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n http.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex b4e7b8d00b..8f1d6d1f56 100644\n--- a/http.c\n+++ b/http.c\n@@ -1608,6 +1608,8 @@ struct active_request_slot *get_active_slot(void)\n \n \tif (!slot->curl) {\n \t\tslot->curl = curl_easy_duphandle(curl_default);\n+\t\tif (!slot->curl)\n+\t\t\tdie(\"curl_easy_duphandle failed\");\n \t\tcurl_session_count++;\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"549755","messageId":"0692704d45060a62579b50dd7a2f07da04f435c8.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 02/11] config: propagate launch_editor() failure in show_editor()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:51Z","receivedAt":"2026-08-05T18:31:06Z","isPatch":true,"body":"From: Johannes Schindelin \n\nshow_editor() calls launch_editor() to open the user's editor on\nthe configuration file, but discards the return value and\nunconditionally returns 0 (success). When the editor fails to\nlaunch (e.g., $EDITOR is not found, or the editor exits with a\nnonzero status), the caller receives no indication that anything\nwent wrong.\n\nThis affects \"git config edit\" and \"git config --edit\": the\ncommand silently succeeds even when the editor could not be\nstarted. In contrast, other editor-launching paths in git (such\nas \"git commit\" and \"git rebase --edit-todo\") properly propagate\neditor failures and exit with an error.\n\nCheck the return value and propagate the failure by returning -1.\nThe two callers (cmd_config_edit at line 1315 and the legacy\ncmd_config at line 1478) both propagate this return to\nhandle_builtin, which translates negative returns into an error\nexit.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/config.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 8d8ec0beea..1307fdb0d6 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -1313,7 +1313,10 @@ static int show_editor(struct config_location_options *opts)\n \t\telse if (errno != EEXIST)\n \t\t\tdie_errno(_(\"cannot create configuration file %s\"), config_file);\n \t}\n-\tlaunch_editor(config_file, NULL, NULL);\n+\tif (launch_editor(config_file, NULL, NULL)) {\n+\t\tfree(config_file);\n+\t\treturn -1;\n+\t}\n \tfree(config_file);\n \n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"549756","messageId":"9bf7e737c740d8a80467ee3b38df9c86bbf7a566.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 03/11] reftable/block: check deflateInit() return value","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:52Z","receivedAt":"2026-08-05T18:31:08Z","isPatch":true,"body":"From: Johannes Schindelin \n\nblock_writer_init() allocates a z_stream and calls deflateInit()\nto prepare it for compressing log records. The return value of\ndeflateInit() is silently discarded. If zlib initialization fails\n(e.g., Z_MEM_ERROR when the system is under memory pressure), the\nz_stream is left in an undefined state.\n\nSubsequent deflate() calls in block_writer_finish() then operate\non this uninitialized stream. Depending on the zlib\nimplementation, this can produce silently corrupted compressed\ndata (which would be written to the reftable file and discovered\nonly when a later reader fails to inflate) or crash outright.\n\nThe function already uses REFTABLE_ZLIB_ERROR for deflate()\nfailures later in the code path (lines 171, 199), so returning\nthe same error code for deflateInit() failure is consistent.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n reftable/block.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/block.c b/reftable/block.c\nindex 920b3f4486..ec81fd0493 100644\n--- a/reftable/block.c\n+++ b/reftable/block.c\n@@ -87,7 +87,8 @@ int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,\n \t\tREFTABLE_CALLOC_ARRAY(bw->zstream, 1);\n \t\tif (!bw->zstream)\n \t\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n-\t\tdeflateInit(bw->zstream, 9);\n+\t\tif (deflateInit(bw->zstream, 9) != Z_OK)\n+\t\t\treturn REFTABLE_ZLIB_ERROR;\n \t}\n \n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"549759","messageId":"711671c3abac64d9bb0872a69d45df4f103afc66.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 04/11] reftable tests: check reftable_table_init_ref_iterator() return","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:53Z","receivedAt":"2026-08-05T18:31:10Z","isPatch":true,"body":"From: Johannes Schindelin \n\ntest_reftable_table__seek_once() and test_reftable_table__reseek()\nboth call reftable_table_init_ref_iterator() without checking its\nreturn value. This function returns an int error code (0 on\nsuccess, negative on failure). Every other reftable function call\nin these same tests checks the return via cl_assert_equal_i() or\ncl_assert(), making this omission inconsistent.\n\nIf the iterator initialization ever fails (e.g., due to a memory\nallocation failure in the reftable internals), the test would\nproceed to seek and read with an uninitialized iterator, producing\nmisleading test results or crashes rather than a clear assertion\nfailure.\n\nCheck the return value via cl_assert_equal_i(ret, 0), consistent\nwith the surrounding code.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n t/unit-tests/u-reftable-table.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c\nindex fae478ee04..6f444f8cf9 100644\n--- a/t/unit-tests/u-reftable-table.c\n+++ b/t/unit-tests/u-reftable-table.c\n@@ -29,7 +29,8 @@ void test_reftable_table__seek_once(void)\n \tret = reftable_table_new(&table, &source, \"name\");\n \tcl_assert(!ret);\n \n-\treftable_table_init_ref_iterator(table, &it);\n+\tret = reftable_table_init_ref_iterator(table, &it);\n+\tcl_assert_equal_i(ret, 0);\n \tret = reftable_iterator_seek_ref(&it, \"\");\n \tcl_assert(!ret);\n \tret = reftable_iterator_next_ref(&it, &ref);\n@@ -71,7 +72,8 @@ void test_reftable_table__reseek(void)\n \tret = reftable_table_new(&table, &source, \"name\");\n \tcl_assert(!ret);\n \n-\treftable_table_init_ref_iterator(table, &it);\n+\tret = reftable_table_init_ref_iterator(table, &it);\n+\tcl_assert_equal_i(ret, 0);\n \n \tfor (size_t i = 0; i < 5; i++) {\n \t\tret = reftable_iterator_seek_ref(&it, \"\");\n-- \ngitgitgadget\n\n"},{"id":"549757","messageId":"72a74c76bec96812bd7017cb7e8c7bb82132a2a9.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 05/11] last-modified: handle repo_parse_commit() failures","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:54Z","receivedAt":"2026-08-05T18:31:11Z","isPatch":true,"body":"From: Johannes Schindelin \n\nlast_modified_run() and process_parent() call repo_parse_commit()\nwithout checking the return value at three sites. When a commit\nobject is corrupt or unavailable (e.g., a shallow clone boundary\nor a missing object in a partial clone), the parse fails and the\ncommit's internal fields (parents, tree, date) are not populated.\n\nThe consequences depend on which call site fails:\n\nAt line 417 (the main walk loop), c->parents stays NULL after a\nfailed parse. The parent-walking loop at line 440 simply does not\nexecute, silently treating the unparsable commit as a root commit.\nThis produces incorrect \"last modified\" results: paths changed in\nancestors beyond the corrupt commit are attributed to the wrong\ncommit or not reported at all.\n\nAt line 423 (the --not exclusion walk), n->parents stays NULL,\ncausing the exclusion walk to stop prematurely. Commits that\nshould be excluded from the output may be incorrectly included.\n\nAt line 293 (process_parent), the parent's tree and parents are\nunavailable, so diff operations against it produce wrong results\nand the parent's own ancestors are never enqueued for walking.\n\nSkip unparsable commits by checking the return value and\ncontinuing to the next iteration (or returning early in\nprocess_parent). This matches the defensive pattern used in other\nrevision walkers such as limit_list() and get_revision_internal().\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nHelped-by: Junio C Hamano \nSigned-off-by: Johannes Schindelin \n---\n builtin/last-modified.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 5478182f2e..3846244dfc 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -290,7 +290,8 @@ static void process_parent(struct last_modified *lm,\n {\n \tstruct bitmap *active_p;\n \n-\trepo_parse_commit(lm->rev.repo, parent);\n+\tif (repo_parse_commit(lm->rev.repo, parent))\n+\t\treturn;\n \tactive_p = active_paths_for(lm, parent);\n \n \t/*\n@@ -414,12 +415,14 @@ static int last_modified_run(struct last_modified *lm)\n \t\t * Otherwise, make sure that 'c' isn't reachable from anything\n \t\t * in the '--not' queue.\n \t\t */\n-\t\trepo_parse_commit(lm->rev.repo, c);\n+\t\tif (repo_parse_commit(lm->rev.repo, c))\n+\t\t\tgoto cleanup;\n \n \t\twhile ((n = prio_queue_get(¬_queue))) {\n \t\t\tstruct commit_list *np;\n \n-\t\t\trepo_parse_commit(lm->rev.repo, n);\n+\t\t\tif (repo_parse_commit(lm->rev.repo, n))\n+\t\t\t\tcontinue;\n \n \t\t\tfor (np = n->parents; np; np = np->next) {\n \t\t\t\tif (!(np->item->object.flags & PARENT2)) {\n-- \ngitgitgadget\n\n"},{"id":"549758","messageId":"f0b1e13979bc41e10ac9fe7b042d8d4c1191c411.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 06/11] compat/pread: check initial lseek for errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:55Z","receivedAt":"2026-08-05T18:31:13Z","isPatch":true,"body":"From: Johannes Schindelin \n\ngit_pread() saves the current file offset via lseek(fd, 0,\nSEEK_CUR) and later restores it. If the initial lseek fails\n(e.g., the fd is a pipe or otherwise non-seekable),\ncurrent_offset is -1. This negative value is later passed to\nlseek(fd, -1, SEEK_SET) at line 16, which sets the file position\nto an unintended location (or fails with EINVAL on some\nplatforms).\n\nCheck the initial lseek return value and return -1 immediately\nif it fails, consistent with the error handling for the other\nlseek calls in the same function.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n compat/pread.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/compat/pread.c b/compat/pread.c\nindex 484e6d4c71..ac7d058cb8 100644\n--- a/compat/pread.c\n+++ b/compat/pread.c\n@@ -7,6 +7,8 @@ ssize_t git_pread(int fd, void *buf, size_t count, off_t offset)\n ssize_t rc;\n \n current_offset = lseek(fd, 0, SEEK_CUR);\n+\tif (current_offset < 0)\n+\t\treturn -1;\n \n if (lseek(fd, offset, SEEK_SET) < 0)\n return -1;\n-- \ngitgitgadget\n\n"},{"id":"549760","messageId":"0facb9e8cabc739cf05829934fb2399ed9a58259.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 07/11] transport-helper: check dup() return in get_exporter","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:56Z","receivedAt":"2026-08-05T18:31:16Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_exporter() duplicates helper->in via dup() and stores the\nresult in fastexport->out. If dup() fails (fd exhaustion), it\nreturns -1. The child_process machinery interprets out = -1 as\n\"create a pipe for stdout\", which would silently change the\nfast-export process's output wiring: instead of sending data\nback through the helper's input fd, it would write to a new pipe\nthat nobody reads from.\n\nCheck the return value and report the error before proceeding.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n transport-helper.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 80f90eb7ba..31883b244e 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -487,6 +487,8 @@ static int get_exporter(struct transport *transport,\n \t/* we need to duplicate helper->in because we want to use it after\n \t * fastexport is done with it. */\n \tfastexport->out = dup(helper->in);\n+\tif (fastexport->out < 0)\n+\t\treturn error_errno(_(\"could not dup helper output fd\"));\n \tstrvec_push(&fastexport->args, \"fast-export\");\n \tstrvec_push(&fastexport->args, \"--use-done-feature\");\n \tstrvec_push(&fastexport->args, data->signed_tags ?\n-- \ngitgitgadget\n\n"},{"id":"549761","messageId":"2b0e4f32fda6672f5d093e203e2e9774cf464aef.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 08/11] transport-helper: warn when export-marks file cannot be finalized","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:57Z","receivedAt":"2026-08-05T18:31:18Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen push_refs_with_export() finalizes a successful push, it writes\nthe fast-export marks file to a .tmp sibling and rename()s it into\nplace. The return value of rename() is currently ignored. If the\nrename fails (permission denied, full disk, or an antivirus product\nlocking the destination on Windows), the .tmp file is left behind\nand the existing export_marks file remains stale; the next\nfast-export operation that resumes from it then silently operates on\ninconsistent bookkeeping.\n\nThe push itself succeeded by that point, so promoting this to a\nfatal error would be inappropriate. Emit warning_errno() naming both\npaths so the user can recover manually, and keep returning 0.\n\nFlagged by Coverity as CID 1427723 (\"Unchecked return value\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n transport-helper.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 31883b244e..ed0543f1ad 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -1184,7 +1184,9 @@ static int push_refs_with_export(struct transport *transport,\n \n \tif (data->export_marks) {\n \t\tstrbuf_addf(&buf, \"%s.tmp\", data->export_marks);\n-\t\trename(buf.buf, data->export_marks);\n+\t\tif (rename(buf.buf, data->export_marks))\n+\t\t\twarning_errno(_(\"could not rename '%s' to '%s'\"),\n+\t\t\t\t buf.buf, data->export_marks);\n \t\tstrbuf_release(&buf);\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"549762","messageId":"7f2b9631032bb2719060b6fe1971fdfd267de6b8.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 09/11] bisect: check strbuf_getline_lf return when reading terms","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:58Z","receivedAt":"2026-08-05T18:31:21Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_terms() in builtin/bisect.c and read_bisect_terms() in\nbisect.c both read the BISECT_TERMS file but do not check the\nstrbuf_getline_lf() return values. If the file is truncated\n(e.g., a partial write from a crash or disk-full condition),\nstrbuf_getline_lf returns EOF and the strbuf remains empty.\nstrbuf_detach then returns an empty string, and the term names\nsilently become \"\" instead of the expected \"bad\"/\"good\" or\ncustom terms.\n\nIn get_terms(), check for EOF and return -1 on truncation,\nmatching the existing -1 return for a missing file.\n\nIn read_bisect_terms(), die with a descriptive message when a\nline cannot be read, consistent with the die_errno for a\nnon-ENOENT open failure in the same function. Unlike get_terms(),\nread_bisect_terms() returns void and uses die() for all error\npaths, so the die is the appropriate error handling here.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nHelped-by: Junio C Hamano \nSigned-off-by: Johannes Schindelin \n---\n bisect.c | 6 ++++--\n builtin/bisect.c | 11 +++++++++--\n 2 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 94c7028d2a..c2ef5da462 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1019,10 +1019,12 @@ void read_bisect_terms(char **read_bad, char **read_good)\n \t\t\tdie_errno(_(\"could not read file '%s'\"), filename);\n \t\t}\n \t} else {\n-\t\tstrbuf_getline_lf(&str, fp);\n+\t\tif (strbuf_getline_lf(&str, fp) == EOF)\n+\t\t\tdie(_(\"could not read bad term from file '%s'\"), filename);\n \t\tfree(*read_bad);\n \t\t*read_bad = strbuf_detach(&str, NULL);\n-\t\tstrbuf_getline_lf(&str, fp);\n+\t\tif (strbuf_getline_lf(&str, fp) == EOF)\n+\t\t\tdie(_(\"could not read good term from file '%s'\"), filename);\n \t\tfree(*read_good);\n \t\t*read_good = strbuf_detach(&str, NULL);\n \t}\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 798e28f501..69ab7ea248 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -498,9 +498,16 @@ static int get_terms(struct bisect_terms *terms)\n \t}\n \n \tfree_terms(terms);\n-\tstrbuf_getline_lf(&str, fp);\n+\tif (strbuf_getline_lf(&str, fp) == EOF) {\n+\t\tres = -1;\n+\t\tgoto finish;\n+\t}\n \tterms->term_bad = strbuf_detach(&str, NULL);\n-\tstrbuf_getline_lf(&str, fp);\n+\tif (strbuf_getline_lf(&str, fp) == EOF) {\n+\t\tres = -1;\n+\t\tFREE_AND_NULL(terms->term_bad);\n+\t\tgoto finish;\n+\t}\n \tterms->term_good = strbuf_detach(&str, NULL);\n \n finish:\n-- \ngitgitgadget\n\n"},{"id":"549763","messageId":"9a9103096a2bd877f84502cffefd019d0a6e229d.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 10/11] bisect: check get_terms return at all call sites","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:30:59Z","receivedAt":"2026-08-05T18:31:23Z","isPatch":true,"body":"From: Johannes Schindelin \n\nSix callers of get_terms() silently discard its return value. When\nget_terms fails (missing or truncated BISECT_TERMS file), the term\nstrings remain NULL or empty, causing confusing downstream\nbehavior: commands like \"bisect next\" or \"bisect run\" proceed with\nempty term strings, producing nonsensical ref names (refs/bisect/\nwith no suffix) and misleading error messages.\n\nLet's not discard the return value, but handle an error with the same\nmessage `bisect_terms()` already uses when reading the terms failed.\n\nPointed out by Coverity.\n\nThere is one slight complication here: One caller _needs_ the return\nvalue to indicate an error when the `BISECT_TERMS` file is absent, all\nthe other call sites are totally okay with a \"missing\" `BISECT_TERMS`\nfile. To address that, extend the function signature of `get_terms()` to\nindicate which behavior the caller wants.\n\nAssisted-by: Claude Opus 4.6\nHelped-by: Patrick Steinhardt \nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 24 +++++++++++++++---------\n 1 file changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 69ab7ea248..ceb60b0626 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -485,7 +485,7 @@ static int bisect_next_check(const struct bisect_terms *terms,\n \treturn decide_next(terms, current_term, !state.nr_good, !state.nr_bad);\n }\n \n-static int get_terms(struct bisect_terms *terms)\n+static int get_terms(struct bisect_terms *terms, int file_missing_is_ok)\n {\n \tstruct strbuf str = STRBUF_INIT;\n \tFILE *fp = NULL;\n@@ -493,7 +493,7 @@ static int get_terms(struct bisect_terms *terms)\n \n \tfp = fopen(git_path_bisect_terms(), \"r\");\n \tif (!fp) {\n-\t\tres = -1;\n+\t\tres = file_missing_is_ok ? 0 : -1;\n \t\tgoto finish;\n \t}\n \n@@ -519,7 +519,7 @@ finish:\n \n static int bisect_terms(struct bisect_terms *terms, const char *option)\n {\n-\tif (get_terms(terms))\n+\tif (get_terms(terms, 0))\n \t\treturn error(_(\"no terms defined\"));\n \n \tif (!option) {\n@@ -1057,7 +1057,8 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n \trev = word_end + strspn(word_end, \" \\t\");\n \t*word_end = '\\0'; /* NUL-terminate the word */\n \n-\tget_terms(terms);\n+\tif (get_terms(terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tif (check_and_set_terms(terms, p))\n \t\treturn -1;\n \n@@ -1383,7 +1384,8 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref\n \tif (argc)\n \t\treturn error(_(\"'%s' requires 0 arguments\"),\n \t\t\t \"git bisect next\");\n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_next(&terms, prefix);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1417,7 +1419,8 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS\n \tstruct bisect_terms terms = { 0 };\n \n \tset_terms(&terms, \"bad\", \"good\");\n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_skip(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1429,7 +1432,8 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix\n \tint res;\n \tstruct bisect_terms terms = { 0 };\n \n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_visualize(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1443,7 +1447,8 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE\n \n \tif (!argc)\n \t\treturn error(_(\"'%s' failed: no command provided.\"), \"git bisect run\");\n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_run(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1482,7 +1487,8 @@ int cmd_bisect(int argc,\n \t\t\tusage_with_options(git_bisect_usage, options);\n \n \t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tget_terms(&terms);\n+\t\tif (get_terms(&terms, 1))\n+\t\t\treturn error(_(\"no terms defined\"));\n \t\tif (check_and_set_terms(&terms, argv[0]) ||\n \t\t !one_of(argv[0], terms.term_good, terms.term_bad, NULL))\n \t\t\tusage_msg_optf(_(\"unknown command: '%s'\"), git_bisect_usage,\n-- \ngitgitgadget\n\n"},{"id":"549764","messageId":"829cd82177a8e72e450d42db2af3166123c5b7c6.1785954661.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v2.git.1785954661.gitgitgadget@gmail.com","subject":"[PATCH v2 11/11] bisect: handle dup() failure when redirecting stdout","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-05T18:31:00Z","receivedAt":"2026-08-05T18:31:24Z","isPatch":true,"body":"From: Johannes Schindelin \n\nTo capture the output of each verdict command, bisect_run()\ntemporarily redirects stdout to a temporary file via the classic\ndup(1) / dup2() pair, restoring it afterwards. The return value of\ndup(1) is not checked, however. When it fails, the saved descriptor\nis -1, which is then passed to close() (the issue Coverity flags),\nand the matching dup2() that is meant to restore stdout also fails,\nleaving the process with stdout still pointing at the temporary file\nfor the remainder of the run.\n\nTreat a failed dup(1) or dup2(..., 1) as a fatal error for this bisect\nstep: close the temporary file descriptor, report the error via\nerror_errno(), and break out of the loop so the existing cleanup path\nhandles the rest, just as on other failure paths in this function.\n\nReported by Coverity as CID 1508242 (\"Improper use of negative\nvalue\").\n\nAssisted-by: Opus 4.7\nHelped-by: Patrick Steinhardt \nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex ceb60b0626..733d28d377 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1308,7 +1308,12 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n \n \t\tfflush(stdout);\n \t\tsaved_stdout = dup(1);\n-\t\tdup2(temporary_stdout_fd, 1);\n+\t\tif (saved_stdout < 0 ||\n+\t\t dup2(temporary_stdout_fd, 1) < 0) {\n+\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n+\t\t\tclose(temporary_stdout_fd);\n+\t\t\tbreak;\n+\t\t}\n \n \t\tres = bisect_state(terms, 1, &new_state);\n \n-- \ngitgitgadget\n"},{"id":"549775","messageId":"xmqqecgcpb4p.fsf@gitster.g","threadId":"65998","inReplyTo":"9a9103096a2bd877f84502cffefd019d0a6e229d.1785954661.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 10/11] bisect: check get_terms return at all call sites","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-05T20:26:14Z","receivedAt":"2026-08-05T20:26:16Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> There is one slight complication here: One caller _needs_ the return\n> value to indicate an error when the `BISECT_TERMS` file is absent, all\n> the other call sites are totally okay with a \"missing\" `BISECT_TERMS`\n> file. To address that, extend the function signature of `get_terms()` to\n> indicate which behavior the caller wants.\n\n> -static int get_terms(struct bisect_terms *terms)\n> +static int get_terms(struct bisect_terms *terms, int file_missing_is_ok)\n> {\n> \tstruct strbuf str = STRBUF_INIT;\n> \tFILE *fp = NULL;\n> @@ -493,7 +493,7 @@ static int get_terms(struct bisect_terms *terms)\n> \n> \tfp = fopen(git_path_bisect_terms(), \"r\");\n> \tif (!fp) {\n> -\t\tres = -1;\n> +\t\tres = file_missing_is_ok ? 0 : -1;\n> \t\tgoto finish;\n> \t}\n\nHmph. So, depending on the caller, a missing file error may have to\nbe treated as OK or as an error, while all other kinds of anomalies\nare treated by all callers as errors.\n\nAs all the existing callsites of this function need to be adjusted\nfor this change anyway, I would have thought a more typical way to\nhandle a situation like this would be to define different error\ncodes for this function and have the callers deal with them. But it\nseems that almost all callers, except for one, pass \"missing is OK.\"\n\nSo, instead of adjusting the majority of callers with something like:\n\n - if (get_terms(...))\n + if (get_terms(...) == BISECT_TERMS_ERROR)\n oops we got an error\n\nand keeping only the single oddball caller to barf on any non-zero\nreturn, \n\n - if (get_terms(...))\n + switch (get_terms(...)) {\n\t+\tcase BISECT_TERMS_ERROR:\n oops we got an error\n\t+\t\tbreak;\n\t+\tcase BISECT_TERMS_MISSING_FILE:\n\t+\t\tdeal with the missing file error\n\t+\t\tbreak;\n\t+\tdefault:\n\t+\t\tbreak; /* ok */\n\t+\t}\n\nit may be simpler to change:\n\n - if (get_terms(...))\n + if (get_terms(..., 1))\n oops we got an error\n\nfor the majority of them. The one oddball caller then becomes:\n\n - if (get_terms(...))\n + if (get_terms(..., 0))\n oops we got an error\n\nto treat a missing file as an error as well.\n\nI guess I can buy that.\n\nIf get_terms() were a public function that had many more callers,\nmy preference would probably be very different. But this is local\nto a single file, so the meaning of the mysterious 0/1 parameter\nwill quickly become evident to those who have to work with this\npart of the system anyway.\n\nThanks.\n"},{"id":"549783","messageId":"xmqqse4sm4ss.fsf@gitster.g","threadId":"65998","inReplyTo":"9bf7e737c740d8a80467ee3b38df9c86bbf7a566.1785954661.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 03/11] reftable/block: check deflateInit() return value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-06T01:11:15Z","receivedAt":"2026-08-06T01:11:17Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> The function already uses REFTABLE_ZLIB_ERROR for deflate()\n> failures later in the code path (lines 171, 199), so returning\n> the same error code for deflateInit() failure is consistent.\n>\n> Pointed out by Coverity.\n>\n> Assisted-by: Claude Opus 4.6\n> Signed-off-by: Johannes Schindelin \n> ---\n> reftable/block.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/reftable/block.c b/reftable/block.c\n> index 920b3f4486..ec81fd0493 100644\n> --- a/reftable/block.c\n> +++ b/reftable/block.c\n> @@ -87,7 +87,8 @@ int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,\n> \t\tREFTABLE_CALLOC_ARRAY(bw->zstream, 1);\n> \t\tif (!bw->zstream)\n> \t\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n> -\t\tdeflateInit(bw->zstream, 9);\n> +\t\tif (deflateInit(bw->zstream, 9) != Z_OK)\n> +\t\t\treturn REFTABLE_ZLIB_ERROR;\n> \t}\n\nPresumably bw->zstream occupies some memory allocated on the heap.\nDoes a failing deflateInit() release it? If not, do we leak memory\nhere? Or do we need\n\n\t\tif (deflateInit(bw->zstream, 9) !+ Z_OK) {\n\t\t\tREFTABLE_FREE_AND_NULL(bw->zstream);\n\t\t\treturn REFTABLE_ZLIB_ERROR;\n\t\t}\n\nhere?\n\nNoticing and returning an error is a good first step. The only\ncaller of it is reftable/writer.c:writer_reinit_block_writer(), and\nit checks and relays the error code from here to its callers, but\nnot all callers of it check the error condition. The most blatant\noffender being reftable_writer_new() that happily keeps going. I do\nnot know if we end up calling zlib on bw->zstream for such a broken\nblock_writer(), as I didn't trace the call graph fully myself.\n\nStepping back a bit, if REFTABLE_CALLOC_ARRAY() fails, bw->zstream\nwould be NULL, and a caller that does not check the return value of\nwriter_reinit_block_writer() would be holding a block writer whose\nzstream is NULL. If the block writer is eventually passed to the\nblock_writer_release() function, we would call deflateEnd() on it.\n\n"},{"id":"549859","messageId":"20260806154103.GA1625706@coredump.intra.peff.net","threadId":"65998","inReplyTo":"829cd82177a8e72e450d42db2af3166123c5b7c6.1785954661.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 11/11] bisect: handle dup() failure when redirecting stdout","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-06T15:41:03Z","receivedAt":"2026-08-06T15:41:05Z","isPatch":true,"body":"On Wed, Aug 05, 2026 at 06:31:00PM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index ceb60b0626..733d28d377 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -1308,7 +1308,12 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n> \n> \t\tfflush(stdout);\n> \t\tsaved_stdout = dup(1);\n> -\t\tdup2(temporary_stdout_fd, 1);\n> +\t\tif (saved_stdout < 0 ||\n> +\t\t dup2(temporary_stdout_fd, 1) < 0) {\n> +\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n> +\t\t\tclose(temporary_stdout_fd);\n> +\t\t\tbreak;\n> +\t\t}\n\nIronically this produces a new Coverity complaint. ;)\n\nIf dup2() fails, then we break out of the loop, leaking saved_stdout.\n\n-Peff\n"},{"id":"549872","messageId":"xmqqtsp7jguc.fsf@gitster.g","threadId":"65998","inReplyTo":"20260806154103.GA1625706@coredump.intra.peff.net","subject":"Re: [PATCH v2 11/11] bisect: handle dup() failure when redirecting stdout","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-06T17:31:39Z","receivedAt":"2026-08-06T17:31:42Z","isPatch":true,"body":"Jeff King writes:\n\n> On Wed, Aug 05, 2026 at 06:31:00PM +0000, Johannes Schindelin via GitGitGadget wrote:\n>\n>> diff --git a/builtin/bisect.c b/builtin/bisect.c\n>> index ceb60b0626..733d28d377 100644\n>> --- a/builtin/bisect.c\n>> +++ b/builtin/bisect.c\n>> @@ -1308,7 +1308,12 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n>> \n>> \t\tfflush(stdout);\n>> \t\tsaved_stdout = dup(1);\n>> -\t\tdup2(temporary_stdout_fd, 1);\n>> +\t\tif (saved_stdout < 0 ||\n>> +\t\t dup2(temporary_stdout_fd, 1) < 0) {\n>> +\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n>> +\t\t\tclose(temporary_stdout_fd);\n>> +\t\t\tbreak;\n>> +\t\t}\n>\n> Ironically this produces a new Coverity complaint. ;)\n>\n> If dup2() fails, then we break out of the loop, leaking saved_stdout.\n\nI didn't notice it while I was looking at this part, and wondering\nif we can (and should) do anything if close() failed there.\n"},{"id":"550366","messageId":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.git.1784069325.gitgitgadget@gmail.com","subject":"[PATCH v3 00/12] coverity: fix unchecked returns","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:08Z","receivedAt":"2026-08-12T08:03:24Z","isPatch":true,"body":"This is the next batch of fixes in response to issues reported by Coverity.\n\nChanges since v2:\n\n * Added a new commit to handle block-writer initialization errors (instead\n of ignoring them).\n * The bw->zstream attribute is now also deinitialized in the error case, as\n suggested by Junio.\n * The commit message of \"reftable/block: check deflateInit() return value\"\n was rephrased to stop suggesting that silent corruption by zlib would be\n possible before that patch: This turned out to be provably incorrect.\n * When aborting the bisect because dup2() failed, a left-over saved_stdout\n is now also cleaned up.\n\nChanges since v1:\n\n * The last-modified patch is now more careful to clean up a commit slab\n when parsing the commit failed.\n * When the \"good\" bisect term was read successfully, but not the \"bad\" one,\n the \"good\" one is now cleaned up.\n * Instead of detecting failed get_terms() calls indirectly, the return\n value is now checked.\n * Failures when bisect_run() calls dup2() are now handled properly, too.\n\nJohannes Schindelin (12):\n http: die on curl_easy_duphandle failure in get_active_slot\n config: propagate launch_editor() failure in show_editor()\n reftable: handle block-writer initialization errors\n reftable/block: check deflateInit() return value\n reftable tests: check reftable_table_init_ref_iterator() return\n last-modified: handle repo_parse_commit() failures\n compat/pread: check initial lseek for errors\n transport-helper: check dup() return in get_exporter\n transport-helper: warn when export-marks file cannot be finalized\n bisect: check strbuf_getline_lf return when reading terms\n bisect: check get_terms return at all call sites\n bisect: handle dup() failure when redirecting stdout\n\n bisect.c | 6 +++--\n builtin/bisect.c | 44 ++++++++++++++++++++++++---------\n builtin/config.c | 5 +++-\n builtin/last-modified.c | 9 ++++---\n compat/pread.c | 2 ++\n http.c | 2 ++\n reftable/block.c | 5 +++-\n reftable/writer.c | 8 +++++-\n t/unit-tests/u-reftable-table.c | 6 +++--\n transport-helper.c | 6 ++++-\n 10 files changed, 70 insertions(+), 23 deletions(-)\n\n\nbase-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2179%2Fdscho%2Fcoverity-fixes-unchecked-returns-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2179/dscho/coverity-fixes-unchecked-returns-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/2179\n\nRange-diff vs v2:\n\n 1: e653255de1 = 1: e653255de1 http: die on curl_easy_duphandle failure in get_active_slot\n 2: 0692704d45 = 2: 0692704d45 config: propagate launch_editor() failure in show_editor()\n -: ---------- > 3: c689148aef reftable: handle block-writer initialization errors\n 3: 9bf7e737c7 ! 4: 66953a65d0 reftable/block: check deflateInit() return value\n @@ Commit message\n z_stream is left in an undefined state.\n \n Subsequent deflate() calls in block_writer_finish() then operate\n - on this uninitialized stream. Depending on the zlib\n - implementation, this can produce silently corrupted compressed\n - data (which would be written to the reftable file and discovered\n - only when a later reader fails to inflate) or crash outright.\n + on this uninitialized stream. Current zlib/zlib-ng versions handle\n + such a stream gracefully, by returning `Z_STREAM_ERROR`, so in\n + practice it would likely not result in catastrophic error.\n \n - The function already uses REFTABLE_ZLIB_ERROR for deflate()\n - failures later in the code path (lines 171, 199), so returning\n - the same error code for deflateInit() failure is consistent.\n + The function already uses REFTABLE_ZLIB_ERROR for deflate() failures\n + later in the code path, so returning the same error code for\n + deflateInit() failure is consistent.\n \n Pointed out by Coverity.\n \n Assisted-by: Claude Opus 4.6\n + Helped-by: Junio C Hamano \n Signed-off-by: Johannes Schindelin \n \n ## reftable/block.c ##\n @@ reftable/block.c: int block_writer_init(struct block_writer *bw, uint8_t typ, ui\n \t\tif (!bw->zstream)\n \t\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n -\t\tdeflateInit(bw->zstream, 9);\n -+\t\tif (deflateInit(bw->zstream, 9) != Z_OK)\n ++\t\tif (deflateInit(bw->zstream, 9) != Z_OK) {\n ++\t\t\tREFTABLE_FREE_AND_NULL(bw->zstream);\n +\t\t\treturn REFTABLE_ZLIB_ERROR;\n ++\t\t}\n \t}\n \n \treturn 0;\n 4: 711671c3ab = 5: a49af20d30 reftable tests: check reftable_table_init_ref_iterator() return\n 5: 72a74c76be = 6: bf06239732 last-modified: handle repo_parse_commit() failures\n 6: f0b1e13979 = 7: 6e2295b8f0 compat/pread: check initial lseek for errors\n 7: 0facb9e8ca = 8: 689bb48fe5 transport-helper: check dup() return in get_exporter\n 8: 2b0e4f32fd = 9: ad6ea19737 transport-helper: warn when export-marks file cannot be finalized\n 9: 7f2b963103 = 10: 7db6ac2ab0 bisect: check strbuf_getline_lf return when reading terms\n 10: 9a9103096a = 11: aefdbe2bdf bisect: check get_terms return at all call sites\n 11: 829cd82177 ! 12: 258dbb0fbd bisect: handle dup() failure when redirecting stdout\n @@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co\n +\t\tif (saved_stdout < 0 ||\n +\t\t dup2(temporary_stdout_fd, 1) < 0) {\n +\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n ++\t\t\tif (saved_stdout >= 0)\n ++\t\t\t\tclose(saved_stdout);\n +\t\t\tclose(temporary_stdout_fd);\n +\t\t\tbreak;\n +\t\t}\n\n-- \ngitgitgadget\n"},{"id":"550367","messageId":"e653255de19decfe45d4ef8d3277aaf69c44c391.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 01/12] http: die on curl_easy_duphandle failure in get_active_slot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:09Z","receivedAt":"2026-08-12T08:03:26Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_active_slot() duplicates the default curl handle via\ncurl_easy_duphandle() to create a per-slot session handle. The\nreturn value is stored directly in slot->curl without checking\nfor NULL. curl_easy_duphandle() can return NULL when memory\nallocation fails internally, and the libcurl documentation\nexplicitly states this possibility.\n\nWhen this happens, slot->curl is NULL and the very next operation\n(curl_easy_setopt on line 1632 for CURLOPT_COOKIEFILE) passes\nNULL as the curl handle, which is undefined behavior in libcurl\nand typically crashes.\n\nEvery HTTP operation in git goes through get_active_slot(), so\nthis affects all remote-https, remote-http, and HTTP-based\noperations (clone, fetch, push over HTTP, bundle-uri downloads).\n\nAdd a NULL check and die() with a clear message. There is no\nreasonable recovery from a failed handle duplication: the process\nis out of memory and cannot perform any HTTP operation.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n http.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex b4e7b8d00b..8f1d6d1f56 100644\n--- a/http.c\n+++ b/http.c\n@@ -1608,6 +1608,8 @@ struct active_request_slot *get_active_slot(void)\n \n \tif (!slot->curl) {\n \t\tslot->curl = curl_easy_duphandle(curl_default);\n+\t\tif (!slot->curl)\n+\t\t\tdie(\"curl_easy_duphandle failed\");\n \t\tcurl_session_count++;\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"550368","messageId":"0692704d45060a62579b50dd7a2f07da04f435c8.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 02/12] config: propagate launch_editor() failure in show_editor()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:10Z","receivedAt":"2026-08-12T08:03:28Z","isPatch":true,"body":"From: Johannes Schindelin \n\nshow_editor() calls launch_editor() to open the user's editor on\nthe configuration file, but discards the return value and\nunconditionally returns 0 (success). When the editor fails to\nlaunch (e.g., $EDITOR is not found, or the editor exits with a\nnonzero status), the caller receives no indication that anything\nwent wrong.\n\nThis affects \"git config edit\" and \"git config --edit\": the\ncommand silently succeeds even when the editor could not be\nstarted. In contrast, other editor-launching paths in git (such\nas \"git commit\" and \"git rebase --edit-todo\") properly propagate\neditor failures and exit with an error.\n\nCheck the return value and propagate the failure by returning -1.\nThe two callers (cmd_config_edit at line 1315 and the legacy\ncmd_config at line 1478) both propagate this return to\nhandle_builtin, which translates negative returns into an error\nexit.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/config.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 8d8ec0beea..1307fdb0d6 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -1313,7 +1313,10 @@ static int show_editor(struct config_location_options *opts)\n \t\telse if (errno != EEXIST)\n \t\t\tdie_errno(_(\"cannot create configuration file %s\"), config_file);\n \t}\n-\tlaunch_editor(config_file, NULL, NULL);\n+\tif (launch_editor(config_file, NULL, NULL)) {\n+\t\tfree(config_file);\n+\t\treturn -1;\n+\t}\n \tfree(config_file);\n \n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"550369","messageId":"c689148aef4302df2a370b78f30c185cd21a42c9.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 03/12] reftable: handle block-writer initialization errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:11Z","receivedAt":"2026-08-12T08:03:32Z","isPatch":true,"body":"From: Johannes Schindelin \n\n2d5dbb37b284 (reftable/block: handle allocation failures, 2024-10-02)\ntaught `writer_reinit_block_writer()` to report initialization failures\nand updated its callers, but `reftable_writer_new()` continued to ignore\nthe return value.\n\nConsequently, the constructor could report success after block-writer\ninitialization had failed. Propagate the error and release the\nconstructor's allocations instead of returning an unusable writer.\n\nPointed out by GPT-5.6 Sol and Claude Opus 4.8.\n\nSigned-off-by: Johannes Schindelin \n---\n reftable/writer.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex d969a6a021..073b9bbd89 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -150,6 +150,7 @@ int reftable_writer_new(struct reftable_writer **out,\n {\n \tstruct reftable_write_options opts = {0};\n \tstruct reftable_writer *wp;\n+\tint err;\n \n \tif (_opts)\n \t\topts = *_opts;\n@@ -177,7 +178,12 @@ int reftable_writer_new(struct reftable_writer **out,\n \twp->opts = opts;\n \twp->hash_id = hash_id;\n \twp->flush = flush_func;\n-\twriter_reinit_block_writer(wp, REFTABLE_BLOCK_TYPE_REF);\n+\terr = writer_reinit_block_writer(wp, REFTABLE_BLOCK_TYPE_REF);\n+\tif (err < 0) {\n+\t\treftable_free(wp->block);\n+\t\treftable_free(wp);\n+\t\treturn err;\n+\t}\n \n \t*out = wp;\n \n-- \ngitgitgadget\n\n"},{"id":"550370","messageId":"66953a65d0a3400eccce197524225a0f71a584da.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 04/12] reftable/block: check deflateInit() return value","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:12Z","receivedAt":"2026-08-12T08:03:34Z","isPatch":true,"body":"From: Johannes Schindelin \n\nblock_writer_init() allocates a z_stream and calls deflateInit()\nto prepare it for compressing log records. The return value of\ndeflateInit() is silently discarded. If zlib initialization fails\n(e.g., Z_MEM_ERROR when the system is under memory pressure), the\nz_stream is left in an undefined state.\n\nSubsequent deflate() calls in block_writer_finish() then operate\non this uninitialized stream. Current zlib/zlib-ng versions handle\nsuch a stream gracefully, by returning `Z_STREAM_ERROR`, so in\npractice it would likely not result in catastrophic error.\n\nThe function already uses REFTABLE_ZLIB_ERROR for deflate() failures\nlater in the code path, so returning the same error code for\ndeflateInit() failure is consistent.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nHelped-by: Junio C Hamano \nSigned-off-by: Johannes Schindelin \n---\n reftable/block.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/reftable/block.c b/reftable/block.c\nindex 920b3f4486..c12fedc5a2 100644\n--- a/reftable/block.c\n+++ b/reftable/block.c\n@@ -87,7 +87,10 @@ int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,\n \t\tREFTABLE_CALLOC_ARRAY(bw->zstream, 1);\n \t\tif (!bw->zstream)\n \t\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n-\t\tdeflateInit(bw->zstream, 9);\n+\t\tif (deflateInit(bw->zstream, 9) != Z_OK) {\n+\t\t\tREFTABLE_FREE_AND_NULL(bw->zstream);\n+\t\t\treturn REFTABLE_ZLIB_ERROR;\n+\t\t}\n \t}\n \n \treturn 0;\n-- \ngitgitgadget\n\n"},{"id":"550371","messageId":"a49af20d3052a7c905920dfd830ff95996d11bc0.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 05/12] reftable tests: check reftable_table_init_ref_iterator() return","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:13Z","receivedAt":"2026-08-12T08:03:35Z","isPatch":true,"body":"From: Johannes Schindelin \n\ntest_reftable_table__seek_once() and test_reftable_table__reseek()\nboth call reftable_table_init_ref_iterator() without checking its\nreturn value. This function returns an int error code (0 on\nsuccess, negative on failure). Every other reftable function call\nin these same tests checks the return via cl_assert_equal_i() or\ncl_assert(), making this omission inconsistent.\n\nIf the iterator initialization ever fails (e.g., due to a memory\nallocation failure in the reftable internals), the test would\nproceed to seek and read with an uninitialized iterator, producing\nmisleading test results or crashes rather than a clear assertion\nfailure.\n\nCheck the return value via cl_assert_equal_i(ret, 0), consistent\nwith the surrounding code.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n t/unit-tests/u-reftable-table.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c\nindex fae478ee04..6f444f8cf9 100644\n--- a/t/unit-tests/u-reftable-table.c\n+++ b/t/unit-tests/u-reftable-table.c\n@@ -29,7 +29,8 @@ void test_reftable_table__seek_once(void)\n \tret = reftable_table_new(&table, &source, \"name\");\n \tcl_assert(!ret);\n \n-\treftable_table_init_ref_iterator(table, &it);\n+\tret = reftable_table_init_ref_iterator(table, &it);\n+\tcl_assert_equal_i(ret, 0);\n \tret = reftable_iterator_seek_ref(&it, \"\");\n \tcl_assert(!ret);\n \tret = reftable_iterator_next_ref(&it, &ref);\n@@ -71,7 +72,8 @@ void test_reftable_table__reseek(void)\n \tret = reftable_table_new(&table, &source, \"name\");\n \tcl_assert(!ret);\n \n-\treftable_table_init_ref_iterator(table, &it);\n+\tret = reftable_table_init_ref_iterator(table, &it);\n+\tcl_assert_equal_i(ret, 0);\n \n \tfor (size_t i = 0; i < 5; i++) {\n \t\tret = reftable_iterator_seek_ref(&it, \"\");\n-- \ngitgitgadget\n\n"},{"id":"550372","messageId":"bf062397320e3e3b5a25505c023eb5c3b2eab87f.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 06/12] last-modified: handle repo_parse_commit() failures","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:14Z","receivedAt":"2026-08-12T08:03:37Z","isPatch":true,"body":"From: Johannes Schindelin \n\nlast_modified_run() and process_parent() call repo_parse_commit()\nwithout checking the return value at three sites. When a commit\nobject is corrupt or unavailable (e.g., a shallow clone boundary\nor a missing object in a partial clone), the parse fails and the\ncommit's internal fields (parents, tree, date) are not populated.\n\nThe consequences depend on which call site fails:\n\nAt line 417 (the main walk loop), c->parents stays NULL after a\nfailed parse. The parent-walking loop at line 440 simply does not\nexecute, silently treating the unparsable commit as a root commit.\nThis produces incorrect \"last modified\" results: paths changed in\nancestors beyond the corrupt commit are attributed to the wrong\ncommit or not reported at all.\n\nAt line 423 (the --not exclusion walk), n->parents stays NULL,\ncausing the exclusion walk to stop prematurely. Commits that\nshould be excluded from the output may be incorrectly included.\n\nAt line 293 (process_parent), the parent's tree and parents are\nunavailable, so diff operations against it produce wrong results\nand the parent's own ancestors are never enqueued for walking.\n\nSkip unparsable commits by checking the return value and\ncontinuing to the next iteration (or returning early in\nprocess_parent). This matches the defensive pattern used in other\nrevision walkers such as limit_list() and get_revision_internal().\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nHelped-by: Junio C Hamano \nSigned-off-by: Johannes Schindelin \n---\n builtin/last-modified.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/last-modified.c b/builtin/last-modified.c\nindex 5478182f2e..3846244dfc 100644\n--- a/builtin/last-modified.c\n+++ b/builtin/last-modified.c\n@@ -290,7 +290,8 @@ static void process_parent(struct last_modified *lm,\n {\n \tstruct bitmap *active_p;\n \n-\trepo_parse_commit(lm->rev.repo, parent);\n+\tif (repo_parse_commit(lm->rev.repo, parent))\n+\t\treturn;\n \tactive_p = active_paths_for(lm, parent);\n \n \t/*\n@@ -414,12 +415,14 @@ static int last_modified_run(struct last_modified *lm)\n \t\t * Otherwise, make sure that 'c' isn't reachable from anything\n \t\t * in the '--not' queue.\n \t\t */\n-\t\trepo_parse_commit(lm->rev.repo, c);\n+\t\tif (repo_parse_commit(lm->rev.repo, c))\n+\t\t\tgoto cleanup;\n \n \t\twhile ((n = prio_queue_get(¬_queue))) {\n \t\t\tstruct commit_list *np;\n \n-\t\t\trepo_parse_commit(lm->rev.repo, n);\n+\t\t\tif (repo_parse_commit(lm->rev.repo, n))\n+\t\t\t\tcontinue;\n \n \t\t\tfor (np = n->parents; np; np = np->next) {\n \t\t\t\tif (!(np->item->object.flags & PARENT2)) {\n-- \ngitgitgadget\n\n"},{"id":"550374","messageId":"6e2295b8f039ccbc3d4432d1e37bd1414b7facbd.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 07/12] compat/pread: check initial lseek for errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:15Z","receivedAt":"2026-08-12T08:03:39Z","isPatch":true,"body":"From: Johannes Schindelin \n\ngit_pread() saves the current file offset via lseek(fd, 0,\nSEEK_CUR) and later restores it. If the initial lseek fails\n(e.g., the fd is a pipe or otherwise non-seekable),\ncurrent_offset is -1. This negative value is later passed to\nlseek(fd, -1, SEEK_SET) at line 16, which sets the file position\nto an unintended location (or fails with EINVAL on some\nplatforms).\n\nCheck the initial lseek return value and return -1 immediately\nif it fails, consistent with the error handling for the other\nlseek calls in the same function.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n compat/pread.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/compat/pread.c b/compat/pread.c\nindex 484e6d4c71..ac7d058cb8 100644\n--- a/compat/pread.c\n+++ b/compat/pread.c\n@@ -7,6 +7,8 @@ ssize_t git_pread(int fd, void *buf, size_t count, off_t offset)\n ssize_t rc;\n \n current_offset = lseek(fd, 0, SEEK_CUR);\n+\tif (current_offset < 0)\n+\t\treturn -1;\n \n if (lseek(fd, offset, SEEK_SET) < 0)\n return -1;\n-- \ngitgitgadget\n\n"},{"id":"550373","messageId":"689bb48fe58186b8478c30ef906ebec4a4d2e091.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 08/12] transport-helper: check dup() return in get_exporter","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:16Z","receivedAt":"2026-08-12T08:03:41Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_exporter() duplicates helper->in via dup() and stores the\nresult in fastexport->out. If dup() fails (fd exhaustion), it\nreturns -1. The child_process machinery interprets out = -1 as\n\"create a pipe for stdout\", which would silently change the\nfast-export process's output wiring: instead of sending data\nback through the helper's input fd, it would write to a new pipe\nthat nobody reads from.\n\nCheck the return value and report the error before proceeding.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n transport-helper.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 80f90eb7ba..31883b244e 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -487,6 +487,8 @@ static int get_exporter(struct transport *transport,\n \t/* we need to duplicate helper->in because we want to use it after\n \t * fastexport is done with it. */\n \tfastexport->out = dup(helper->in);\n+\tif (fastexport->out < 0)\n+\t\treturn error_errno(_(\"could not dup helper output fd\"));\n \tstrvec_push(&fastexport->args, \"fast-export\");\n \tstrvec_push(&fastexport->args, \"--use-done-feature\");\n \tstrvec_push(&fastexport->args, data->signed_tags ?\n-- \ngitgitgadget\n\n"},{"id":"550375","messageId":"ad6ea197374f48f0837a40993588ae0cf69affc6.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 09/12] transport-helper: warn when export-marks file cannot be finalized","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:17Z","receivedAt":"2026-08-12T08:03:43Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen push_refs_with_export() finalizes a successful push, it writes\nthe fast-export marks file to a .tmp sibling and rename()s it into\nplace. The return value of rename() is currently ignored. If the\nrename fails (permission denied, full disk, or an antivirus product\nlocking the destination on Windows), the .tmp file is left behind\nand the existing export_marks file remains stale; the next\nfast-export operation that resumes from it then silently operates on\ninconsistent bookkeeping.\n\nThe push itself succeeded by that point, so promoting this to a\nfatal error would be inappropriate. Emit warning_errno() naming both\npaths so the user can recover manually, and keep returning 0.\n\nFlagged by Coverity as CID 1427723 (\"Unchecked return value\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n transport-helper.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 31883b244e..ed0543f1ad 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -1184,7 +1184,9 @@ static int push_refs_with_export(struct transport *transport,\n \n \tif (data->export_marks) {\n \t\tstrbuf_addf(&buf, \"%s.tmp\", data->export_marks);\n-\t\trename(buf.buf, data->export_marks);\n+\t\tif (rename(buf.buf, data->export_marks))\n+\t\t\twarning_errno(_(\"could not rename '%s' to '%s'\"),\n+\t\t\t\t buf.buf, data->export_marks);\n \t\tstrbuf_release(&buf);\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"550376","messageId":"7db6ac2ab0d346da992b812c8588d90b16aebb49.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 10/12] bisect: check strbuf_getline_lf return when reading terms","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:18Z","receivedAt":"2026-08-12T08:03:44Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_terms() in builtin/bisect.c and read_bisect_terms() in\nbisect.c both read the BISECT_TERMS file but do not check the\nstrbuf_getline_lf() return values. If the file is truncated\n(e.g., a partial write from a crash or disk-full condition),\nstrbuf_getline_lf returns EOF and the strbuf remains empty.\nstrbuf_detach then returns an empty string, and the term names\nsilently become \"\" instead of the expected \"bad\"/\"good\" or\ncustom terms.\n\nIn get_terms(), check for EOF and return -1 on truncation,\nmatching the existing -1 return for a missing file.\n\nIn read_bisect_terms(), die with a descriptive message when a\nline cannot be read, consistent with the die_errno for a\nnon-ENOENT open failure in the same function. Unlike get_terms(),\nread_bisect_terms() returns void and uses die() for all error\npaths, so the die is the appropriate error handling here.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nHelped-by: Junio C Hamano \nSigned-off-by: Johannes Schindelin \n---\n bisect.c | 6 ++++--\n builtin/bisect.c | 11 +++++++++--\n 2 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 94c7028d2a..c2ef5da462 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1019,10 +1019,12 @@ void read_bisect_terms(char **read_bad, char **read_good)\n \t\t\tdie_errno(_(\"could not read file '%s'\"), filename);\n \t\t}\n \t} else {\n-\t\tstrbuf_getline_lf(&str, fp);\n+\t\tif (strbuf_getline_lf(&str, fp) == EOF)\n+\t\t\tdie(_(\"could not read bad term from file '%s'\"), filename);\n \t\tfree(*read_bad);\n \t\t*read_bad = strbuf_detach(&str, NULL);\n-\t\tstrbuf_getline_lf(&str, fp);\n+\t\tif (strbuf_getline_lf(&str, fp) == EOF)\n+\t\t\tdie(_(\"could not read good term from file '%s'\"), filename);\n \t\tfree(*read_good);\n \t\t*read_good = strbuf_detach(&str, NULL);\n \t}\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 798e28f501..69ab7ea248 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -498,9 +498,16 @@ static int get_terms(struct bisect_terms *terms)\n \t}\n \n \tfree_terms(terms);\n-\tstrbuf_getline_lf(&str, fp);\n+\tif (strbuf_getline_lf(&str, fp) == EOF) {\n+\t\tres = -1;\n+\t\tgoto finish;\n+\t}\n \tterms->term_bad = strbuf_detach(&str, NULL);\n-\tstrbuf_getline_lf(&str, fp);\n+\tif (strbuf_getline_lf(&str, fp) == EOF) {\n+\t\tres = -1;\n+\t\tFREE_AND_NULL(terms->term_bad);\n+\t\tgoto finish;\n+\t}\n \tterms->term_good = strbuf_detach(&str, NULL);\n \n finish:\n-- \ngitgitgadget\n\n"},{"id":"550377","messageId":"aefdbe2bdfe7509e1660aa55e46bbdb79ddf619c.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 11/12] bisect: check get_terms return at all call sites","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:19Z","receivedAt":"2026-08-12T08:03:46Z","isPatch":true,"body":"From: Johannes Schindelin \n\nSix callers of get_terms() silently discard its return value. When\nget_terms fails (missing or truncated BISECT_TERMS file), the term\nstrings remain NULL or empty, causing confusing downstream\nbehavior: commands like \"bisect next\" or \"bisect run\" proceed with\nempty term strings, producing nonsensical ref names (refs/bisect/\nwith no suffix) and misleading error messages.\n\nLet's not discard the return value, but handle an error with the same\nmessage `bisect_terms()` already uses when reading the terms failed.\n\nPointed out by Coverity.\n\nThere is one slight complication here: One caller _needs_ the return\nvalue to indicate an error when the `BISECT_TERMS` file is absent, all\nthe other call sites are totally okay with a \"missing\" `BISECT_TERMS`\nfile. To address that, extend the function signature of `get_terms()` to\nindicate which behavior the caller wants.\n\nAssisted-by: Claude Opus 4.6\nHelped-by: Patrick Steinhardt \nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 24 +++++++++++++++---------\n 1 file changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 69ab7ea248..ceb60b0626 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -485,7 +485,7 @@ static int bisect_next_check(const struct bisect_terms *terms,\n \treturn decide_next(terms, current_term, !state.nr_good, !state.nr_bad);\n }\n \n-static int get_terms(struct bisect_terms *terms)\n+static int get_terms(struct bisect_terms *terms, int file_missing_is_ok)\n {\n \tstruct strbuf str = STRBUF_INIT;\n \tFILE *fp = NULL;\n@@ -493,7 +493,7 @@ static int get_terms(struct bisect_terms *terms)\n \n \tfp = fopen(git_path_bisect_terms(), \"r\");\n \tif (!fp) {\n-\t\tres = -1;\n+\t\tres = file_missing_is_ok ? 0 : -1;\n \t\tgoto finish;\n \t}\n \n@@ -519,7 +519,7 @@ finish:\n \n static int bisect_terms(struct bisect_terms *terms, const char *option)\n {\n-\tif (get_terms(terms))\n+\tif (get_terms(terms, 0))\n \t\treturn error(_(\"no terms defined\"));\n \n \tif (!option) {\n@@ -1057,7 +1057,8 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)\n \trev = word_end + strspn(word_end, \" \\t\");\n \t*word_end = '\\0'; /* NUL-terminate the word */\n \n-\tget_terms(terms);\n+\tif (get_terms(terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tif (check_and_set_terms(terms, p))\n \t\treturn -1;\n \n@@ -1383,7 +1384,8 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref\n \tif (argc)\n \t\treturn error(_(\"'%s' requires 0 arguments\"),\n \t\t\t \"git bisect next\");\n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_next(&terms, prefix);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1417,7 +1419,8 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS\n \tstruct bisect_terms terms = { 0 };\n \n \tset_terms(&terms, \"bad\", \"good\");\n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_skip(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1429,7 +1432,8 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix\n \tint res;\n \tstruct bisect_terms terms = { 0 };\n \n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_visualize(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1443,7 +1447,8 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE\n \n \tif (!argc)\n \t\treturn error(_(\"'%s' failed: no command provided.\"), \"git bisect run\");\n-\tget_terms(&terms);\n+\tif (get_terms(&terms, 1))\n+\t\treturn error(_(\"no terms defined\"));\n \tres = bisect_run(&terms, argc, argv);\n \tfree_terms(&terms);\n \treturn res;\n@@ -1482,7 +1487,8 @@ int cmd_bisect(int argc,\n \t\t\tusage_with_options(git_bisect_usage, options);\n \n \t\tset_terms(&terms, \"bad\", \"good\");\n-\t\tget_terms(&terms);\n+\t\tif (get_terms(&terms, 1))\n+\t\t\treturn error(_(\"no terms defined\"));\n \t\tif (check_and_set_terms(&terms, argv[0]) ||\n \t\t !one_of(argv[0], terms.term_good, terms.term_bad, NULL))\n \t\t\tusage_msg_optf(_(\"unknown command: '%s'\"), git_bisect_usage,\n-- \ngitgitgadget\n\n"},{"id":"550378","messageId":"258dbb0fbda31ab0627f9da179c1c37cdd64666c.1786521801.git.gitgitgadget@gmail.com","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"[PATCH v3 12/12] bisect: handle dup() failure when redirecting stdout","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T08:03:20Z","receivedAt":"2026-08-12T08:03:48Z","isPatch":true,"body":"From: Johannes Schindelin \n\nTo capture the output of each verdict command, bisect_run()\ntemporarily redirects stdout to a temporary file via the classic\ndup(1) / dup2() pair, restoring it afterwards. The return value of\ndup(1) is not checked, however. When it fails, the saved descriptor\nis -1, which is then passed to close() (the issue Coverity flags),\nand the matching dup2() that is meant to restore stdout also fails,\nleaving the process with stdout still pointing at the temporary file\nfor the remainder of the run.\n\nTreat a failed dup(1) or dup2(..., 1) as a fatal error for this bisect\nstep: close the temporary file descriptor, report the error via\nerror_errno(), and break out of the loop so the existing cleanup path\nhandles the rest, just as on other failure paths in this function.\n\nReported by Coverity as CID 1508242 (\"Improper use of negative\nvalue\").\n\nAssisted-by: Opus 4.7\nHelped-by: Patrick Steinhardt \nSigned-off-by: Johannes Schindelin \n---\n builtin/bisect.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex ceb60b0626..be42468af6 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -1308,7 +1308,14 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n \n \t\tfflush(stdout);\n \t\tsaved_stdout = dup(1);\n-\t\tdup2(temporary_stdout_fd, 1);\n+\t\tif (saved_stdout < 0 ||\n+\t\t dup2(temporary_stdout_fd, 1) < 0) {\n+\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n+\t\t\tif (saved_stdout >= 0)\n+\t\t\t\tclose(saved_stdout);\n+\t\t\tclose(temporary_stdout_fd);\n+\t\t\tbreak;\n+\t\t}\n \n \t\tres = bisect_state(terms, 1, &new_state);\n \n-- \ngitgitgadget\n"},{"id":"550434","messageId":"xmqq5x1fxn5u.fsf@gitster.g","threadId":"65998","inReplyTo":"pull.2179.v3.git.1786521801.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 00/12] coverity: fix unchecked returns","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-12T17:29:33Z","receivedAt":"2026-08-12T17:29:36Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> This is the next batch of fixes in response to issues reported by Coverity.\n>\n> Changes since v2:\n>\n> * Added a new commit to handle block-writer initialization errors (instead\n> of ignoring them).\n> * The bw->zstream attribute is now also deinitialized in the error case, as\n> suggested by Junio.\n> * The commit message of \"reftable/block: check deflateInit() return value\"\n> was rephrased to stop suggesting that silent corruption by zlib would be\n> possible before that patch: This turned out to be provably incorrect.\n> * When aborting the bisect because dup2() failed, a left-over saved_stdout\n> is now also cleaned up.\n\nEverything looks sensible. I am fine with declaring victory, but\ndoes anyone want to second it?\n"},{"id":"550454","messageId":"20260812213336.GB152730@coredump.intra.peff.net","threadId":"65998","inReplyTo":"258dbb0fbda31ab0627f9da179c1c37cdd64666c.1786521801.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 12/12] bisect: handle dup() failure when redirecting stdout","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-12T21:33:36Z","receivedAt":"2026-08-12T21:33:38Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 08:03:20AM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> @@ -1308,7 +1308,14 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n> \n> \t\tfflush(stdout);\n> \t\tsaved_stdout = dup(1);\n> -\t\tdup2(temporary_stdout_fd, 1);\n> +\t\tif (saved_stdout < 0 ||\n> +\t\t dup2(temporary_stdout_fd, 1) < 0) {\n> +\t\t\tres = error_errno(_(\"could not duplicate stdout\"));\n> +\t\t\tif (saved_stdout >= 0)\n> +\t\t\t\tclose(saved_stdout);\n> +\t\t\tclose(temporary_stdout_fd);\n> +\t\t\tbreak;\n> +\t\t}\n\nOK. The extra \"if (saved_stdout >= 0)\" is not strictly necessary if we\nare OK considering close(-1) as a noop, but it doesn't hurt too much.\n\nIt could also be avoided with two separate checks:\n\n saved_stdout = dup(1);\n if (saved_stdout < 0)\n\t...\n if (dup2_temporary_stdout_fd, 1) < 0)\n\t...\n\nbut that would involve a little bit of repetition of the other cleanup\nlines (though it would also allow more specific error messages).\n\nProbably not worth polishing this further, though. What you have here is\ncorrect and I would be surprised if any user ever sees this error case.\nIt is mostly about covering all of the paths for leaks.\n\n-Peff\n"},{"id":"550456","messageId":"20260812213438.GC152730@coredump.intra.peff.net","threadId":"65998","inReplyTo":"xmqq5x1fxn5u.fsf@gitster.g","subject":"Re: [PATCH v3 00/12] coverity: fix unchecked returns","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-12T21:34:38Z","receivedAt":"2026-08-12T21:34:40Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 10:29:33AM -0700, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > This is the next batch of fixes in response to issues reported by Coverity.\n> >\n> > Changes since v2:\n> >\n> > * Added a new commit to handle block-writer initialization errors (instead\n> > of ignoring them).\n> > * The bw->zstream attribute is now also deinitialized in the error case, as\n> > suggested by Junio.\n> > * The commit message of \"reftable/block: check deflateInit() return value\"\n> > was rephrased to stop suggesting that silent corruption by zlib would be\n> > possible before that patch: This turned out to be provably incorrect.\n> > * When aborting the bisect because dup2() failed, a left-over saved_stdout\n> > is now also cleaned up.\n> \n> Everything looks sensible. I am fine with declaring victory, but\n> does anyone want to second it?\n\nI cannot claim to have read all of the patches carefully, but this\nversion addressed the sole concern I raised, and in the few other\npatches I glanced over I didn't see anything to complain about. So maybe\nconsider that a weak second. :)\n\n-Peff\n"},{"id":"550472","messageId":"an1kpG_KA-iNgyAO@pks.im","threadId":"65998","inReplyTo":"ad6ea197374f48f0837a40993588ae0cf69affc6.1786521801.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 09/12] transport-helper: warn when export-marks file cannot be finalized","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-13T06:31:15Z","receivedAt":"2026-08-13T06:31:28Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 08:03:17AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> When push_refs_with_export() finalizes a successful push, it writes\n> the fast-export marks file to a .tmp sibling and rename()s it into\n> place. The return value of rename() is currently ignored. If the\n> rename fails (permission denied, full disk, or an antivirus product\n> locking the destination on Windows), the .tmp file is left behind\n> and the existing export_marks file remains stale; the next\n> fast-export operation that resumes from it then silently operates on\n> inconsistent bookkeeping.\n\nOne question here would be whether we should try to unlink the file\ninstead if renaming it into place failed. But not doing so potentially\ngives the user the ability to fix that issue. So I'm not sure whether\nthat's really a sensible thing to do in the first place.\n\nIn any case, the post-image of this patch is a clear improvement as we\nnow enable the user to act on the warning in the first place, whereas\npreviously they wouldn't ever learn about it until the failed rename may\ncause errors. So overall I think this is okay as-is.\n\nPatrick\n"},{"id":"550473","messageId":"an1k0d5fI4EVsfsM@pks.im","threadId":"65998","inReplyTo":"20260812213438.GC152730@coredump.intra.peff.net","subject":"Re: [PATCH v3 00/12] coverity: fix unchecked returns","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-13T06:31:45Z","receivedAt":"2026-08-13T06:31:51Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 05:34:38PM -0400, Jeff King wrote:\n> On Wed, Aug 12, 2026 at 10:29:33AM -0700, Junio C Hamano wrote:\n> \n> > \"Johannes Schindelin via GitGitGadget\" \n> > writes:\n> > \n> > > This is the next batch of fixes in response to issues reported by Coverity.\n> > >\n> > > Changes since v2:\n> > >\n> > > * Added a new commit to handle block-writer initialization errors (instead\n> > > of ignoring them).\n> > > * The bw->zstream attribute is now also deinitialized in the error case, as\n> > > suggested by Junio.\n> > > * The commit message of \"reftable/block: check deflateInit() return value\"\n> > > was rephrased to stop suggesting that silent corruption by zlib would be\n> > > possible before that patch: This turned out to be provably incorrect.\n> > > * When aborting the bisect because dup2() failed, a left-over saved_stdout\n> > > is now also cleaned up.\n> > \n> > Everything looks sensible. I am fine with declaring victory, but\n> > does anyone want to second it?\n> \n> I cannot claim to have read all of the patches carefully, but this\n> version addressed the sole concern I raised, and in the few other\n> patches I glanced over I didn't see anything to complain about. So maybe\n> consider that a weak second. :)\n\nI didn't spot anything that needs to change, either. Thanks!\n\nPatrick\n"}]}