{"thread":{"id":"65900","subject":"[PATCH 00/13] coverity: fix leaks and error paths","startedAt":"2026-07-01T07:04:34Z","lastAt":"2026-07-07T19:25:49Z","messageCount":44,"participants":["Johannes Schindelin via GitGitGadget","Patrick Steinhardt","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"546830","messageId":"pull.2163.git.1782889472.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":null,"subject":"[PATCH 00/13] coverity: fix leaks and error paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:18Z","receivedAt":"2026-07-01T07:04:34Z","isPatch":true,"body":"I wanted to whittle down the many issues reported by Coverity in the Git for\nWindows project. Turns out: The vast majority of the issues are false\npositives. Most of the remaining issues are in core Git proper.\n\nThis effort was forced on pause while Coverity was down from May 16\n[https://web.archive.org/web/20260516152422/https://scan.coverity.com/] to\nJune 22\n[https://web.archive.org/web/20260622182153/https://scan.coverity.com/]).\n\nHere is a first batch of fixes for those issues.\n\nJohannes Schindelin (13):\n load_one_loose_object_map(): fix resource leak\n loose: avoid closing invalid fd on error path\n download_https_uri_to_file(): do not leak fd upon failure\n run-command: avoid close(-1) in start_command() error paths\n run_diff_files: avoid memory leak\n line-log: avoid redundant copy that leaks in process_ranges\n dir: free allocations on parse-error paths in read_one_dir()\n submodule: fix cwd leak in get_superproject_working_tree()\n worktree: fix resource leaks when branch creation fails\n imap-send: avoid leaking the IMAP upload buffer\n reftable/table: release filter on error path\n fsmonitor: plug token-data leak on early daemon-startup failures\n mingw: make exit_process() own the process handle on all paths\n\n builtin/fsmonitor--daemon.c | 2 ++\n builtin/worktree.c | 7 +++++--\n bundle-uri.c | 2 +-\n compat/mingw.c | 4 +---\n compat/win32/exit-process.h | 1 +\n diff-lib.c | 3 ++-\n dir.c | 9 +++++++--\n imap-send.c | 1 +\n line-log.c | 3 +--\n loose.c | 11 ++++++-----\n reftable/table.c | 4 ++++\n run-command.c | 6 +++---\n submodule.c | 8 ++++++--\n 13 files changed, 40 insertions(+), 21 deletions(-)\n\n\nbase-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2163%2Fdscho%2Fcoverity-fixes-leaks-and-error-paths-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2163/dscho/coverity-fixes-leaks-and-error-paths-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2163\n-- \ngitgitgadget\n"},{"id":"546831","messageId":"17242c249f0beb387fd30634663f13ce42d34f79.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 01/13] load_one_loose_object_map(): fix resource leak","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:19Z","receivedAt":"2026-07-01T07:04:36Z","isPatch":true,"body":"From: Johannes Schindelin \n\nPointed out by Coverity.\n\nWhile at it, reduce near-duplicate clean-up code at the end of the\nfunction.\n\nSigned-off-by: Johannes Schindelin \n---\n loose.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/loose.c b/loose.c\nindex 0b626c1b85..47b7f5ec38 100644\n--- a/loose.c\n+++ b/loose.c\n@@ -65,6 +65,7 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n {\n \tstruct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;\n \tFILE *fp;\n+\tint ret = -1;\n \n \tif (!loose->map)\n \t\tloose_object_map_init(&loose->map);\n@@ -98,13 +99,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n \t\tinsert_loose_map(loose, &oid, &compat_oid);\n \t}\n \n-\tstrbuf_release(&buf);\n-\tstrbuf_release(&path);\n-\treturn errno ? -1 : 0;\n+\tret = 0;\n err:\n+\tfclose(fp);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&path);\n-\treturn -1;\n+\treturn ret;\n }\n \n int repo_read_loose_object_map(struct repository *repo)\n-- \ngitgitgadget\n\n"},{"id":"546832","messageId":"a1cd229e33c0ecf8ccbef9ab07b4b93896eae22e.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 02/13] loose: avoid closing invalid fd on error path","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:20Z","receivedAt":"2026-07-01T07:04:36Z","isPatch":true,"body":"From: Johannes Schindelin \n\nwrite_one_object() opens a file at line 186 and jumps to the\nerrout label on failure. The errout cleanup unconditionally calls\nclose(fd), but when open() itself failed, fd is -1. Calling\nclose(-1) is harmless on most platforms (returns EBADF) but is\nundefined behavior per POSIX and can confuse fd tracking in\nsanitizer builds.\n\nGuard the close with fd >= 0.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n loose.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/loose.c b/loose.c\nindex 47b7f5ec38..2c6db45245 100644\n--- a/loose.c\n+++ b/loose.c\n@@ -202,7 +202,8 @@ static int write_one_object(struct odb_source_loose *loose,\n \treturn 0;\n errout:\n \terror_errno(_(\"failed to write loose object index %s\"), path.buf);\n-\tclose(fd);\n+\tif (fd >= 0)\n+\t\tclose(fd);\n \trollback_lock_file(&lock);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&path);\n-- \ngitgitgadget\n\n"},{"id":"546833","messageId":"a770d9708d806ed6a7334a0db053ad94c51a892e.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 03/13] download_https_uri_to_file(): do not leak fd upon failure","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:21Z","receivedAt":"2026-07-01T07:04:39Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen the `git-remote-https` command fails, we do not want to leak\n`child_out`.\n\nPointed out by Coverity.\n\nSigned-off-by: Johannes Schindelin \n---\n bundle-uri.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex 3b2e347288..34fa452e76 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -378,7 +378,7 @@ cleanup:\n \tif (child_in)\n \t\tfclose(child_in);\n \tif (finish_command(&cp))\n-\t\treturn 1;\n+\t\tresult = 1;\n \tif (child_out)\n \t\tfclose(child_out);\n \treturn result;\n-- \ngitgitgadget\n\n"},{"id":"546834","messageId":"d7bcdda31276b5a17f11c307deb0f99ae1dc2861.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 04/13] run-command: avoid close(-1) in start_command() error paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:22Z","receivedAt":"2026-07-01T07:04:40Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen start_command() fails to set up a pipe partway through, it\nrolls back by closing the pipe ends it has already opened. For\ndescriptors supplied by the caller rather than allocated locally,\nthat rollback tested `if (cmd->in)` / `if (cmd->out)` before calling\nclose(). The CHILD_PROCESS_INIT default of -1 (\"no descriptor\") is\nnon-zero and so passes the test, meaning a caller that sets\ncmd->no_stdin or cmd->no_stdout without supplying a real fd ends up\ntriggering close(-1) on the error path.\n\nThe stdin-pipe failure branch a few lines above already uses the\nright idiom, `if (cmd->out > 0)`, which rejects both the -1 sentinel\nand 0 (the parent's own standard streams). Apply it to the three\nremaining rollback sites.\n\nReported by Coverity as CID 1049722 (\"Argument cannot be negative\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n run-command.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex e70a8a387b..ce84db8782 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -706,7 +706,7 @@ int start_command(struct child_process *cmd)\n \t\t\tfailed_errno = errno;\n \t\t\tif (need_in)\n \t\t\t\tclose_pair(fdin);\n-\t\t\telse if (cmd->in)\n+\t\t\telse if (cmd->in > 0)\n \t\t\t\tclose(cmd->in);\n \t\t\tstr = \"standard output\";\n \t\t\tgoto fail_pipe;\n@@ -720,11 +720,11 @@ int start_command(struct child_process *cmd)\n \t\t\tfailed_errno = errno;\n \t\t\tif (need_in)\n \t\t\t\tclose_pair(fdin);\n-\t\t\telse if (cmd->in)\n+\t\t\telse if (cmd->in > 0)\n \t\t\t\tclose(cmd->in);\n \t\t\tif (need_out)\n \t\t\t\tclose_pair(fdout);\n-\t\t\telse if (cmd->out)\n+\t\t\telse if (cmd->out > 0)\n \t\t\t\tclose(cmd->out);\n \t\t\tstr = \"standard error\";\n fail_pipe:\n-- \ngitgitgadget\n\n"},{"id":"546835","messageId":"860bc8f52dc9be8bbfafcda296be831a1ffaf1c2.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 05/13] run_diff_files: avoid memory leak","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:23Z","receivedAt":"2026-07-01T07:04:42Z","isPatch":true,"body":"From: Johannes Schindelin \n\nIn 4fc970c4388 (diff --cc: fix display of symlink conflicts during a\nmerge., 2007-02-25) a conditional block was introduced in\n`run_diff_files()` that skips the rest of the loop iteration and\nadvances directly to the next iteration.\n\nHowever, it missed that there was a similar conditional block that was\nlast touched in b4b1550315c (Don't instantiate structures with FAMs.,\n2006-06-18) and which demonstrated that the `dpath` structure needed to\nbe released.\n\nLet's fix this.\n\nPointed out by Coverity.\n\nSigned-off-by: Johannes Schindelin \n---\n diff-lib.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex ae91027a02..7ba839b4a8 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -152,7 +152,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\tcontinue;\n \n \t\tif (ce_stage(ce)) {\n-\t\t\tstruct combine_diff_path *dpath;\n+\t\t\tstruct combine_diff_path *dpath = NULL;\n \t\t\tstruct diff_filepair *pair;\n \t\t\tunsigned int wt_mode = 0;\n \t\t\tint num_compare_stages = 0;\n@@ -164,6 +164,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\telse {\n \t\t\t\tif (changed < 0) {\n \t\t\t\t\tperror(ce->name);\n+\t\t\t\t\tfree(dpath);\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \t\t\t\twt_mode = 0;\n-- \ngitgitgadget\n\n"},{"id":"546836","messageId":"5a6b17f075ca2d0442d512a0021557aa112860fc.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 06/13] line-log: avoid redundant copy that leaks in process_ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:24Z","receivedAt":"2026-07-01T07:04:43Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen bloom_filter_check() indicates that a commit does not touch\nany of the tracked paths, line_log_process_ranges_arbitrary_commit()\npropagates the current ranges to the parent by calling\nline_log_data_copy() and passing the copy to add_line_range().\nHowever, add_line_range() always makes its own copy internally\n(via line_log_data_copy or line_log_data_merge), so the caller's\ncopy is never freed and leaks every time this path is taken.\n\nPass range directly to add_line_range() instead of making a\nredundant intermediate copy. The callee's internal copy handles\nownership correctly.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n line-log.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex 5fc75ae275..0179f138f7 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1141,8 +1141,7 @@ int line_log_process_ranges_arbitrary_commit(struct rev_info *rev, struct commit\n \n \tif (range) {\n \t\tif (commit->parents && !bloom_filter_check(rev, commit, range)) {\n-\t\t\tstruct line_log_data *prange = line_log_data_copy(range);\n-\t\t\tadd_line_range(rev, commit->parents->item, prange);\n+\t\t\tadd_line_range(rev, commit->parents->item, range);\n \t\t\tclear_commit_line_range(rev, commit);\n \t\t} else if (commit->parents && commit->parents->next)\n \t\t\tchanged = process_ranges_merge_commit(rev, commit, range);\n-- \ngitgitgadget\n\n"},{"id":"546837","messageId":"62ce03454aa1928edd8fa538e0600155629939cd.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 07/13] dir: free allocations on parse-error paths in read_one_dir()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:25Z","receivedAt":"2026-07-01T07:04:44Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen read_one_dir() encounters a parse error while reading the\nuntracked cache from disk, it returns -1 immediately. Two\nallocations made earlier in the function can leak on these\nearly-return paths: ud.untracked (allocated at line 3846 when\nuntracked_nr > 0) and ud.dirs (allocated at line 3851).\n\nFree both before returning on the two error paths between these\nallocations and the point where they are transferred into the\nfinal xmalloc'd struct at line 3857.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n dir.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 32430090dc..23335b9f7a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -3792,13 +3792,18 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,\n \t\tALLOC_ARRAY(ud.untracked, ud.untracked_nr);\n \n \tud.dirs_alloc = ud.dirs_nr = decode_varint(&data);\n-\tif (data > end)\n+\tif (data > end) {\n+\t\tfree(ud.untracked);\n \t\treturn -1;\n+\t}\n \tALLOC_ARRAY(ud.dirs, ud.dirs_nr);\n \n \teos = memchr(data, '\\0', end - data);\n-\tif (!eos || eos == end)\n+\tif (!eos || eos == end) {\n+\t\tfree(ud.untracked);\n+\t\tfree(ud.dirs);\n \t\treturn -1;\n+\t}\n \n \t*untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), eos - data, 1));\n \tmemcpy(untracked, &ud, sizeof(ud));\n-- \ngitgitgadget\n\n"},{"id":"546838","messageId":"6a43f952417259c23ca456c547b0e4587a0ce6fa.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 08/13] submodule: fix cwd leak in get_superproject_working_tree()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:26Z","receivedAt":"2026-07-01T07:04:45Z","isPatch":true,"body":"From: Johannes Schindelin \n\nget_superproject_working_tree() allocates cwd via xgetcwd() at\nthe top of the function, but two early-return paths (when not\ninside a work tree, and when strbuf_realpath for \"../\" fails)\nreturn 0 without freeing it.\n\nRedirect these early returns through a cleanup label that frees\ncwd before returning.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n submodule.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex fd91201a92..8ddeebd8af 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2627,10 +2627,10 @@ int get_superproject_working_tree(struct strbuf *buf)\n \t\t * We might have a superproject, but it is harder\n \t\t * to determine.\n \t\t */\n-\t\treturn 0;\n+\t\tgoto out;\n \n \tif (!strbuf_realpath(&one_up, \"../\", 0))\n-\t\treturn 0;\n+\t\tgoto out;\n \n \tsubpath = relative_path(cwd, one_up.buf, &sb);\n \tstrbuf_release(&one_up);\n@@ -2693,6 +2693,10 @@ int get_superproject_working_tree(struct strbuf *buf)\n \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n \n \treturn ret;\n+\n+out:\n+\tfree(cwd);\n+\treturn 0;\n }\n \n /*\n-- \ngitgitgadget\n\n"},{"id":"546839","messageId":"e39e2f5aa4c7e380d51c95ca276afab393e71b5e.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 09/13] worktree: fix resource leaks when branch creation fails","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:27Z","receivedAt":"2026-07-01T07:04:47Z","isPatch":true,"body":"From: Johannes Schindelin \n\nIn the \"add\" subcommand, when run_command() fails while creating\na new branch (line 948), the function returns -1 immediately\nwithout freeing the allocations made earlier: path (from\nprefix_filename at line 858), opt_track, branch_to_free, and\nnew_branch_to_free.\n\nRedirect the error return through the existing cleanup block at\nthe end of the function so all four allocations are properly\nfreed.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/worktree.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d21c43fde3..4bc7b4f6e7 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -945,14 +945,17 @@ static int add(int ac, const char **av, const char *prefix,\n \t\tstrvec_push(&cp.args, branch);\n \t\tif (opt_track)\n \t\t\tstrvec_push(&cp.args, opt_track);\n-\t\tif (run_command(&cp))\n-\t\t\treturn -1;\n+\t\tif (run_command(&cp)) {\n+\t\t\tret = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbranch = new_branch;\n \t} else if (opt_track) {\n \t\tdie(_(\"--[no-]track can only be used if a new branch is created\"));\n \t}\n \n \tret = add_worktree(path, branch, &opts);\n+cleanup:\n \tfree(path);\n \tfree(opt_track);\n \tfree(branch_to_free);\n-- \ngitgitgadget\n\n"},{"id":"546841","messageId":"cc19a300f5c4735fe91718d4a3ddf414f11eed23.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 10/13] imap-send: avoid leaking the IMAP upload buffer","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:28Z","receivedAt":"2026-07-01T07:04:49Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen uploading messages via libcurl, curl_append_msgs_to_imap()\naccumulates each one in a strbuf that grows across loop iterations\nbut is never released before the function returns.\n\nRelease it alongside the existing libcurl cleanup.\n\nReported by Coverity as CID 1671507 (\"Resource leak\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n imap-send.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex cfd6a5120c..0d16d02029 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1750,6 +1750,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,\n \n \tcurl_easy_cleanup(curl);\n \tcurl_global_cleanup();\n+\tstrbuf_release(&msgbuf.buf);\n \n \tif (cred.username) {\n \t\tif (res == CURLE_OK)\n-- \ngitgitgadget\n\n"},{"id":"546842","messageId":"198062addd9ccd3e7bb32fa81970ac00aa48c46e.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 11/13] reftable/table: release filter on error path","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:29Z","receivedAt":"2026-07-01T07:04:50Z","isPatch":true,"body":"From: Johannes Schindelin \n\nreftable_table_refs_for_unindexed() allocates a filtering_ref_iterator\nand then calls reftable_buf_add() to populate its oid buffer. On\nsuccess ownership is transferred to the output iterator, but if\nreftable_buf_add() fails, the goto-out cleanup only frees the table\niterator and walks away from both the filter allocation and the\noid buffer that reftable_buf_add() may have grown.\n\nRelease filter->oid and free filter alongside the existing table\niterator cleanup.\n\nReported by Coverity as CID 1671512 (\"Resource leak\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n reftable/table.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/reftable/table.c b/reftable/table.c\nindex 56362df0ed..d604ddebf4 100644\n--- a/reftable/table.c\n+++ b/reftable/table.c\n@@ -709,6 +709,10 @@ out:\n \t\tif (ti)\n \t\t\ttable_iter_close(ti);\n \t\treftable_free(ti);\n+\t\tif (filter) {\n+\t\t\treftable_buf_release(&filter->oid);\n+\t\t\treftable_free(filter);\n+\t\t}\n \t}\n \treturn err;\n }\n-- \ngitgitgadget\n\n"},{"id":"546843","messageId":"8ad6b220e9ef58cf90c3bf40b80fc96b6d8bf55d.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 12/13] fsmonitor: plug token-data leak on early daemon-startup failures","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:30Z","receivedAt":"2026-07-01T07:04:52Z","isPatch":true,"body":"From: Johannes Schindelin \n\n`fsmonitor_run_daemon()` allocates `state.current_token_data`\nbefore any subordinate setup step that may fail (alias resolution,\nlistener/health constructors, asynchronous IPC server init). On\nthe successful path the listener thread takes ownership and clears\nthe field during its teardown, so the `done:` cleanup block sees a\nNULL pointer. On every early-error path, however, control jumps\nstraight to `done:` with the freshly allocated token data still\nreferenced, and it is never freed, as Coverity flagged.\n\nFree it at the top of `done:` and clear the pointer. The success\npath is a no-op (the pointer is already NULL there); the error\npaths now drop the otherwise-leaked allocation.\n`fsmonitor_free_token_data()` is NULL-safe and asserts\n`client_ref_count == 0`, which holds trivially here because the\nIPC server has not yet begun accepting clients when these failures\noccur.\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n builtin/fsmonitor--daemon.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c\nindex f920cf3a82..4161dd8282 100644\n--- a/builtin/fsmonitor--daemon.c\n+++ b/builtin/fsmonitor--daemon.c\n@@ -1418,6 +1418,8 @@ static int fsmonitor_run_daemon(void)\n \terr = fsmonitor_run_daemon_1(&state);\n \n done:\n+\tfsmonitor_free_token_data(state.current_token_data);\n+\tstate.current_token_data = NULL;\n \tpthread_cond_destroy(&state.cookies_cond);\n \tpthread_mutex_destroy(&state.main_lock);\n \t{\n-- \ngitgitgadget\n\n"},{"id":"546844","messageId":"23ab9864b2a2b9894379d33350a122c71d411e3a.1782889472.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH 13/13] mingw: make exit_process() own the process handle on all paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-01T07:04:31Z","receivedAt":"2026-07-01T07:04:54Z","isPatch":true,"body":"From: Johannes Schindelin \n\nAfter \"mingw: kill child processes in a gentler way\", the ownership of\nthe HANDLE passed to exit_process() and terminate_process_tree() is\ninconsistent. terminate_process_tree() always closes the handle;\nexit_process() closes it on success and on the terminate-tree\nfallback, but leaks it on the early return where GetExitCodeProcess()\nfails or reports the process is no longer STILL_ACTIVE.\n\nmingw_kill() compensated by closing the handle on its own error path,\nwhich is a double-close on every error path that does not hit that\none leaky branch -- the callee has already closed the handle by then.\nCoverity flagged the resulting use-after-free as CID 1437238.\n\nPin down the invariant that exit_process() and\nterminate_process_tree() own the handle from the call onward and\nclose it on every return path; with that, the bogus close in\nmingw_kill() goes away.\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 4 +---\n compat/win32/exit-process.h | 1 +\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 41e055f7de..e2cb92a414 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2269,10 +2269,8 @@ int mingw_kill(pid_t pid, int sig)\n \t\t\t}\n \t\t\tret = terminate_process_tree(h, 128 + sig);\n \t\t}\n-\t\tif (ret) {\n+\t\tif (ret)\n \t\t\terrno = err_win_to_posix(GetLastError());\n-\t\t\tCloseHandle(h);\n-\t\t}\n \t\treturn ret;\n \t} else if (pid > 0 && sig == 0) {\n \t\tHANDLE h = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid);\ndiff --git a/compat/win32/exit-process.h b/compat/win32/exit-process.h\nindex d53989884c..26004161bc 100644\n--- a/compat/win32/exit-process.h\n+++ b/compat/win32/exit-process.h\n@@ -159,6 +159,7 @@ static int exit_process(HANDLE process, int exit_code)\n \t\treturn terminate_process_tree(process, exit_code);\n \t}\n \n+\tCloseHandle(process);\n \treturn 0;\n }\n \n-- \ngitgitgadget\n"},{"id":"546847","messageId":"akTIIaxwqqFaVxj1@pks.im","threadId":"65900","inReplyTo":"a1cd229e33c0ecf8ccbef9ab07b4b93896eae22e.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 02/13] loose: avoid closing invalid fd on error path","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-01T07:56:17Z","receivedAt":"2026-07-01T07:56:28Z","isPatch":true,"body":"On Wed, Jul 01, 2026 at 07:04:20AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/loose.c b/loose.c\n> index 47b7f5ec38..2c6db45245 100644\n> --- a/loose.c\n> +++ b/loose.c\n> @@ -202,7 +202,8 @@ static int write_one_object(struct odb_source_loose *loose,\n> \treturn 0;\n> errout:\n> \terror_errno(_(\"failed to write loose object index %s\"), path.buf);\n> -\tclose(fd);\n> +\tif (fd >= 0)\n> +\t\tclose(fd);\n> \trollback_lock_file(&lock);\n> \tstrbuf_release(&buf);\n> \tstrbuf_release(&path);\n\nMakes sense. At the time we hit the first `goto errout` we have already\nassigned `fd = open(...)`, so we know it should be either negative or a\npositive file descriptor.\n\nThere's also a second call to `close(fd)`, but if that call is\nsuccessful then we would not use the `errout` path. If it fails we may\ntry to close the file descriptor a second time, but that's probably a\nnon-issue.\n\nPatrick\n"},{"id":"546848","messageId":"akTIK6j9UY4U4x80@pks.im","threadId":"65900","inReplyTo":"d7bcdda31276b5a17f11c307deb0f99ae1dc2861.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 04/13] run-command: avoid close(-1) in start_command() error paths","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-01T07:56:27Z","receivedAt":"2026-07-01T07:56:32Z","isPatch":true,"body":"On Wed, Jul 01, 2026 at 07:04:22AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/run-command.c b/run-command.c\n> index e70a8a387b..ce84db8782 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -706,7 +706,7 @@ int start_command(struct child_process *cmd)\n> \t\t\tfailed_errno = errno;\n> \t\t\tif (need_in)\n> \t\t\t\tclose_pair(fdin);\n> -\t\t\telse if (cmd->in)\n> +\t\t\telse if (cmd->in > 0)\n> \t\t\t\tclose(cmd->in);\n> \t\t\tstr = \"standard output\";\n> \t\t\tgoto fail_pipe;\n> @@ -720,11 +720,11 @@ int start_command(struct child_process *cmd)\n> \t\t\tfailed_errno = errno;\n> \t\t\tif (need_in)\n> \t\t\t\tclose_pair(fdin);\n> -\t\t\telse if (cmd->in)\n> +\t\t\telse if (cmd->in > 0)\n> \t\t\t\tclose(cmd->in);\n> \t\t\tif (need_out)\n> \t\t\t\tclose_pair(fdout);\n> -\t\t\telse if (cmd->out)\n> +\t\t\telse if (cmd->out > 0)\n> \t\t\t\tclose(cmd->out);\n> \t\t\tstr = \"standard error\";\n> fail_pipe:\n\nRight. There's a fourth site that does `close(cmd->out)`, but that site\nalready guards with `if (cmd->out > 0)`.\n\nPatrick\n"},{"id":"546849","messageId":"akTIMM6qLfDNdg-a@pks.im","threadId":"65900","inReplyTo":"860bc8f52dc9be8bbfafcda296be831a1ffaf1c2.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 05/13] run_diff_files: avoid memory leak","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-01T07:56:32Z","receivedAt":"2026-07-01T07:56:36Z","isPatch":true,"body":"On Wed, Jul 01, 2026 at 07:04:23AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/diff-lib.c b/diff-lib.c\n> index ae91027a02..7ba839b4a8 100644\n> --- a/diff-lib.c\n> +++ b/diff-lib.c\n> @@ -152,7 +152,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n> \t\t\tcontinue;\n> \n> \t\tif (ce_stage(ce)) {\n> -\t\t\tstruct combine_diff_path *dpath;\n> +\t\t\tstruct combine_diff_path *dpath = NULL;\n> \t\t\tstruct diff_filepair *pair;\n> \t\t\tunsigned int wt_mode = 0;\n> \t\t\tint num_compare_stages = 0;\n> @@ -164,6 +164,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n> \t\t\telse {\n> \t\t\t\tif (changed < 0) {\n> \t\t\t\t\tperror(ce->name);\n> +\t\t\t\t\tfree(dpath);\n> \t\t\t\t\tcontinue;\n> \t\t\t\t}\n> \t\t\t\twt_mode = 0;\n\nHuh. There is no assignment between the variable declaration and this\ncall to `continue`, so how could this ever plug a memory leak? None of\nthe other paths seem to leak the variable, either.\n\nPatrick\n"},{"id":"546850","messageId":"akTINO_S_NgWbGxG@pks.im","threadId":"65900","inReplyTo":"6a43f952417259c23ca456c547b0e4587a0ce6fa.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 08/13] submodule: fix cwd leak in get_superproject_working_tree()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-01T07:56:36Z","receivedAt":"2026-07-01T07:56:41Z","isPatch":true,"body":"On Wed, Jul 01, 2026 at 07:04:26AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/submodule.c b/submodule.c\n> index fd91201a92..8ddeebd8af 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -2627,10 +2627,10 @@ int get_superproject_working_tree(struct strbuf *buf)\n> \t\t * We might have a superproject, but it is harder\n> \t\t * to determine.\n> \t\t */\n> -\t\treturn 0;\n> +\t\tgoto out;\n> \n> \tif (!strbuf_realpath(&one_up, \"../\", 0))\n> -\t\treturn 0;\n> +\t\tgoto out;\n> \n> \tsubpath = relative_path(cwd, one_up.buf, &sb);\n> \tstrbuf_release(&one_up);\n> @@ -2693,6 +2693,10 @@ int get_superproject_working_tree(struct strbuf *buf)\n> \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n> \n> \treturn ret;\n> +\n> +out:\n> +\tfree(cwd);\n> +\treturn 0;\n> }\n\nOkay. This is fine, but it feels a bit fragile as we also have a call to\n`free(cwd)` a bit further up. So if somebody were to add a `goto out`\nafter that call we'd have a double free. Makes me wonder whether we want\nto have a single exit path for the complete function and then drop the\nother call to free(3p).\n\nPatrick\n"},{"id":"546851","messageId":"akTIOeXFhNjJ7V3i@pks.im","threadId":"65900","inReplyTo":"62ce03454aa1928edd8fa538e0600155629939cd.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 07/13] dir: free allocations on parse-error paths in read_one_dir()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-01T07:56:41Z","receivedAt":"2026-07-01T07:56:46Z","isPatch":true,"body":"On Wed, Jul 01, 2026 at 07:04:25AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/dir.c b/dir.c\n> index 32430090dc..23335b9f7a 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -3792,13 +3792,18 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,\n> \t\tALLOC_ARRAY(ud.untracked, ud.untracked_nr);\n> \n> \tud.dirs_alloc = ud.dirs_nr = decode_varint(&data);\n> -\tif (data > end)\n> +\tif (data > end) {\n> +\t\tfree(ud.untracked);\n> \t\treturn -1;\n> +\t}\n> \tALLOC_ARRAY(ud.dirs, ud.dirs_nr);\n> \n> \teos = memchr(data, '\\0', end - data);\n> -\tif (!eos || eos == end)\n> +\tif (!eos || eos == end) {\n> +\t\tfree(ud.untracked);\n> +\t\tfree(ud.dirs);\n> \t\treturn -1;\n> +\t}\n> \n> \t*untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), eos - data, 1));\n> \tmemcpy(untracked, &ud, sizeof(ud));\n\nHm. Here we assign ownership to the caller, but this still feels quite\noff to me as we also have two more early returns after this point that\nseem to leak memory. Do the callers make sure to always free the data?\n\nPatrick\n"},{"id":"546852","messageId":"akTIAGKOS2uUcuZG@pks.im","threadId":"65900","inReplyTo":"17242c249f0beb387fd30634663f13ce42d34f79.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/13] load_one_loose_object_map(): fix resource leak","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-01T07:56:46Z","receivedAt":"2026-07-01T07:56:51Z","isPatch":true,"body":"On Wed, Jul 01, 2026 at 07:04:19AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/loose.c b/loose.c\n> index 0b626c1b85..47b7f5ec38 100644\n> --- a/loose.c\n> +++ b/loose.c\n> @@ -65,6 +65,7 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n> {\n> \tstruct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;\n> \tFILE *fp;\n> +\tint ret = -1;\n> \n> \tif (!loose->map)\n> \t\tloose_object_map_init(&loose->map);\n> @@ -98,13 +99,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n> \t\tinsert_loose_map(loose, &oid, &compat_oid);\n> \t}\n> \n> -\tstrbuf_release(&buf);\n> -\tstrbuf_release(&path);\n> -\treturn errno ? -1 : 0;\n> +\tret = 0;\n> err:\n> +\tfclose(fp);\n> \tstrbuf_release(&buf);\n> \tstrbuf_release(&path);\n> -\treturn -1;\n> +\treturn ret;\n> }\n\nMakes sense. There's no `goto err` before we assign `fp`, and when the\ncall to `fopen()` fails we return via a different path. So the added\ncall to `fclose(fp)` is fine.\n\nPatrick\n"},{"id":"546854","messageId":"20260701080224.GA813310@coredump.intra.peff.net","threadId":"65900","inReplyTo":"5a6b17f075ca2d0442d512a0021557aa112860fc.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 06/13] line-log: avoid redundant copy that leaks in process_ranges","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-01T08:02:24Z","receivedAt":"2026-07-01T08:02:26Z","isPatch":true,"body":"On Wed, Jul 01, 2026 at 07:04:24AM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n> \n> When bloom_filter_check() indicates that a commit does not touch\n> any of the tracked paths, line_log_process_ranges_arbitrary_commit()\n> propagates the current ranges to the parent by calling\n> line_log_data_copy() and passing the copy to add_line_range().\n> However, add_line_range() always makes its own copy internally\n> (via line_log_data_copy or line_log_data_merge), so the caller's\n> copy is never freed and leaks every time this path is taken.\n> \n> Pass range directly to add_line_range() instead of making a\n> redundant intermediate copy. The callee's internal copy handles\n> ownership correctly.\n> \n> Pointed out by Coverity.\n\nHeh, I just posted the identical patch (in my case found by running the\ntest suite with GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1).\n\nSo yeah, looks good to me. :)\n\n-Peff\n"},{"id":"546911","messageId":"xmqqcxx6pths.fsf@gitster.g","threadId":"65900","inReplyTo":"17242c249f0beb387fd30634663f13ce42d34f79.1782889472.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 01/13] load_one_loose_object_map(): fix resource leak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-01T16:25:19Z","receivedAt":"2026-07-01T16:25:21Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> Pointed out by Coverity.\n>\n> While at it, reduce near-duplicate clean-up code at the end of the\n> function.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> loose.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/loose.c b/loose.c\n> index 0b626c1b85..47b7f5ec38 100644\n> --- a/loose.c\n> +++ b/loose.c\n> @@ -65,6 +65,7 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n> {\n> \tstruct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;\n> \tFILE *fp;\n> +\tint ret = -1;\n> \n> \tif (!loose->map)\n> \t\tloose_object_map_init(&loose->map);\n> @@ -98,13 +99,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n> \t\tinsert_loose_map(loose, &oid, &compat_oid);\n> \t}\n> \n> -\tstrbuf_release(&buf);\n> -\tstrbuf_release(&path);\n> -\treturn errno ? -1 : 0;\n\nWow, this is bad bad bad. We do not even know what is in errno as\nwe are supposed to have jumped to out-of-line err label in all error\ncases.\n\n> +\tret = 0;\n\nOr we can do\n\n\tret = ferror(fp) ? -1 : 0;\n\nif we want to be sure that we have caught all the errors.\n\n> err:\n> +\tfclose(fp);\n> \tstrbuf_release(&buf);\n> \tstrbuf_release(&path);\n> -\treturn -1;\n> +\treturn ret;\n> }\n> \n> int repo_read_loose_object_map(struct repository *repo)\n"},{"id":"546928","messageId":"xmqqjyreobpd.fsf@gitster.g","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"Re: [PATCH 00/13] coverity: fix leaks and error paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-01T17:34:54Z","receivedAt":"2026-07-01T17:34:56Z","isPatch":true,"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> I wanted to whittle down the many issues reported by Coverity in the Git for\n> Windows project. Turns out: The vast majority of the issues are false\n> positives. Most of the remaining issues are in core Git proper.\n\nI read through the series and did not see anything jumping at me as\nwrong. Looking good. Will queue.\n\nThanks.\n\n>\n> This effort was forced on pause while Coverity was down from May 16\n> [https://web.archive.org/web/20260516152422/https://scan.coverity.com/] to\n> June 22\n> [https://web.archive.org/web/20260622182153/https://scan.coverity.com/]).\n>\n> Here is a first batch of fixes for those issues.\n>\n> Johannes Schindelin (13):\n> load_one_loose_object_map(): fix resource leak\n> loose: avoid closing invalid fd on error path\n> download_https_uri_to_file(): do not leak fd upon failure\n> run-command: avoid close(-1) in start_command() error paths\n> run_diff_files: avoid memory leak\n> line-log: avoid redundant copy that leaks in process_ranges\n> dir: free allocations on parse-error paths in read_one_dir()\n> submodule: fix cwd leak in get_superproject_working_tree()\n> worktree: fix resource leaks when branch creation fails\n> imap-send: avoid leaking the IMAP upload buffer\n> reftable/table: release filter on error path\n> fsmonitor: plug token-data leak on early daemon-startup failures\n> mingw: make exit_process() own the process handle on all paths\n>\n> builtin/fsmonitor--daemon.c | 2 ++\n> builtin/worktree.c | 7 +++++--\n> bundle-uri.c | 2 +-\n> compat/mingw.c | 4 +---\n> compat/win32/exit-process.h | 1 +\n> diff-lib.c | 3 ++-\n> dir.c | 9 +++++++--\n> imap-send.c | 1 +\n> line-log.c | 3 +--\n> loose.c | 11 ++++++-----\n> reftable/table.c | 4 ++++\n> run-command.c | 6 +++---\n> submodule.c | 8 ++++++--\n> 13 files changed, 40 insertions(+), 21 deletions(-)\n>\n>\n> base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2163%2Fdscho%2Fcoverity-fixes-leaks-and-error-paths-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2163/dscho/coverity-fixes-leaks-and-error-paths-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/2163\n"},{"id":"547132","messageId":"9dd7e482-5a4d-b75d-56a4-2c422cbd8812@gmx.de","threadId":"65900","inReplyTo":"xmqqcxx6pths.fsf@gitster.g","subject":"Re: [PATCH 01/13] load_one_loose_object_map(): fix resource leak","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-07-04T08:58:45Z","receivedAt":"2026-07-04T08:58:45Z","isPatch":true,"body":"Hi Junio,\n\nOn Wed, 1 Jul 2026, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > +\tret = 0;\n> \n> Or we can do\n> \n> \tret = ferror(fp) ? -1 : 0;\n> \n> if we want to be sure that we have caught all the errors.\n\nAgreed; that is what v2 will use.\n\nTo corroborate the diagnosis: `strbuf_getline_lf()` ultimately calls\n`getdelim()`, which returns -1 on both EOF and I/O error, so `ferror(fp)`\non the underlying stream is the only reliable way to distinguish the two.\nThat also makes the `errno = 0;` I had added at the top of the loop dead,\nso it goes away in v2.\n\nCiao,\nJohannes\n"},{"id":"547133","messageId":"ac7844ee-a401-76b7-56aa-6318e2bdcc4a@gmx.de","threadId":"65900","inReplyTo":"akTIMM6qLfDNdg-a@pks.im","subject":"Re: [PATCH 05/13] run_diff_files: avoid memory leak","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-07-04T08:58:51Z","receivedAt":"2026-07-04T08:58:53Z","isPatch":true,"body":"Hi Patrick,\n\nOn Wed, 1 Jul 2026, Patrick Steinhardt wrote:\n\n> On Wed, Jul 01, 2026 at 07:04:23AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > diff --git a/diff-lib.c b/diff-lib.c\n> > index ae91027a02..7ba839b4a8 100644\n> > --- a/diff-lib.c\n> > +++ b/diff-lib.c\n> > @@ -152,7 +152,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n> > \t\t\tcontinue;\n> > \n> > \t\tif (ce_stage(ce)) {\n> > -\t\t\tstruct combine_diff_path *dpath;\n> > +\t\t\tstruct combine_diff_path *dpath = NULL;\n> > \t\t\tstruct diff_filepair *pair;\n> > \t\t\tunsigned int wt_mode = 0;\n> > \t\t\tint num_compare_stages = 0;\n> > @@ -164,6 +164,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)\n> > \t\t\telse {\n> > \t\t\t\tif (changed < 0) {\n> > \t\t\t\t\tperror(ce->name);\n> > +\t\t\t\t\tfree(dpath);\n> > \t\t\t\t\tcontinue;\n> > \t\t\t\t}\n> > \t\t\t\twt_mode = 0;\n> \n> Huh. There is no assignment between the variable declaration and this\n> call to `continue`, so how could this ever plug a memory leak? None of\n> the other paths seem to leak the variable, either.\n\nYou are right; the patch as posted plugs nothing.\n\nThe reason it looks pointless is that the leak it was written against was\nfixed independently in the meantime by 949bb8f74f4a (run_diff_files():\ndelay allocation of combine_diff_path, 2025-01-09), which moved the `dpath\n= xmalloc(...)` to after the `check_removed()` call. Before that\nreordering, the two `continue` statements did leak the just-allocated\n`dpath` (originally introduced by 4fc970c43884, 2007-02-25).\n\nI had missed that when picking back up the work on addressing Coverity\nreports, sorry! I will drop this patch from v2.\n\nCiao,\nJohannes\n"},{"id":"547134","messageId":"0278c01a-5a7d-e6a4-bee3-4df6df7e5276@gmx.de","threadId":"65900","inReplyTo":"akTIOeXFhNjJ7V3i@pks.im","subject":"Re: [PATCH 07/13] dir: free allocations on parse-error paths in read_one_dir()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-07-04T08:58:56Z","receivedAt":"2026-07-04T08:58:56Z","isPatch":true,"body":"Hi Patrick,\n\nOn Wed, 1 Jul 2026, Patrick Steinhardt wrote:\n\n> On Wed, Jul 01, 2026 at 07:04:25AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > diff --git a/dir.c b/dir.c\n> > index 32430090dc..23335b9f7a 100644\n> > --- a/dir.c\n> > +++ b/dir.c\n> > @@ -3792,13 +3792,18 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,\n> > \t\tALLOC_ARRAY(ud.untracked, ud.untracked_nr);\n> > \n> > \tud.dirs_alloc = ud.dirs_nr = decode_varint(&data);\n> > -\tif (data > end)\n> > +\tif (data > end) {\n> > +\t\tfree(ud.untracked);\n> > \t\treturn -1;\n> > +\t}\n> > \tALLOC_ARRAY(ud.dirs, ud.dirs_nr);\n> > \n> > \teos = memchr(data, '\\0', end - data);\n> > -\tif (!eos || eos == end)\n> > +\tif (!eos || eos == end) {\n> > +\t\tfree(ud.untracked);\n> > +\t\tfree(ud.dirs);\n> > \t\treturn -1;\n> > +\t}\n> > \n> > \t*untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), eos - data, 1));\n> > \tmemcpy(untracked, &ud, sizeof(ud));\n> \n> Hm. Here we assign ownership to the caller, but this still feels quite\n> off to me as we also have two more early returns after this point that\n> seem to leak memory. Do the callers make sure to always free the data?\n\nOwnership transfers to the caller on the `xmalloc`/`memcpy` line: the\n`memcpy` copies the `ud.untracked` and `ud.dirs` pointers into the freshly\nxmalloc'd struct that becomes `*untracked_`. From there, any subsequent\nfailure in the caller reaches `free_untracked_cache()` and then\n`free_untracked()`, which releases both arrays. So the two further early\nreturns are correct as-are.\n\nI will fold that reasoning into the v2 commit message so a future\nreader does not have to re-derive it.\n\nIncidentally, and orthogonal to Coverity's leak report: on those same\nfailure paths, individual slots of `->dirs` and `->untracked` remain\nuninitialised, so `free_untracked()` walks garbage pointers before it\never reaches the two `free()` calls above. That is a separate\ncrash-on-cleanup bug and I would prefer to address it in a follow-up\nrather than widen the scope of this series.\n\nCiao,\nJohannes\n"},{"id":"547135","messageId":"7ab4ffc7-aade-45fc-5456-b57eb32a3276@gmx.de","threadId":"65900","inReplyTo":"akTINO_S_NgWbGxG@pks.im","subject":"Re: [PATCH 08/13] submodule: fix cwd leak in get_superproject_working_tree()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-07-04T08:59:05Z","receivedAt":"2026-07-04T08:59:08Z","isPatch":true,"body":"Hi Patrick,\n\nOn Wed, 1 Jul 2026, Patrick Steinhardt wrote:\n\n> On Wed, Jul 01, 2026 at 07:04:26AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > diff --git a/submodule.c b/submodule.c\n> > index fd91201a92..8ddeebd8af 100644\n> > --- a/submodule.c\n> > +++ b/submodule.c\n> > @@ -2627,10 +2627,10 @@ int get_superproject_working_tree(struct strbuf *buf)\n> > \t\t * We might have a superproject, but it is harder\n> > \t\t * to determine.\n> > \t\t */\n> > -\t\treturn 0;\n> > +\t\tgoto out;\n> > \n> > \tif (!strbuf_realpath(&one_up, \"../\", 0))\n> > -\t\treturn 0;\n> > +\t\tgoto out;\n> > \n> > \tsubpath = relative_path(cwd, one_up.buf, &sb);\n> > \tstrbuf_release(&one_up);\n> > @@ -2693,6 +2693,10 @@ int get_superproject_working_tree(struct strbuf *buf)\n> > \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n> > \n> > \treturn ret;\n> > +\n> > +out:\n> > +\tfree(cwd);\n> > +\treturn 0;\n> > }\n> \n> Okay. This is fine, but it feels a bit fragile as we also have a call to\n> `free(cwd)` a bit further up. So if somebody were to add a `goto out`\n> after that call we'd have a double free. Makes me wonder whether we want\n> to have a single exit path for the complete function and then drop the\n> other call to free(3p).\n\nAgreed. In v2 the function has a single exit path: all late returns\nfall through to the `out:` label, which additionally releases `sb`\nand `one_up`.\n\nA side effect worth noting is that consolidation also closes a latent\nleak the original had on the `strbuf_realpath(&one_up, \"../\", 0)`\nfailure path. `strbuf_realpath_1()` calls `strbuf_reset(resolved)` on\nerror, which does not free the backing buffer, so `one_up` could\ncarry a residual allocation that the previous shape never released.\n\nCiao,\nJohannes\n"},{"id":"547149","messageId":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.git.1782889472.gitgitgadget@gmail.com","subject":"[PATCH v2 00/12] coverity: fix leaks and error paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:17Z","receivedAt":"2026-07-05T08:24:33Z","isPatch":true,"body":"I wanted to whittle down the many issues reported by Coverity in the Git for\nWindows project. Turns out: The vast majority of the issues are false\npositives. Most of the remaining issues are in core Git proper.\n\nThis effort was forced on pause while Coverity was down from May 16\n[https://web.archive.org/web/20260516152422/https://scan.coverity.com/] to\nJune 22\n[https://web.archive.org/web/20260622182153/https://scan.coverity.com/]).\n\nHere is a first batch of fixes for those issues.\n\nChanges since v1:\n\n * Edited the commit messages to put function names in backticks, and\n reflowed the messages afterwards.\n * Took Junio's suggestion to avoid (ab-)using errno to determine the return\n value of load_one_loose_object_map().\n * Dropped the obsolete patch \"run_diff_files: avoid memory leak\".\n * Rewrote the commit message of \"dir: free allocations on parse-error paths\n in read_one_dir()\" to clarify ownership of the allocated untracked/dirs\n buffers.\n * Changed \"submodule: fix cwd leak in get_superproject_working_tree()\" to\n reduce the cognitive load on the reader (i.e. to make it a lot easier to\n reason about the correctness of the patch).\n\nJohannes Schindelin (12):\n load_one_loose_object_map(): fix resource leak\n loose: avoid closing invalid fd on error path\n download_https_uri_to_file(): do not leak fd upon failure\n run-command: avoid `close(-1)` in `start_command()` error paths\n line-log: avoid redundant copy that leaks in process_ranges\n dir: free allocations on parse-error paths in `read_one_dir()`\n submodule: fix cwd leak in `get_superproject_working_tree()`\n worktree: fix resource leaks when branch creation fails\n imap-send: avoid leaking the IMAP upload buffer\n reftable/table: release filter on error path\n fsmonitor: plug token-data leak on early daemon-startup failures\n mingw: make `exit_process()` own the process handle on all paths\n\n builtin/fsmonitor--daemon.c | 2 ++\n builtin/worktree.c | 7 +++++--\n bundle-uri.c | 2 +-\n compat/mingw.c | 4 +---\n compat/win32/exit-process.h | 1 +\n dir.c | 9 +++++++--\n imap-send.c | 1 +\n line-log.c | 3 +--\n loose.c | 12 ++++++------\n reftable/table.c | 4 ++++\n run-command.c | 6 +++---\n submodule.c | 19 ++++++++++---------\n 12 files changed, 42 insertions(+), 28 deletions(-)\n\n\nbase-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2163%2Fdscho%2Fcoverity-fixes-leaks-and-error-paths-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2163/dscho/coverity-fixes-leaks-and-error-paths-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2163\n\nRange-diff vs v1:\n\n 1: 17242c249f ! 1: 80ae35227d load_one_loose_object_map(): fix resource leak\n @@ loose.c: static int load_one_loose_object_map(struct repository *repo, struct od\n \n \tif (!loose->map)\n \t\tloose_object_map_init(&loose->map);\n +@@ loose.c: static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n + \t\treturn 0;\n + \t}\n + \n +-\terrno = 0;\n + \tif (strbuf_getwholeline(&buf, fp, '\\n') || strcmp(buf.buf, loose_object_header))\n + \t\tgoto err;\n + \twhile (!strbuf_getline_lf(&buf, fp)) {\n @@ loose.c: static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n \t\tinsert_loose_map(loose, &oid, &compat_oid);\n \t}\n @@ loose.c: static int load_one_loose_object_map(struct repository *repo, struct od\n -\tstrbuf_release(&buf);\n -\tstrbuf_release(&path);\n -\treturn errno ? -1 : 0;\n -+\tret = 0;\n ++\tret = ferror(fp) ? -1 : 0;\n err:\n +\tfclose(fp);\n \tstrbuf_release(&buf);\n 2: a1cd229e33 ! 2: 546a7c5d9f loose: avoid closing invalid fd on error path\n @@ Metadata\n ## Commit message ##\n loose: avoid closing invalid fd on error path\n \n - write_one_object() opens a file at line 186 and jumps to the\n - errout label on failure. The errout cleanup unconditionally calls\n - close(fd), but when open() itself failed, fd is -1. Calling\n - close(-1) is harmless on most platforms (returns EBADF) but is\n - undefined behavior per POSIX and can confuse fd tracking in\n - sanitizer builds.\n + `write_one_object()` opens a file at line 186 and jumps to the errout\n + label on failure. The errout cleanup unconditionally calls `close(fd)`,\n + but when `open()` itself failed, fd is -1. Calling `close(-1)` is\n + harmless on most platforms (returns EBADF) but is undefined behavior per\n + POSIX and can confuse fd tracking in sanitizer builds.\n \n Guard the close with fd >= 0.\n \n 3: a770d9708d = 3: 17c3b4ce4f download_https_uri_to_file(): do not leak fd upon failure\n 4: d7bcdda312 ! 4: 0360016d91 run-command: avoid close(-1) in start_command() error paths\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - run-command: avoid close(-1) in start_command() error paths\n + run-command: avoid `close(-1)` in `start_command()` error paths\n \n - When start_command() fails to set up a pipe partway through, it\n - rolls back by closing the pipe ends it has already opened. For\n - descriptors supplied by the caller rather than allocated locally,\n - that rollback tested `if (cmd->in)` / `if (cmd->out)` before calling\n - close(). The CHILD_PROCESS_INIT default of -1 (\"no descriptor\") is\n - non-zero and so passes the test, meaning a caller that sets\n - cmd->no_stdin or cmd->no_stdout without supplying a real fd ends up\n - triggering close(-1) on the error path.\n + When `start_command()` fails to set up a pipe partway through, it rolls\n + back by closing the pipe ends it has already opened. For descriptors\n + supplied by the caller rather than allocated locally, that rollback\n + tested `if (cmd->in)` / `if (cmd->out)` before calling close(). The\n + CHILD_PROCESS_INIT default of -1 (\"no descriptor\") is non-zero and so\n + passes the test, meaning a caller that sets cmd->no_stdin or\n + cmd->no_stdout without supplying a real fd ends up triggering close(-1)\n + on the error path.\n \n - The stdin-pipe failure branch a few lines above already uses the\n - right idiom, `if (cmd->out > 0)`, which rejects both the -1 sentinel\n - and 0 (the parent's own standard streams). Apply it to the three\n - remaining rollback sites.\n + The stdin-pipe failure branch a few lines above already uses the right\n + idiom, `if (cmd->out > 0)`, which rejects both the -1 sentinel and 0\n + (the parent's own standard streams). Apply it to the three remaining\n + rollback sites.\n \n Reported by Coverity as CID 1049722 (\"Argument cannot be negative\").\n \n 5: 860bc8f52d < -: ---------- run_diff_files: avoid memory leak\n 6: 5a6b17f075 ! 5: 8c623cc28f line-log: avoid redundant copy that leaks in process_ranges\n @@ Metadata\n ## Commit message ##\n line-log: avoid redundant copy that leaks in process_ranges\n \n - When bloom_filter_check() indicates that a commit does not touch\n - any of the tracked paths, line_log_process_ranges_arbitrary_commit()\n + When `bloom_filter_check()` indicates that a commit does not touch any\n + of the tracked paths, `line_log_process_ranges_arbitrary_commit()`\n propagates the current ranges to the parent by calling\n - line_log_data_copy() and passing the copy to add_line_range().\n - However, add_line_range() always makes its own copy internally\n - (via line_log_data_copy or line_log_data_merge), so the caller's\n - copy is never freed and leaks every time this path is taken.\n + `line_log_data_copy()` and passing the copy to add_line_range().\n + However, `add_line_range()` always makes its own copy internally (via\n + line_log_data_copy or line_log_data_merge), so the caller's copy is\n + never freed and leaks every time this path is taken.\n \n - Pass range directly to add_line_range() instead of making a\n - redundant intermediate copy. The callee's internal copy handles\n - ownership correctly.\n + Pass range directly to `add_line_range()` instead of making a redundant\n + intermediate copy. The callee's internal copy handles ownership\n + correctly.\n \n Pointed out by Coverity.\n \n 7: 62ce03454a ! 6: 8a8fe2d3e3 dir: free allocations on parse-error paths in read_one_dir()\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - dir: free allocations on parse-error paths in read_one_dir()\n + dir: free allocations on parse-error paths in `read_one_dir()`\n \n - When read_one_dir() encounters a parse error while reading the\n - untracked cache from disk, it returns -1 immediately. Two\n - allocations made earlier in the function can leak on these\n - early-return paths: ud.untracked (allocated at line 3846 when\n - untracked_nr > 0) and ud.dirs (allocated at line 3851).\n + Two of `read_one_dir()`'s parse-error early returns leak ud.untracked\n + and ud.dirs. Plug them.\n \n - Free both before returning on the two error paths between these\n - allocations and the point where they are transferred into the\n - final xmalloc'd struct at line 3857.\n + The other early returns in the same function are fine: they occur after\n + the `xmalloc()`+`memcpy()` that copies ud into `*untracked_`, at which\n + point ownership is transferred to the caller.\n + `read_untracked_extension()` then releases everything via\n + `free_untracked_cache()` on failure.\n \n Pointed out by Coverity.\n \n 8: 6a43f95241 ! 7: 5397ea785c submodule: fix cwd leak in get_superproject_working_tree()\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - submodule: fix cwd leak in get_superproject_working_tree()\n + submodule: fix cwd leak in `get_superproject_working_tree()`\n \n - get_superproject_working_tree() allocates cwd via xgetcwd() at\n - the top of the function, but two early-return paths (when not\n - inside a work tree, and when strbuf_realpath for \"../\" fails)\n - return 0 without freeing it.\n + `get_superproject_working_tree()` allocates cwd via `xgetcwd()` at the\n + top of the function, but two early-return paths (when not inside a work\n + tree, and when strbuf_realpath for \"../\" fails) return 0 without freeing\n + it.\n \n - Redirect these early returns through a cleanup label that frees\n - cwd before returning.\n + Redirect these early returns through a cleanup label that frees cwd\n + before returning.\n \n Pointed out by Coverity.\n \n @@ submodule.c: int get_superproject_working_tree(struct strbuf *buf)\n +\t\tgoto out;\n \n \tsubpath = relative_path(cwd, one_up.buf, &sb);\n - \tstrbuf_release(&one_up);\n +-\tstrbuf_release(&one_up);\n + \n + \tprepare_submodule_repo_env(&cp.env);\n + \tstrvec_pop(&cp.env);\n @@ submodule.c: int get_superproject_working_tree(struct strbuf *buf)\n + \t\tret = 1;\n + \t\tfree(super_wt);\n + \t}\n +-\tfree(cwd);\n +-\tstrbuf_release(&sb);\n + \n + \tcode = finish_command(&cp);\n + \n + \tif (code == 128)\n + \t\t/* '../' is not a git repository */\n +-\t\treturn 0;\n +-\tif (code == 0 && len == 0)\n ++\t\tret = 0;\n ++\telse if (code == 0 && len == 0)\n + \t\t/* There is an unrelated git repository at '../' */\n +-\t\treturn 0;\n +-\tif (code)\n ++\t\tret = 0;\n ++\telse if (code)\n \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n \n - \treturn ret;\n -+\n +out:\n ++\tstrbuf_release(&sb);\n ++\tstrbuf_release(&one_up);\n +\tfree(cwd);\n -+\treturn 0;\n + \treturn ret;\n }\n \n - /*\n 9: e39e2f5aa4 ! 8: 0048c0ca27 worktree: fix resource leaks when branch creation fails\n @@ Metadata\n ## Commit message ##\n worktree: fix resource leaks when branch creation fails\n \n - In the \"add\" subcommand, when run_command() fails while creating\n - a new branch (line 948), the function returns -1 immediately\n - without freeing the allocations made earlier: path (from\n - prefix_filename at line 858), opt_track, branch_to_free, and\n - new_branch_to_free.\n + In the \"add\" subcommand, when `run_command()` fails while creating a new\n + branch (line 948), the function returns -1 immediately without freeing\n + the allocations made earlier: path (from prefix_filename at line 858),\n + opt_track, branch_to_free, and new_branch_to_free.\n \n - Redirect the error return through the existing cleanup block at\n - the end of the function so all four allocations are properly\n - freed.\n + Redirect the error return through the existing cleanup block at the end\n + of the function so all four allocations are properly freed.\n \n Pointed out by Coverity.\n \n 10: cc19a300f5 ! 9: 4048a225a5 imap-send: avoid leaking the IMAP upload buffer\n @@ Metadata\n ## Commit message ##\n imap-send: avoid leaking the IMAP upload buffer\n \n - When uploading messages via libcurl, curl_append_msgs_to_imap()\n - accumulates each one in a strbuf that grows across loop iterations\n - but is never released before the function returns.\n + When uploading messages via libcurl, `curl_append_msgs_to_imap()`\n + accumulates each one in a strbuf that grows across loop iterations but\n + is never released before the function returns.\n \n Release it alongside the existing libcurl cleanup.\n \n 11: 198062addd ! 10: 13ecebcdee reftable/table: release filter on error path\n @@ Metadata\n ## Commit message ##\n reftable/table: release filter on error path\n \n - reftable_table_refs_for_unindexed() allocates a filtering_ref_iterator\n - and then calls reftable_buf_add() to populate its oid buffer. On\n + `reftable_table_refs_for_unindexed()` allocates a filtering_ref_iterator\n + and then calls `reftable_buf_add()` to populate its oid buffer. On\n success ownership is transferred to the output iterator, but if\n - reftable_buf_add() fails, the goto-out cleanup only frees the table\n - iterator and walks away from both the filter allocation and the\n - oid buffer that reftable_buf_add() may have grown.\n + `reftable_buf_add()` fails, the goto-out cleanup only frees the table\n + iterator and walks away from both the filter allocation and the oid\n + buffer that `reftable_buf_add()` may have grown.\n \n Release filter->oid and free filter alongside the existing table\n iterator cleanup.\n 12: 8ad6b220e9 = 11: 97049d7cc3 fsmonitor: plug token-data leak on early daemon-startup failures\n 13: 23ab9864b2 ! 12: a5a6c27184 mingw: make exit_process() own the process handle on all paths\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - mingw: make exit_process() own the process handle on all paths\n + mingw: make `exit_process()` own the process handle on all paths\n \n After \"mingw: kill child processes in a gentler way\", the ownership of\n - the HANDLE passed to exit_process() and terminate_process_tree() is\n - inconsistent. terminate_process_tree() always closes the handle;\n - exit_process() closes it on success and on the terminate-tree\n + the HANDLE passed to `exit_process()` and `terminate_process_tree()` is\n + inconsistent. `terminate_process_tree()` always closes the handle;\n + `exit_process()` closes it on success and on the terminate-tree\n fallback, but leaks it on the early return where GetExitCodeProcess()\n fails or reports the process is no longer STILL_ACTIVE.\n \n - mingw_kill() compensated by closing the handle on its own error path,\n - which is a double-close on every error path that does not hit that\n - one leaky branch -- the callee has already closed the handle by then.\n + `mingw_kill()` compensated by closing the handle on its own error path,\n + which is a double-close on every error path that does not hit that one\n + leaky branch -- the callee has already closed the handle by then.\n Coverity flagged the resulting use-after-free as CID 1437238.\n \n - Pin down the invariant that exit_process() and\n - terminate_process_tree() own the handle from the call onward and\n - close it on every return path; with that, the bogus close in\n - mingw_kill() goes away.\n + Pin down the invariant that `exit_process()` and\n + `terminate_process_tree()` own the handle from the call onward and close\n + it on every return path; with that, the bogus close in `mingw_kill()`\n + goes away.\n \n Assisted-by: Opus 4.7\n Signed-off-by: Johannes Schindelin \n\n-- \ngitgitgadget\n"},{"id":"547150","messageId":"80ae35227d566977ad21eb6e35f49e1ca5d5a940.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 01/12] load_one_loose_object_map(): fix resource leak","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:18Z","receivedAt":"2026-07-05T08:24:35Z","isPatch":true,"body":"From: Johannes Schindelin \n\nPointed out by Coverity.\n\nWhile at it, reduce near-duplicate clean-up code at the end of the\nfunction.\n\nSigned-off-by: Johannes Schindelin \n---\n loose.c | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/loose.c b/loose.c\nindex 0b626c1b85..940a9e0dfe 100644\n--- a/loose.c\n+++ b/loose.c\n@@ -65,6 +65,7 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n {\n \tstruct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;\n \tFILE *fp;\n+\tint ret = -1;\n \n \tif (!loose->map)\n \t\tloose_object_map_init(&loose->map);\n@@ -84,7 +85,6 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n \t\treturn 0;\n \t}\n \n-\terrno = 0;\n \tif (strbuf_getwholeline(&buf, fp, '\\n') || strcmp(buf.buf, loose_object_header))\n \t\tgoto err;\n \twhile (!strbuf_getline_lf(&buf, fp)) {\n@@ -98,13 +98,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n \t\tinsert_loose_map(loose, &oid, &compat_oid);\n \t}\n \n-\tstrbuf_release(&buf);\n-\tstrbuf_release(&path);\n-\treturn errno ? -1 : 0;\n+\tret = ferror(fp) ? -1 : 0;\n err:\n+\tfclose(fp);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&path);\n-\treturn -1;\n+\treturn ret;\n }\n \n int repo_read_loose_object_map(struct repository *repo)\n-- \ngitgitgadget\n\n"},{"id":"547151","messageId":"546a7c5d9f76fc9cb71305bd0bcb4bc7693ecb39.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 02/12] loose: avoid closing invalid fd on error path","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:19Z","receivedAt":"2026-07-05T08:24:36Z","isPatch":true,"body":"From: Johannes Schindelin \n\n`write_one_object()` opens a file at line 186 and jumps to the errout\nlabel on failure. The errout cleanup unconditionally calls `close(fd)`,\nbut when `open()` itself failed, fd is -1. Calling `close(-1)` is\nharmless on most platforms (returns EBADF) but is undefined behavior per\nPOSIX and can confuse fd tracking in sanitizer builds.\n\nGuard the close with fd >= 0.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n loose.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/loose.c b/loose.c\nindex 940a9e0dfe..bf01d3e42d 100644\n--- a/loose.c\n+++ b/loose.c\n@@ -201,7 +201,8 @@ static int write_one_object(struct odb_source_loose *loose,\n \treturn 0;\n errout:\n \terror_errno(_(\"failed to write loose object index %s\"), path.buf);\n-\tclose(fd);\n+\tif (fd >= 0)\n+\t\tclose(fd);\n \trollback_lock_file(&lock);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&path);\n-- \ngitgitgadget\n\n"},{"id":"547152","messageId":"17c3b4ce4f0051bf0c27ae157c25af97275ba742.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 03/12] download_https_uri_to_file(): do not leak fd upon failure","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:20Z","receivedAt":"2026-07-05T08:24:37Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen the `git-remote-https` command fails, we do not want to leak\n`child_out`.\n\nPointed out by Coverity.\n\nSigned-off-by: Johannes Schindelin \n---\n bundle-uri.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex 3b2e347288..34fa452e76 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -378,7 +378,7 @@ cleanup:\n \tif (child_in)\n \t\tfclose(child_in);\n \tif (finish_command(&cp))\n-\t\treturn 1;\n+\t\tresult = 1;\n \tif (child_out)\n \t\tfclose(child_out);\n \treturn result;\n-- \ngitgitgadget\n\n"},{"id":"547153","messageId":"0360016d91bfca251c914e46700ba190798e1911.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 04/12] run-command: avoid `close(-1)` in `start_command()` error paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:21Z","receivedAt":"2026-07-05T08:24:39Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `start_command()` fails to set up a pipe partway through, it rolls\nback by closing the pipe ends it has already opened. For descriptors\nsupplied by the caller rather than allocated locally, that rollback\ntested `if (cmd->in)` / `if (cmd->out)` before calling close(). The\nCHILD_PROCESS_INIT default of -1 (\"no descriptor\") is non-zero and so\npasses the test, meaning a caller that sets cmd->no_stdin or\ncmd->no_stdout without supplying a real fd ends up triggering close(-1)\non the error path.\n\nThe stdin-pipe failure branch a few lines above already uses the right\nidiom, `if (cmd->out > 0)`, which rejects both the -1 sentinel and 0\n(the parent's own standard streams). Apply it to the three remaining\nrollback sites.\n\nReported by Coverity as CID 1049722 (\"Argument cannot be negative\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n run-command.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex e70a8a387b..ce84db8782 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -706,7 +706,7 @@ int start_command(struct child_process *cmd)\n \t\t\tfailed_errno = errno;\n \t\t\tif (need_in)\n \t\t\t\tclose_pair(fdin);\n-\t\t\telse if (cmd->in)\n+\t\t\telse if (cmd->in > 0)\n \t\t\t\tclose(cmd->in);\n \t\t\tstr = \"standard output\";\n \t\t\tgoto fail_pipe;\n@@ -720,11 +720,11 @@ int start_command(struct child_process *cmd)\n \t\t\tfailed_errno = errno;\n \t\t\tif (need_in)\n \t\t\t\tclose_pair(fdin);\n-\t\t\telse if (cmd->in)\n+\t\t\telse if (cmd->in > 0)\n \t\t\t\tclose(cmd->in);\n \t\t\tif (need_out)\n \t\t\t\tclose_pair(fdout);\n-\t\t\telse if (cmd->out)\n+\t\t\telse if (cmd->out > 0)\n \t\t\t\tclose(cmd->out);\n \t\t\tstr = \"standard error\";\n fail_pipe:\n-- \ngitgitgadget\n\n"},{"id":"547154","messageId":"8c623cc28f5c86b33b03deec0c2d0b5486b08c02.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 05/12] line-log: avoid redundant copy that leaks in process_ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:22Z","receivedAt":"2026-07-05T08:24:41Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen `bloom_filter_check()` indicates that a commit does not touch any\nof the tracked paths, `line_log_process_ranges_arbitrary_commit()`\npropagates the current ranges to the parent by calling\n`line_log_data_copy()` and passing the copy to add_line_range().\nHowever, `add_line_range()` always makes its own copy internally (via\nline_log_data_copy or line_log_data_merge), so the caller's copy is\nnever freed and leaks every time this path is taken.\n\nPass range directly to `add_line_range()` instead of making a redundant\nintermediate copy. The callee's internal copy handles ownership\ncorrectly.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n line-log.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex 5fc75ae275..0179f138f7 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1141,8 +1141,7 @@ int line_log_process_ranges_arbitrary_commit(struct rev_info *rev, struct commit\n \n \tif (range) {\n \t\tif (commit->parents && !bloom_filter_check(rev, commit, range)) {\n-\t\t\tstruct line_log_data *prange = line_log_data_copy(range);\n-\t\t\tadd_line_range(rev, commit->parents->item, prange);\n+\t\t\tadd_line_range(rev, commit->parents->item, range);\n \t\t\tclear_commit_line_range(rev, commit);\n \t\t} else if (commit->parents && commit->parents->next)\n \t\t\tchanged = process_ranges_merge_commit(rev, commit, range);\n-- \ngitgitgadget\n\n"},{"id":"547155","messageId":"8a8fe2d3e342b912de1013082ac838e8544d7031.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 06/12] dir: free allocations on parse-error paths in `read_one_dir()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:23Z","receivedAt":"2026-07-05T08:24:43Z","isPatch":true,"body":"From: Johannes Schindelin \n\nTwo of `read_one_dir()`'s parse-error early returns leak ud.untracked\nand ud.dirs. Plug them.\n\nThe other early returns in the same function are fine: they occur after\nthe `xmalloc()`+`memcpy()` that copies ud into `*untracked_`, at which\npoint ownership is transferred to the caller.\n`read_untracked_extension()` then releases everything via\n`free_untracked_cache()` on failure.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n dir.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 32430090dc..23335b9f7a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -3792,13 +3792,18 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,\n \t\tALLOC_ARRAY(ud.untracked, ud.untracked_nr);\n \n \tud.dirs_alloc = ud.dirs_nr = decode_varint(&data);\n-\tif (data > end)\n+\tif (data > end) {\n+\t\tfree(ud.untracked);\n \t\treturn -1;\n+\t}\n \tALLOC_ARRAY(ud.dirs, ud.dirs_nr);\n \n \teos = memchr(data, '\\0', end - data);\n-\tif (!eos || eos == end)\n+\tif (!eos || eos == end) {\n+\t\tfree(ud.untracked);\n+\t\tfree(ud.dirs);\n \t\treturn -1;\n+\t}\n \n \t*untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), eos - data, 1));\n \tmemcpy(untracked, &ud, sizeof(ud));\n-- \ngitgitgadget\n\n"},{"id":"547156","messageId":"5397ea785c6da50e977598a35d03af82cb2a5e4d.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 07/12] submodule: fix cwd leak in `get_superproject_working_tree()`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:24Z","receivedAt":"2026-07-05T08:24:46Z","isPatch":true,"body":"From: Johannes Schindelin \n\n`get_superproject_working_tree()` allocates cwd via `xgetcwd()` at the\ntop of the function, but two early-return paths (when not inside a work\ntree, and when strbuf_realpath for \"../\" fails) return 0 without freeing\nit.\n\nRedirect these early returns through a cleanup label that frees cwd\nbefore returning.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n submodule.c | 19 ++++++++++---------\n 1 file changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex fd91201a92..92dfb0fc2d 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2627,13 +2627,12 @@ int get_superproject_working_tree(struct strbuf *buf)\n \t\t * We might have a superproject, but it is harder\n \t\t * to determine.\n \t\t */\n-\t\treturn 0;\n+\t\tgoto out;\n \n \tif (!strbuf_realpath(&one_up, \"../\", 0))\n-\t\treturn 0;\n+\t\tgoto out;\n \n \tsubpath = relative_path(cwd, one_up.buf, &sb);\n-\tstrbuf_release(&one_up);\n \n \tprepare_submodule_repo_env(&cp.env);\n \tstrvec_pop(&cp.env);\n@@ -2678,20 +2677,22 @@ int get_superproject_working_tree(struct strbuf *buf)\n \t\tret = 1;\n \t\tfree(super_wt);\n \t}\n-\tfree(cwd);\n-\tstrbuf_release(&sb);\n \n \tcode = finish_command(&cp);\n \n \tif (code == 128)\n \t\t/* '../' is not a git repository */\n-\t\treturn 0;\n-\tif (code == 0 && len == 0)\n+\t\tret = 0;\n+\telse if (code == 0 && len == 0)\n \t\t/* There is an unrelated git repository at '../' */\n-\t\treturn 0;\n-\tif (code)\n+\t\tret = 0;\n+\telse if (code)\n \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n \n+out:\n+\tstrbuf_release(&sb);\n+\tstrbuf_release(&one_up);\n+\tfree(cwd);\n \treturn ret;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"547157","messageId":"0048c0ca2752853dfba7ae1bf89dd70c8e501d54.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 08/12] worktree: fix resource leaks when branch creation fails","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:25Z","receivedAt":"2026-07-05T08:24:49Z","isPatch":true,"body":"From: Johannes Schindelin \n\nIn the \"add\" subcommand, when `run_command()` fails while creating a new\nbranch (line 948), the function returns -1 immediately without freeing\nthe allocations made earlier: path (from prefix_filename at line 858),\nopt_track, branch_to_free, and new_branch_to_free.\n\nRedirect the error return through the existing cleanup block at the end\nof the function so all four allocations are properly freed.\n\nPointed out by Coverity.\n\nAssisted-by: Claude Opus 4.6\nSigned-off-by: Johannes Schindelin \n---\n builtin/worktree.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d21c43fde3..4bc7b4f6e7 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -945,14 +945,17 @@ static int add(int ac, const char **av, const char *prefix,\n \t\tstrvec_push(&cp.args, branch);\n \t\tif (opt_track)\n \t\t\tstrvec_push(&cp.args, opt_track);\n-\t\tif (run_command(&cp))\n-\t\t\treturn -1;\n+\t\tif (run_command(&cp)) {\n+\t\t\tret = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbranch = new_branch;\n \t} else if (opt_track) {\n \t\tdie(_(\"--[no-]track can only be used if a new branch is created\"));\n \t}\n \n \tret = add_worktree(path, branch, &opts);\n+cleanup:\n \tfree(path);\n \tfree(opt_track);\n \tfree(branch_to_free);\n-- \ngitgitgadget\n\n"},{"id":"547158","messageId":"4048a225a5c3c0b698a2dbc58c756d217f851a72.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 09/12] imap-send: avoid leaking the IMAP upload buffer","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:26Z","receivedAt":"2026-07-05T08:24:51Z","isPatch":true,"body":"From: Johannes Schindelin \n\nWhen uploading messages via libcurl, `curl_append_msgs_to_imap()`\naccumulates each one in a strbuf that grows across loop iterations but\nis never released before the function returns.\n\nRelease it alongside the existing libcurl cleanup.\n\nReported by Coverity as CID 1671507 (\"Resource leak\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n imap-send.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex cfd6a5120c..0d16d02029 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1750,6 +1750,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,\n \n \tcurl_easy_cleanup(curl);\n \tcurl_global_cleanup();\n+\tstrbuf_release(&msgbuf.buf);\n \n \tif (cred.username) {\n \t\tif (res == CURLE_OK)\n-- \ngitgitgadget\n\n"},{"id":"547159","messageId":"13ecebcdee633689b861418a005cf4f64c190fb3.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 10/12] reftable/table: release filter on error path","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:27Z","receivedAt":"2026-07-05T08:24:52Z","isPatch":true,"body":"From: Johannes Schindelin \n\n`reftable_table_refs_for_unindexed()` allocates a filtering_ref_iterator\nand then calls `reftable_buf_add()` to populate its oid buffer. On\nsuccess ownership is transferred to the output iterator, but if\n`reftable_buf_add()` fails, the goto-out cleanup only frees the table\niterator and walks away from both the filter allocation and the oid\nbuffer that `reftable_buf_add()` may have grown.\n\nRelease filter->oid and free filter alongside the existing table\niterator cleanup.\n\nReported by Coverity as CID 1671512 (\"Resource leak\").\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n reftable/table.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/reftable/table.c b/reftable/table.c\nindex 56362df0ed..d604ddebf4 100644\n--- a/reftable/table.c\n+++ b/reftable/table.c\n@@ -709,6 +709,10 @@ out:\n \t\tif (ti)\n \t\t\ttable_iter_close(ti);\n \t\treftable_free(ti);\n+\t\tif (filter) {\n+\t\t\treftable_buf_release(&filter->oid);\n+\t\t\treftable_free(filter);\n+\t\t}\n \t}\n \treturn err;\n }\n-- \ngitgitgadget\n\n"},{"id":"547160","messageId":"97049d7cc3960937d822fb9403849d1dba063b78.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 11/12] fsmonitor: plug token-data leak on early daemon-startup failures","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:28Z","receivedAt":"2026-07-05T08:24:54Z","isPatch":true,"body":"From: Johannes Schindelin \n\n`fsmonitor_run_daemon()` allocates `state.current_token_data`\nbefore any subordinate setup step that may fail (alias resolution,\nlistener/health constructors, asynchronous IPC server init). On\nthe successful path the listener thread takes ownership and clears\nthe field during its teardown, so the `done:` cleanup block sees a\nNULL pointer. On every early-error path, however, control jumps\nstraight to `done:` with the freshly allocated token data still\nreferenced, and it is never freed, as Coverity flagged.\n\nFree it at the top of `done:` and clear the pointer. The success\npath is a no-op (the pointer is already NULL there); the error\npaths now drop the otherwise-leaked allocation.\n`fsmonitor_free_token_data()` is NULL-safe and asserts\n`client_ref_count == 0`, which holds trivially here because the\nIPC server has not yet begun accepting clients when these failures\noccur.\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n builtin/fsmonitor--daemon.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c\nindex f920cf3a82..4161dd8282 100644\n--- a/builtin/fsmonitor--daemon.c\n+++ b/builtin/fsmonitor--daemon.c\n@@ -1418,6 +1418,8 @@ static int fsmonitor_run_daemon(void)\n \terr = fsmonitor_run_daemon_1(&state);\n \n done:\n+\tfsmonitor_free_token_data(state.current_token_data);\n+\tstate.current_token_data = NULL;\n \tpthread_cond_destroy(&state.cookies_cond);\n \tpthread_mutex_destroy(&state.main_lock);\n \t{\n-- \ngitgitgadget\n\n"},{"id":"547161","messageId":"a5a6c27184097f0f8bfc1174e691dd1b94eb165d.1783239870.git.gitgitgadget@gmail.com","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"[PATCH v2 12/12] mingw: make `exit_process()` own the process handle on all paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-05T08:24:29Z","receivedAt":"2026-07-05T08:24:56Z","isPatch":true,"body":"From: Johannes Schindelin \n\nAfter \"mingw: kill child processes in a gentler way\", the ownership of\nthe HANDLE passed to `exit_process()` and `terminate_process_tree()` is\ninconsistent. `terminate_process_tree()` always closes the handle;\n`exit_process()` closes it on success and on the terminate-tree\nfallback, but leaks it on the early return where GetExitCodeProcess()\nfails or reports the process is no longer STILL_ACTIVE.\n\n`mingw_kill()` compensated by closing the handle on its own error path,\nwhich is a double-close on every error path that does not hit that one\nleaky branch -- the callee has already closed the handle by then.\nCoverity flagged the resulting use-after-free as CID 1437238.\n\nPin down the invariant that `exit_process()` and\n`terminate_process_tree()` own the handle from the call onward and close\nit on every return path; with that, the bogus close in `mingw_kill()`\ngoes away.\n\nAssisted-by: Opus 4.7\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 4 +---\n compat/win32/exit-process.h | 1 +\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 41e055f7de..e2cb92a414 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2269,10 +2269,8 @@ int mingw_kill(pid_t pid, int sig)\n \t\t\t}\n \t\t\tret = terminate_process_tree(h, 128 + sig);\n \t\t}\n-\t\tif (ret) {\n+\t\tif (ret)\n \t\t\terrno = err_win_to_posix(GetLastError());\n-\t\t\tCloseHandle(h);\n-\t\t}\n \t\treturn ret;\n \t} else if (pid > 0 && sig == 0) {\n \t\tHANDLE h = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid);\ndiff --git a/compat/win32/exit-process.h b/compat/win32/exit-process.h\nindex d53989884c..26004161bc 100644\n--- a/compat/win32/exit-process.h\n+++ b/compat/win32/exit-process.h\n@@ -159,6 +159,7 @@ static int exit_process(HANDLE process, int exit_code)\n \t\treturn terminate_process_tree(process, exit_code);\n \t}\n \n+\tCloseHandle(process);\n \treturn 0;\n }\n \n-- \ngitgitgadget\n"},{"id":"547353","messageId":"ak0goVzo1oNMTlO5@pks.im","threadId":"65900","inReplyTo":"5397ea785c6da50e977598a35d03af82cb2a5e4d.1783239870.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 07/12] submodule: fix cwd leak in `get_superproject_working_tree()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-07T15:52:01Z","receivedAt":"2026-07-07T15:52:08Z","isPatch":true,"body":"On Sun, Jul 05, 2026 at 08:24:24AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/submodule.c b/submodule.c\n> index fd91201a92..92dfb0fc2d 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -2627,13 +2627,12 @@ int get_superproject_working_tree(struct strbuf *buf)\n> \t\t * We might have a superproject, but it is harder\n> \t\t * to determine.\n> \t\t */\n> -\t\treturn 0;\n> +\t\tgoto out;\n> \n> \tif (!strbuf_realpath(&one_up, \"../\", 0))\n> -\t\treturn 0;\n> +\t\tgoto out;\n> \n> \tsubpath = relative_path(cwd, one_up.buf, &sb);\n> -\tstrbuf_release(&one_up);\n> \n> \tprepare_submodule_repo_env(&cp.env);\n> \tstrvec_pop(&cp.env);\n\nRight. `ret` is already zero-initialized at the beginning of the\nfunction, so it's fine to just `goto out` here.\n\n> @@ -2678,20 +2677,22 @@ int get_superproject_working_tree(struct strbuf *buf)\n> \t\tret = 1;\n> \t\tfree(super_wt);\n> \t}\n> -\tfree(cwd);\n> -\tstrbuf_release(&sb);\n> \n> \tcode = finish_command(&cp);\n> \n> \tif (code == 128)\n> \t\t/* '../' is not a git repository */\n> -\t\treturn 0;\n> -\tif (code == 0 && len == 0)\n> +\t\tret = 0;\n> +\telse if (code == 0 && len == 0)\n> \t\t/* There is an unrelated git repository at '../' */\n> -\t\treturn 0;\n> -\tif (code)\n> +\t\tret = 0;\n> +\telse if (code)\n> \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n\nThe diff is a bit hard to read as we also convert this to use `else if`,\nbut overall the end result is easier to reason about.\n\n> +out:\n> +\tstrbuf_release(&sb);\n> +\tstrbuf_release(&one_up);\n> +\tfree(cwd);\n> \treturn ret;\n> }\n\nAll of these variables are always initialized, so this change looks good\nto me.\n\nThanks!\n\nPatrick\n"},{"id":"547354","messageId":"ak0hjKEOwfo9lgkf@pks.im","threadId":"65900","inReplyTo":"80ae35227d566977ad21eb6e35f49e1ca5d5a940.1783239870.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 01/12] load_one_loose_object_map(): fix resource leak","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-07T15:55:56Z","receivedAt":"2026-07-07T15:56:01Z","isPatch":true,"body":"On Sun, Jul 05, 2026 at 08:24:18AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> @@ -98,13 +98,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_\n> \t\tinsert_loose_map(loose, &oid, &compat_oid);\n> \t}\n> \n> -\tstrbuf_release(&buf);\n> -\tstrbuf_release(&path);\n> -\treturn errno ? -1 : 0;\n> +\tret = ferror(fp) ? -1 : 0;\n> err:\n> +\tfclose(fp);\n> \tstrbuf_release(&buf);\n> \tstrbuf_release(&path);\n> -\treturn -1;\n> +\treturn ret;\n\nNit: it might've made sense to explain the switch to ferror(3p) in the\ncommit message, but that alone isn't worth a reroll.\n\nPatrick\n"},{"id":"547355","messageId":"ak0hj9em1agVr4rj@pks.im","threadId":"65900","inReplyTo":"pull.2163.v2.git.1783239870.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 00/12] coverity: fix leaks and error paths","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-07T15:55:59Z","receivedAt":"2026-07-07T15:56:04Z","isPatch":true,"body":"On Sun, Jul 05, 2026 at 08:24:17AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> I wanted to whittle down the many issues reported by Coverity in the Git for\n> Windows project. Turns out: The vast majority of the issues are false\n> positives. Most of the remaining issues are in core Git proper.\n> \n> This effort was forced on pause while Coverity was down from May 16\n> [https://web.archive.org/web/20260516152422/https://scan.coverity.com/] to\n> June 22\n> [https://web.archive.org/web/20260622182153/https://scan.coverity.com/]).\n> \n> Here is a first batch of fixes for those issues.\n> \n> Changes since v1:\n> \n> * Edited the commit messages to put function names in backticks, and\n> reflowed the messages afterwards.\n> * Took Junio's suggestion to avoid (ab-)using errno to determine the return\n> value of load_one_loose_object_map().\n> * Dropped the obsolete patch \"run_diff_files: avoid memory leak\".\n> * Rewrote the commit message of \"dir: free allocations on parse-error paths\n> in read_one_dir()\" to clarify ownership of the allocated untracked/dirs\n> buffers.\n> * Changed \"submodule: fix cwd leak in get_superproject_working_tree()\" to\n> reduce the cognitive load on the reader (i.e. to make it a lot easier to\n> reason about the correctness of the patch).\n\nThanks. The reflow of the commit messages made the range-diff somewhat\nhard to read, but from all I could see the changes all make sense.\n\nPatrick\n"},{"id":"547390","messageId":"xmqqa4s238lg.fsf@gitster.g","threadId":"65900","inReplyTo":"ak0hj9em1agVr4rj@pks.im","subject":"Re: [PATCH v2 00/12] coverity: fix leaks and error paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T19:25:47Z","receivedAt":"2026-07-07T19:25:49Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> On Sun, Jul 05, 2026 at 08:24:17AM +0000, Johannes Schindelin via GitGitGadget wrote:\n>> I wanted to whittle down the many issues reported by Coverity in the Git for\n>> Windows project. Turns out: The vast majority of the issues are false\n>> positives. Most of the remaining issues are in core Git proper.\n>> \n>> This effort was forced on pause while Coverity was down from May 16\n>> [https://web.archive.org/web/20260516152422/https://scan.coverity.com/] to\n>> June 22\n>> [https://web.archive.org/web/20260622182153/https://scan.coverity.com/]).\n>> \n>> Here is a first batch of fixes for those issues.\n>> \n>> Changes since v1:\n>> \n>> * Edited the commit messages to put function names in backticks, and\n>> reflowed the messages afterwards.\n>> * Took Junio's suggestion to avoid (ab-)using errno to determine the return\n>> value of load_one_loose_object_map().\n>> * Dropped the obsolete patch \"run_diff_files: avoid memory leak\".\n>> * Rewrote the commit message of \"dir: free allocations on parse-error paths\n>> in read_one_dir()\" to clarify ownership of the allocated untracked/dirs\n>> buffers.\n>> * Changed \"submodule: fix cwd leak in get_superproject_working_tree()\" to\n>> reduce the cognitive load on the reader (i.e. to make it a lot easier to\n>> reason about the correctness of the patch).\n>\n> Thanks. The reflow of the commit messages made the range-diff somewhat\n> hard to read, but from all I could see the changes all make sense.\n\nYup, this round looks good to me, too. Thanks, both.\n\n"}]}