{"thread":{"id":"62312","subject":"[PATCH 00/21] Memory leak fixes (pt.9)","startedAt":"2024-10-11T05:32:10Z","lastAt":"2024-11-05T15:28:57Z","messageCount":100,"participants":["Patrick Steinhardt","Toon Claes","Taylor Blau","Kristoffer Haugsbakk","Justin Tobler","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":21},"messages":[{"id":"504779","messageId":"cover.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":null,"subject":"[PATCH 00/21] Memory leak fixes (pt.9)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:04Z","receivedAt":"2024-10-11T05:32:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nhere's another round of memory leak fixes. With this series we're down\nto 10 test suites which are failing with the leak sanitizer. There are\ntwo patch series in flight [1][2] that fix three more test suites, so in\ntotal we're down to 7 test suites that we'll still have to fix up. So:\nwe're almost done!\n\nPatrick\n\n[1]: <20240826083052.1542228-1-toon@iotcl.com>\n[2]: \n\nPatrick Steinhardt (21):\n builtin/ls-remote: plug leaking server options\n t/helper: fix leaks in \"reach\" test tool\n grep: fix leak in `grep_splice_or()`\n builtin/grep: fix leak with `--max-count=0`\n revision: fix leaking bloom filters\n diff-lib: fix leaking diffopts in `do_diff_cache()`\n pretty: clear signature check\n upload-pack: fix leaking URI protocols\n builtin/commit: fix leaking change data contents\n trailer: fix leaking trailer values\n builtin/commit: fix leaking cleanup config\n transport-helper: fix leaking import/export marks\n builtin/tag: fix leaking key ID on failure to sign\n combine-diff: fix leaking lost lines\n dir: release untracked cache data\n sparse-index: correctly free EWAH contents\n t/helper: stop re-initialization of `the_repository`\n t/helper: fix leaking buffer in \"dump-untracked-cache\"\n dir: fix leak when parsing \"status.showUntrackedFiles\"\n builtin/merge: release outbut buffer after performing merge\n list-objects-filter-options: work around reported leak on error\n\n builtin/commit.c | 26 +++++++++++++++++------\n builtin/grep.c | 13 +++++++++---\n builtin/ls-remote.c | 1 +\n builtin/merge.c | 1 +\n builtin/tag.c | 2 +-\n combine-diff.c | 2 +-\n diff-lib.c | 1 +\n dir.c | 12 +++++++++--\n grep.c | 1 +\n list-objects-filter-options.c | 17 ++++++---------\n pretty.c | 1 +\n revision.c | 5 +++++\n sparse-index.c | 7 ++++--\n t/helper/test-dump-untracked-cache.c | 2 ++\n t/helper/test-reach.c | 10 +++++++++\n t/helper/test-read-cache.c | 2 --\n t/t4038-diff-combined.sh | 1 +\n t/t4202-log.sh | 1 +\n t/t4216-log-bloom.sh | 1 +\n t/t5702-protocol-v2.sh | 1 +\n t/t5801-remote-helpers.sh | 1 +\n t/t6112-rev-list-filters-objects.sh | 1 +\n t/t6424-merge-unrelated-index-changes.sh | 1 +\n t/t6600-test-reach.sh | 1 +\n t/t7004-tag.sh | 1 +\n t/t7031-verify-tag-signed-ssh.sh | 1 +\n t/t7063-status-untracked-cache.sh | 1 +\n t/t7500-commit-template-squash-signoff.sh | 1 +\n t/t7502-commit-porcelain.sh | 1 +\n t/t7510-signed-commit.sh | 1 +\n t/t7513-interpret-trailers.sh | 1 +\n t/t7519-status-fsmonitor.sh | 1 +\n t/t7528-signed-commit-ssh.sh | 1 +\n t/t7610-mergetool.sh | 1 +\n t/t7810-grep.sh | 1 +\n trailer.c | 18 +++++++++++-----\n transport-helper.c | 2 ++\n upload-pack.c | 1 +\n 38 files changed, 111 insertions(+), 32 deletions(-)\n\n-- \n2.47.0.dirty\n\n"},{"id":"504780","messageId":"89b66411354437a1e3a97751f185889dffb84126.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 01/21] builtin/ls-remote: plug leaking server options","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:08Z","receivedAt":"2024-10-11T05:32:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The server options populated via `OPT_STRING_LIST()` is never cleared,\ncausing a memory leak. Plug it.\n\nThis leak is exposed by t5702, but plugging it alone does not make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/ls-remote.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex f723b3bf3bb..f333821b994 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -166,6 +166,7 @@ int cmd_ls_remote(int argc,\n \t\tstatus = 0; /* we found something */\n \t}\n \n+\tstring_list_clear(&server_options, 0);\n \tref_sorting_release(sorting);\n \tref_array_clear(&ref_array);\n \tif (transport_disconnect(transport))\n-- \n2.47.0.dirty\n\n"},{"id":"504781","messageId":"1c42e194b20637ef84c61b62e29fdd9047b1acfb.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 02/21] t/helper: fix leaks in \"reach\" test tool","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:13Z","receivedAt":"2024-10-11T05:32:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"reach\" test tool doesn't bother to clean up any of its allocated\nresources, causing various leaks. Plug them.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-reach.c | 10 ++++++++++\n t/t6600-test-reach.sh | 1 +\n 2 files changed, 11 insertions(+)\n\ndiff --git a/t/helper/test-reach.c b/t/helper/test-reach.c\nindex 995e382863a..84deee604ad 100644\n--- a/t/helper/test-reach.c\n+++ b/t/helper/test-reach.c\n@@ -127,10 +127,12 @@ int cmd__reach(int ac, const char **av)\n \t\t\texit(128);\n \t\tprintf(\"%s(A,X):\\n\", av[1]);\n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t} else if (!strcmp(av[1], \"reduce_heads\")) {\n \t\tstruct commit_list *list = reduce_heads(X);\n \t\tprintf(\"%s(X):\\n\", av[1]);\n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t} else if (!strcmp(av[1], \"can_all_from_reach\")) {\n \t\tprintf(\"%s(X,Y):%d\\n\", av[1], can_all_from_reach(X, Y, 1));\n \t} else if (!strcmp(av[1], \"can_all_from_reach_with_flag\")) {\n@@ -153,6 +155,7 @@ int cmd__reach(int ac, const char **av)\n \t\t\tfilter.with_commit_tag_algo = 0;\n \n \t\tprintf(\"%s(_,A,X,_):%d\\n\", av[1], commit_contains(&filter, A, X, &cache));\n+\t\tclear_contains_cache(&cache);\n \t} else if (!strcmp(av[1], \"get_reachable_subset\")) {\n \t\tconst int reachable_flag = 1;\n \t\tint i, count = 0;\n@@ -176,7 +179,14 @@ int cmd__reach(int ac, const char **av)\n \t\t\tdie(_(\"too many commits marked reachable\"));\n \n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t}\n \n+\tobject_array_clear(&X_obj);\n+\tstrbuf_release(&buf);\n+\tfree_commit_list(X);\n+\tfree_commit_list(Y);\n+\tfree(X_array);\n+\tfree(Y_array);\n \treturn 0;\n }\ndiff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh\nindex 2591f8b8b39..307deefed2c 100755\n--- a/t/t6600-test-reach.sh\n+++ b/t/t6600-test-reach.sh\n@@ -2,6 +2,7 @@\n \n test_description='basic commit reachability tests'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Construct a grid-like commit graph with points (x,y)\n-- \n2.47.0.dirty\n\n"},{"id":"504782","messageId":"cb4eee37b4024a451a385f94ad57e7278e542de7.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 03/21] grep: fix leak in `grep_splice_or()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:16Z","receivedAt":"2024-10-11T05:32:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `grep_splice_or()` we search for the next `TRUE` node in our tree of\ngrep exrpessions and replace it with the given new expression. But we\ndon't free the old node, which causes a memory leak. Plug it.\n\nThis leak is exposed by t7810, but plugging it alone isn't sufficient to\nmake the test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n grep.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/grep.c b/grep.c\nindex 701e58de04e..e9337f32cbf 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -756,6 +756,7 @@ static struct grep_expr *grep_splice_or(struct grep_expr *x, struct grep_expr *y\n \t\tassert(x->node == GREP_NODE_OR);\n \t\tif (x->u.binary.right &&\n \t\t x->u.binary.right->node == GREP_NODE_TRUE) {\n+\t\t\tfree(x->u.binary.right);\n \t\t\tx->u.binary.right = y;\n \t\t\tbreak;\n \t\t}\n-- \n2.47.0.dirty\n\n"},{"id":"504783","messageId":"6b2c8842ef53a3e550aa3de7447c9c3d23bc6772.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 04/21] builtin/grep: fix leak with `--max-count=0`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:18Z","receivedAt":"2024-10-11T05:32:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When executing with `--max-count=0` we'll return early from git-grep(1)\nwithout performing any cleanup, which causes memory leaks. Plug these.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/grep.c | 13 ++++++++++---\n t/t7810-grep.sh | 1 +\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex f17d46a06e4..98b85c7fcac 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -906,6 +906,7 @@ int cmd_grep(int argc,\n \tint dummy;\n \tint use_index = 1;\n \tint allow_revs;\n+\tint ret;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"cached\", &cached,\n@@ -1172,8 +1173,10 @@ int cmd_grep(int argc,\n \t * Optimize out the case where the amount of matches is limited to zero.\n \t * We do this to keep results consistent with GNU grep(1).\n \t */\n-\tif (opt.max_count == 0)\n-\t\treturn 1;\n+\tif (opt.max_count == 0) {\n+\t\tret = 1;\n+\t\tgoto out;\n+\t}\n \n \tif (show_in_pager) {\n \t\tif (num_threads > 1)\n@@ -1267,10 +1270,14 @@ int cmd_grep(int argc,\n \t\thit |= wait_all();\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\n+\n+\tret = !hit;\n+\n+out:\n \tclear_pathspec(&pathspec);\n \tstring_list_clear(&path_list, 0);\n \tfree_grep_patterns(&opt);\n \tobject_array_clear(&list);\n \tfree_repos();\n-\treturn !hit;\n+\treturn ret;\n }\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex af2cf2f78ab..9e7681f0831 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -9,6 +9,7 @@ test_description='git grep various.\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_invalid_grep_expression() {\n-- \n2.47.0.dirty\n\n"},{"id":"504784","messageId":"7527b31a28f4c9d2e689c9749b6f9f0615ba1261.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 05/21] revision: fix leaking bloom filters","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:21Z","receivedAt":"2024-10-11T05:32:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The memory allocated by `prepare_to_use_bloom_filter()` is not released\nby `release_revisions()`, causing a memory leak. Plug it.\n\nSigned-off-by: Patrick Steinhardt \n---\n revision.c | 5 +++++\n t/t4216-log-bloom.sh | 1 +\n 2 files changed, 6 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex f5f5b84f2b0..8df75b82249 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3227,6 +3227,11 @@ void release_revisions(struct rev_info *revs)\n \tclear_decoration(&revs->treesame, free);\n \tline_log_free(revs);\n \toidset_clear(&revs->missing_commits);\n+\n+\tfor (int i = 0; i < revs->bloom_keys_nr; i++)\n+\t\tclear_bloom_key(&revs->bloom_keys[i]);\n+\tFREE_AND_NULL(revs->bloom_keys);\n+\trevs->bloom_keys_nr = 0;\n }\n \n static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)\ndiff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh\nindex 3f163dc3969..8d22338f6aa 100755\n--- a/t/t4216-log-bloom.sh\n+++ b/t/t4216-log-bloom.sh\n@@ -4,6 +4,7 @@ test_description='git log for a path with Bloom filters'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-chunk.sh\n \n-- \n2.47.0.dirty\n\n"},{"id":"504785","messageId":"60af98cb2c7752edc7cd5c5fe8173dc5b2522a7b.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 06/21] diff-lib: fix leaking diffopts in `do_diff_cache()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:26Z","receivedAt":"2024-10-11T05:32:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `do_diff_cache()` we initialize a new `rev_info` and then overwrite\nits `diffopt` with a user-provided set of options. This can leak memory\nbecause `repo_init_revisions()` may end up allocating memory for the\n`diffopt` itself depending on the configuration. And as that field is\noverwritten we won't ever free that.\n\nPlug the memory leak by releasing the diffopts before we overwrite them.\n\nSigned-off-by: Patrick Steinhardt \n---\n diff-lib.c | 1 +\n t/t7610-mergetool.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 6b14b959629..3cf353946f5 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -661,6 +661,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)\n \n \trepo_init_revisions(opt->repo, &revs, NULL);\n \tcopy_pathspec(&revs.prune_data, &opt->pathspec);\n+\tdiff_free(&revs.diffopt);\n \trevs.diffopt = *opt;\n \trevs.diffopt.no_free = 1;\n \ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 22b3a85b3e9..5c5e79e9905 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -10,6 +10,7 @@ Testing basic merge tool invocation'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # All the mergetool test work by checking out a temporary branch based\n-- \n2.47.0.dirty\n\n"},{"id":"504786","messageId":"5d5f6867f918460001f62aaa78f24cf3cbe53a3c.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 07/21] pretty: clear signature check","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:29Z","receivedAt":"2024-10-11T05:32:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The signature check in of the formatting context is never getting\nreleased. Fix this to plug the resulting memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n pretty.c | 1 +\n t/t4202-log.sh | 1 +\n t/t7031-verify-tag-signed-ssh.sh | 1 +\n t/t7510-signed-commit.sh | 1 +\n t/t7528-signed-commit-ssh.sh | 1 +\n 5 files changed, 5 insertions(+)\n\ndiff --git a/pretty.c b/pretty.c\nindex 6403e268900..098378720a4 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -2032,6 +2032,7 @@ void repo_format_commit_message(struct repository *r,\n \n \tfree(context.commit_encoding);\n \trepo_unuse_commit_buffer(r, commit, context.message);\n+\tsignature_check_clear(&context.signature_check);\n }\n \n static void pp_header(struct pretty_print_context *pp,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 51f7beb59f8..35bec4089a3 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -5,6 +5,7 @@ test_description='git log'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n . \"$TEST_DIRECTORY/lib-terminal.sh\"\ndiff --git a/t/t7031-verify-tag-signed-ssh.sh b/t/t7031-verify-tag-signed-ssh.sh\nindex 20913b37134..2ee62c07293 100755\n--- a/t/t7031-verify-tag-signed-ssh.sh\n+++ b/t/t7031-verify-tag-signed-ssh.sh\n@@ -4,6 +4,7 @@ test_description='signed tag tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 0d2dd29fe6a..eb229082e40 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -4,6 +4,7 @@ test_description='signed commit tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex 065f7806362..68e18856b66 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -4,6 +4,7 @@ test_description='ssh signed commit tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n-- \n2.47.0.dirty\n\n"},{"id":"504787","messageId":"0d503e40194c5d1912c7dd8a23f685546c490027.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 08/21] upload-pack: fix leaking URI protocols","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:32Z","receivedAt":"2024-10-11T05:32:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We don't clear `struct upload_pack::uri_protocols`, which causes a\nmemory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5702-protocol-v2.sh | 1 +\n upload-pack.c | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1ef540f73d3..ed55a2f7f95 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Test protocol v2 with 'git://' transport\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 6d6e0f9f980..b4a59c3518b 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -166,6 +166,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)\n \tobject_array_clear(&data->extra_edge_obj);\n \tlist_objects_filter_release(&data->filter_options);\n \tstring_list_clear(&data->allowed_filters, 0);\n+\tstring_list_clear(&data->uri_protocols, 0);\n \n \tfree((char *)data->pack_objects_hook);\n }\n-- \n2.47.0.dirty\n\n"},{"id":"504788","messageId":"9f967dfe5d55ca7150bf3e118279388290f7d28c.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 09/21] builtin/commit: fix leaking change data contents","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:35Z","receivedAt":"2024-10-11T05:32:37Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While we free the worktree change data, we never free its contents. Fix\nthis.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/commit.c | 9 ++++++++-\n t/t7500-commit-template-squash-signoff.sh | 1 +\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8db4e9df0c9..18a55bd1b91 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -728,6 +728,13 @@ static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n \trepo_unuse_commit_buffer(the_repository, commit, buffer);\n }\n \n+static void change_data_free(void *util, const char *str UNUSED)\n+{\n+\tstruct wt_status_change_data *d = util;\n+\tfree(d->rename_source);\n+\tfree(d);\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t struct commit *current_head,\n \t\t\t struct wt_status *s,\n@@ -991,7 +998,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\ts->use_color = 0;\n \t\tcommittable = run_status(s->fp, index_file, prefix, 1, s);\n \t\ts->use_color = saved_color_setting;\n-\t\tstring_list_clear(&s->change, 1);\n+\t\tstring_list_clear_func(&s->change, change_data_free);\n \t} else {\n \t\tstruct object_id oid;\n \t\tconst char *parent = \"HEAD\";\ndiff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\nindex 4dca8d97a77..379d3ed3413 100755\n--- a/t/t7500-commit-template-squash-signoff.sh\n+++ b/t/t7500-commit-template-squash-signoff.sh\n@@ -7,6 +7,7 @@ test_description='git commit\n \n Tests for template, signoff, squash and -F functions.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/lib-rebase.sh\n-- \n2.47.0.dirty\n\n"},{"id":"504789","messageId":"ca5370d572d5750e5fb21c84d4a4134669e7e3c1.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 10/21] trailer: fix leaking trailer values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:37Z","receivedAt":"2024-10-11T05:32:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Fix leaking trailer values when replacing the value with a command or\nwhen the token value is empty.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t7513-interpret-trailers.sh | 1 +\n trailer.c | 18 +++++++++++++-----\n 2 files changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 0f7d8938d98..38d6ccaa001 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -5,6 +5,7 @@\n \n test_description='git interpret-trailers'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # When we want one trailing space at the end of each line, let's use sed\ndiff --git a/trailer.c b/trailer.c\nindex 682d74505bf..5c0bfb735a9 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -249,17 +249,23 @@ static char *apply_command(struct conf_info *conf, const char *arg)\n static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)\n {\n \tif (arg_tok->conf.command || arg_tok->conf.cmd) {\n-\t\tconst char *arg;\n+\t\tchar *value_to_free = NULL;\n+\t\tchar *arg;\n+\n \t\tif (arg_tok->value && arg_tok->value[0]) {\n-\t\t\targ = arg_tok->value;\n+\t\t\targ = (char *)arg_tok->value;\n \t\t} else {\n \t\t\tif (in_tok && in_tok->value)\n \t\t\t\targ = xstrdup(in_tok->value);\n \t\t\telse\n \t\t\t\targ = xstrdup(\"\");\n+\t\t\tvalue_to_free = arg_tok->value;\n \t\t}\n+\n \t\targ_tok->value = apply_command(&arg_tok->conf, arg);\n-\t\tfree((char *)arg);\n+\n+\t\tfree(value_to_free);\n+\t\tfree(arg);\n \t}\n }\n \n@@ -1114,6 +1120,7 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\tif (item->token) {\n \t\t\tstruct strbuf tok = STRBUF_INIT;\n \t\t\tstruct strbuf val = STRBUF_INIT;\n+\n \t\t\tstrbuf_addstr(&tok, item->token);\n \t\t\tstrbuf_addstr(&val, item->value);\n \n@@ -1124,7 +1131,7 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t\t * corresponding value).\n \t\t\t */\n \t\t\tif (opts->trim_empty && !strlen(item->value))\n-\t\t\t\tcontinue;\n+\t\t\t\tgoto next;\n \n \t\t\tif (!opts->filter || opts->filter(&tok, opts->filter_data)) {\n \t\t\t\tif (opts->separator && out->len != origlen)\n@@ -1145,9 +1152,10 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t\t\tif (!opts->separator)\n \t\t\t\t\tstrbuf_addch(out, '\\n');\n \t\t\t}\n+\n+next:\n \t\t\tstrbuf_release(&tok);\n \t\t\tstrbuf_release(&val);\n-\n \t\t} else if (!opts->only_trailers) {\n \t\t\tif (opts->separator && out->len != origlen) {\n \t\t\t\tstrbuf_addbuf(out, opts->separator);\n-- \n2.47.0.dirty\n\n"},{"id":"504790","messageId":"8ca4344ed8674b6c40b5fee96d23f64517c7b9e0.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 11/21] builtin/commit: fix leaking cleanup config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:40Z","receivedAt":"2024-10-11T05:32:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The cleanup string set by the config is leaking when it is being\noverridden by an option. Fix this by tracking these via two separate\nvariables such that we can free the old value.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/commit.c | 17 ++++++++++++-----\n t/t7502-commit-porcelain.sh | 1 +\n 2 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 18a55bd1b91..71d674138c9 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -135,7 +135,7 @@ static struct strvec trailer_args = STRVEC_INIT;\n * is specified explicitly.\n */\n static enum commit_msg_cleanup_mode cleanup_mode;\n-static char *cleanup_arg;\n+static char *cleanup_config;\n \n static enum commit_whence whence;\n static int use_editor = 1, include_status = 1;\n@@ -1387,8 +1387,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (0 <= edit_flag)\n \t\tuse_editor = edit_flag;\n \n-\tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n-\n \thandle_untracked_files_arg(s);\n \n \tif (all && argc > 0)\n@@ -1636,8 +1634,10 @@ static int git_commit_config(const char *k, const char *v,\n \t\tinclude_status = git_config_bool(k, v);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(k, \"commit.cleanup\"))\n-\t\treturn git_config_string(&cleanup_arg, k, v);\n+\tif (!strcmp(k, \"commit.cleanup\")) {\n+\t\tFREE_AND_NULL(cleanup_config);\n+\t\treturn git_config_string(&cleanup_config, k, v);\n+\t}\n \tif (!strcmp(k, \"commit.gpgsign\")) {\n \t\tsign_commit = git_config_bool(k, v) ? \"\" : NULL;\n \t\treturn 0;\n@@ -1658,6 +1658,7 @@ int cmd_commit(int argc,\n \t struct repository *repo UNUSED)\n {\n \tstatic struct wt_status s;\n+\tstatic const char *cleanup_arg = NULL;\n \tstatic struct option builtin_commit_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress summary after successful commit\")),\n \t\tOPT__VERBOSE(&verbose, N_(\"show diff in commit message template\")),\n@@ -1757,6 +1758,12 @@ int cmd_commit(int argc,\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n+\tif (cleanup_arg) {\n+\t\tfree(cleanup_config);\n+\t\tcleanup_config = xstrdup(cleanup_arg);\n+\t}\n+\tcleanup_mode = get_cleanup_mode(cleanup_config, use_editor);\n+\n \tif (dry_run)\n \t\treturn dry_run_commit(argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argv, prefix, current_head, 0);\ndiff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\nindex b37e2018a74..84f1ff52b67 100755\n--- a/t/t7502-commit-porcelain.sh\n+++ b/t/t7502-commit-porcelain.sh\n@@ -5,6 +5,7 @@ test_description='git commit porcelain-ish'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n commit_msg_is () {\n-- \n2.47.0.dirty\n\n"},{"id":"504791","messageId":"16d969ed7b16a4de34484b70616291512a686005.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 12/21] transport-helper: fix leaking import/export marks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:46Z","receivedAt":"2024-10-11T05:32:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Fix leaking import and export marks for transport helpers.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5801-remote-helpers.sh | 1 +\n transport-helper.c | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex d21877150ed..d4882288a30 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -8,6 +8,7 @@ test_description='Test remote-helper import and export commands'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex 013ec79dc9c..bc27653cdee 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -399,6 +399,8 @@ static int release_helper(struct transport *transport)\n \tint res = 0;\n \tstruct helper_data *data = transport->data;\n \trefspec_clear(&data->rs);\n+\tfree(data->import_marks);\n+\tfree(data->export_marks);\n \tres = disconnect_helper(transport);\n \tfree(transport->data);\n \treturn res;\n-- \n2.47.0.dirty\n\n"},{"id":"504792","messageId":"9e4bc5bf73f47d59af012ae3e5798570570722ba.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 13/21] builtin/tag: fix leaking key ID on failure to sign","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:48Z","receivedAt":"2024-10-11T05:32:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We do not free the key ID when signing a tag fails. Do so by using\nthe common exit path.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/tag.c | 2 +-\n t/t7004-tag.sh | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 93d10d59157..c37c0a68fda 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -164,7 +164,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,\n \tint ret = -1;\n \n \tif (sign_buffer(buffer, &sig, keyid))\n-\t\treturn -1;\n+\t\tgoto out;\n \n \tif (compat) {\n \t\tconst struct git_hash_algo *algo = the_repository->hash_algo;\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b1316e62f46..42b3327e69b 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -10,6 +10,7 @@ Tests for operations with tags.'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n-- \n2.47.0.dirty\n\n"},{"id":"504793","messageId":"8d305d9b1c8b234f0a029d5db462a0f3bf5b7d12.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 14/21] combine-diff: fix leaking lost lines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:51Z","receivedAt":"2024-10-11T05:32:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `cnt` variable tracks the number of lines in a patch diff. It can\nhappen though that there are no newlines, in which case we'd still end\nup allocating our array of `sline`s. In fact, we always allocate it with\n`cnt + 2` entries. But when we loop through the array to clear it at the\nend of this function we only loop until `lno < cnt`, and thus we may not\nend up releasing whatever the two extra `sline`s contain.\n\nPlug this memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n combine-diff.c | 2 +-\n t/t4038-diff-combined.sh | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex f6b624dc288..3c6d9507fec 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1220,7 +1220,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t}\n \tfree(result);\n \n-\tfor (lno = 0; lno < cnt; lno++) {\n+\tfor (lno = 0; lno < cnt + 2; lno++) {\n \t\tif (sline[lno].lost) {\n \t\t\tstruct lline *ll = sline[lno].lost;\n \t\t\twhile (ll) {\ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex 2ce26e585c9..00190802d83 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -5,6 +5,7 @@ test_description='combined diff'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-diff.sh\n \n-- \n2.47.0.dirty\n\n"},{"id":"504794","messageId":"f977a033cf4b9d09ec32cac841038d1f5ea98374.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 15/21] dir: release untracked cache data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:54Z","receivedAt":"2024-10-11T05:32:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several cases where we invalidate untracked cache directory\nentries where we do not free the underlying data, but reset the number\nof entries. This causes us to leak memory because `free_untracked()`\nwill not iterate over any potential entries which we still had in the\narray.\n\nFix this issue by freeing old entries. The leak is exposed by t7519, but\nplugging it alone does not make the whole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n dir.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex e3ddd5b5296..cb9782fa11f 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1056,6 +1056,8 @@ static void do_invalidate_gitignore(struct untracked_cache_dir *dir)\n {\n \tint i;\n \tdir->valid = 0;\n+\tfor (size_t i = 0; i < dir->untracked_nr; i++)\n+\t\tfree(dir->untracked[i]);\n \tdir->untracked_nr = 0;\n \tfor (i = 0; i < dir->dirs_nr; i++)\n \t\tdo_invalidate_gitignore(dir->dirs[i]);\n@@ -1083,6 +1085,8 @@ static void invalidate_directory(struct untracked_cache *uc,\n \t\tuc->dir_invalidated++;\n \n \tdir->valid = 0;\n+\tfor (size_t i = 0; i < dir->untracked_nr; i++)\n+\t\tfree(dir->untracked[i]);\n \tdir->untracked_nr = 0;\n \tfor (i = 0; i < dir->dirs_nr; i++)\n \t\tdir->dirs[i]->recurse = 0;\n@@ -3573,6 +3577,8 @@ static void write_one_dir(struct untracked_cache_dir *untracked,\n \t * for safety..\n \t */\n \tif (!untracked->valid) {\n+\t\tfor (size_t i = 0; i < untracked->untracked_nr; i++)\n+\t\t\tfree(untracked->untracked[i]);\n \t\tuntracked->untracked_nr = 0;\n \t\tuntracked->check_only = 0;\n \t}\n@@ -3905,6 +3911,8 @@ static void invalidate_one_directory(struct untracked_cache *uc,\n {\n \tuc->dir_invalidated++;\n \tucd->valid = 0;\n+\tfor (size_t i = 0; i < ucd->untracked_nr; i++)\n+\t\tfree(ucd->untracked[i]);\n \tucd->untracked_nr = 0;\n }\n \n-- \n2.47.0.dirty\n\n"},{"id":"504795","messageId":"170cc61edaa38d3ff76578f2228e855f19b8b8b0.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 16/21] sparse-index: correctly free EWAH contents","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:57Z","receivedAt":"2024-10-11T05:32:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While we free the `fsmonitor_dirty` member of `struct index_state`, we\ndo not free the contents of that EWAH. Do so by using `ewah_free()`\ninstead of `FREE_AND_NULL()`.\n\nThis leak is exposed by t7519, but plugging it alone does not make the\ntest suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n sparse-index.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 3d7f2164e25..2107840bfc5 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -2,6 +2,7 @@\n \n #include \"git-compat-util.h\"\n #include \"environment.h\"\n+#include \"ewah/ewok.h\"\n #include \"gettext.h\"\n #include \"name-hash.h\"\n #include \"read-cache-ll.h\"\n@@ -242,7 +243,8 @@ int convert_to_sparse(struct index_state *istate, int flags)\n \tcache_tree_update(istate, 0);\n \n \tistate->fsmonitor_has_run_once = 0;\n-\tFREE_AND_NULL(istate->fsmonitor_dirty);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n \tFREE_AND_NULL(istate->fsmonitor_last_update);\n \n \tistate->sparse_index = INDEX_COLLAPSED;\n@@ -438,7 +440,8 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \tistate->cache_nr = full->cache_nr;\n \tistate->cache_alloc = full->cache_alloc;\n \tistate->fsmonitor_has_run_once = 0;\n-\tFREE_AND_NULL(istate->fsmonitor_dirty);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n \tFREE_AND_NULL(istate->fsmonitor_last_update);\n \n \tstrbuf_release(&base);\n-- \n2.47.0.dirty\n\n"},{"id":"504796","messageId":"8e10ee844c6bcc98f1a848452932408b20244fda.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 17/21] t/helper: stop re-initialization of `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:32:59Z","receivedAt":"2024-10-11T05:33:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While \"common-main.c\" already initializes `the_repository` for us, we do\nso a second time in the \"read-cache\" test helper. This causes a memory\nleak because the old repository's contents isn't released.\n\nStop calling `initialize_repository()` to plug this leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-read-cache.c | 2 --\n t/t7519-status-fsmonitor.sh | 1 +\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex d285c656bd3..e277dde8e71 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -11,8 +11,6 @@ int cmd__read_cache(int argc, const char **argv)\n \tint i, cnt = 1;\n \tconst char *name = NULL;\n \n-\tinitialize_repository(the_repository);\n-\n \tif (argc > 1 && skip_prefix(argv[1], \"--print-and-refresh=\", &name)) {\n \t\targc--;\n \t\targv++;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 7ee69ecdd4a..0f88a58a819 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -2,6 +2,7 @@\n \n test_description='git status with file system watcher'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Note, after \"git reset --hard HEAD\" no extensions exist other than 'TREE'\n-- \n2.47.0.dirty\n\n"},{"id":"504797","messageId":"71fd1c76b8a9daa338b2669abe0b7fbe19484abf.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 18/21] t/helper: fix leaking buffer in \"dump-untracked-cache\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:33:05Z","receivedAt":"2024-10-11T05:33:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never release the local `struct strbuf base` buffer, thus leaking\nmemory. Fix this leak.\n\nThis leak is exposed by t7063, but plugging it alone does not make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-dump-untracked-cache.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/helper/test-dump-untracked-cache.c b/t/helper/test-dump-untracked-cache.c\nindex 4f010d53249..b2e70837a90 100644\n--- a/t/helper/test-dump-untracked-cache.c\n+++ b/t/helper/test-dump-untracked-cache.c\n@@ -68,5 +68,7 @@ int cmd__dump_untracked_cache(int ac UNUSED, const char **av UNUSED)\n \tprintf(\"flags %08x\\n\", uc->dir_flags);\n \tif (uc->root)\n \t\tdump(uc->root, &base);\n+\n+\tstrbuf_release(&base);\n \treturn 0;\n }\n-- \n2.47.0.dirty\n\n"},{"id":"504798","messageId":"8ccc246432da8060b4c3a12d17f6fecebe54dff3.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 19/21] dir: fix leak when parsing \"status.showUntrackedFiles\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:33:07Z","receivedAt":"2024-10-11T05:33:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We use `repo_config_get_string()` to read \"status.showUntrackedFiles\"\nfrom the config subsystem. This function allocates the result, but we\nnever free the result after parsing it.\n\nThe value never leaves the scope of the calling function, so refactor it\nto instead use `repo_config_get_string_tmp()`, which does not hand over\nownership to the caller.\n\nSigned-off-by: Patrick Steinhardt \n---\n dir.c | 4 ++--\n t/t7063-status-untracked-cache.sh | 1 +\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex cb9782fa11f..7f35a3e3175 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2872,14 +2872,14 @@ static void set_untracked_ident(struct untracked_cache *uc)\n static unsigned new_untracked_cache_flags(struct index_state *istate)\n {\n \tstruct repository *repo = istate->repo;\n-\tchar *val;\n+\tconst char *val;\n \n \t/*\n \t * This logic is coordinated with the setting of these flags in\n \t * wt-status.c#wt_status_collect_untracked(), and the evaluation\n \t * of the config setting in commit.c#git_status_config()\n \t */\n-\tif (!repo_config_get_string(repo, \"status.showuntrackedfiles\", &val) &&\n+\tif (!repo_config_get_string_tmp(repo, \"status.showuntrackedfiles\", &val) &&\n \t !strcmp(val, \"all\"))\n \t\treturn 0;\n \ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex 8929ef481f9..13fea7931cd 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -5,6 +5,7 @@ test_description='test untracked cache'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # On some filesystems (e.g. FreeBSD's ext2 and ufs) directory mtime\n-- \n2.47.0.dirty\n\n"},{"id":"504799","messageId":"bc2206aa47e7e8be0642bb4540e7ddec22fbac91.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 20/21] builtin/merge: release outbut buffer after performing merge","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:33:10Z","receivedAt":"2024-10-11T05:33:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `obuf` member of `struct merge_options` is used to buffer output in\nsome cases. In order to not discard its allocated memory we only release\nits contents in `merge_finalize()` when we're not currently recursing\ninto a subtree.\n\nThis results in some situations where we seemingly do not release the\nbuffer reliably. We thus have calls to `strbuf_release()` for this\nbuffer scattered across the codebase. But we're missing one callsite in\ngit-merge(1), which causes a memory leak.\n\nWe should ideally refactor this interface so that callers don't have to\nknow about any such internals. But for now, paper over the issue by\nadding one more `strbuf_release()` call.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/merge.c | 1 +\n t/t6424-merge-unrelated-index-changes.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 84d0f3604bc..51038eaca84 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -754,6 +754,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\t\tclean = merge_recursive(&o, head, remoteheads->item,\n \t\t\t\t\t\treversed, &result);\n \t\tfree_commit_list(reversed);\n+\t\tstrbuf_release(&o.obuf);\n \n \t\tif (clean < 0) {\n \t\t\trollback_lock_file(&lock);\ndiff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh\nindex 7677c5f08d0..a7ea8acb845 100755\n--- a/t/t6424-merge-unrelated-index-changes.sh\n+++ b/t/t6424-merge-unrelated-index-changes.sh\n@@ -2,6 +2,7 @@\n \n test_description=\"merges with unrelated index changes\"\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Testcase for some simple merges\n-- \n2.47.0.dirty\n\n"},{"id":"504800","messageId":"6a2baf0d3e538e5f450c45c22248fbc3fefd77af.1728624670.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH 21/21] list-objects-filter-options: work around reported leak on error","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-11T05:33:13Z","receivedAt":"2024-10-11T05:33:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"This one is a little bit more curious. In t6112, we have a test that\nexercises the `git rev-list --filter` option with invalid filters. We\nexecute git-rev-list(1) via `test_must_fail`, which means that we check\nfor leaks even though Git exits with an error code. This causes the\nfollowing leak:\n\n Direct leak of 27 byte(s) in 1 object(s) allocated from:\n #0 0x5555555e6946 in realloc.part.0 lsan_interceptors.cpp.o\n #1 0x5555558fb4b6 in xrealloc wrapper.c:137:8\n #2 0x5555558b6e06 in strbuf_grow strbuf.c:112:2\n #3 0x5555558b7550 in strbuf_add strbuf.c:311:2\n #4 0x5555557c1a88 in strbuf_addstr strbuf.h:310:2\n #5 0x5555557c1d4c in parse_list_objects_filter list-objects-filter-options.c:261:3\n #6 0x555555885ead in handle_revision_pseudo_opt revision.c:2899:3\n #7 0x555555884e20 in setup_revisions revision.c:3014:11\n #8 0x5555556c4b42 in cmd_rev_list builtin/rev-list.c:588:9\n #9 0x5555555ec5e3 in run_builtin git.c:483:11\n #10 0x5555555eb1e4 in handle_builtin git.c:749:13\n #11 0x5555555ec001 in run_argv git.c:819:4\n #12 0x5555555eaf94 in cmd_main git.c:954:19\n #13 0x5555556fd569 in main common-main.c:64:11\n #14 0x7ffff7ca714d in __libc_start_call_main (.../lib/libc.so.6+0x2a14d)\n #15 0x7ffff7ca7208 in __libc_start_main@GLIBC_2.2.5 (.../libc.so.6+0x2a208)\n #16 0x5555555ad064 in _start (git+0x59064)\n\nThis leak is valid, as we call `die()` and do not clean up the memory at\nall. But what's curious is that this is the only leak reported, because\nwe don't clean up any other allocated memory, either, and I have no idea\nwhy the leak sanitizer treats this buffer specially.\n\nIn any case, we can work around the leak by shuffling things around a\nbit. Instead of calling `gently_parse_list_objects_filter()` and dying\nafter we have modified the filter spec, we simply do so beforehand. Like\nthis we don't allocate the buffer in the error case, which makes the\nreported leak go away.\n\nIt's not pretty, but it manages to make t6112 leak free.\n\nSigned-off-by: Patrick Steinhardt \n---\n list-objects-filter-options.c | 17 +++++++----------\n t/t6112-rev-list-filters-objects.sh | 1 +\n 2 files changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 00611107d20..fa72e81e4ad 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -252,16 +252,14 @@ void parse_list_objects_filter(\n \tconst char *arg)\n {\n \tstruct strbuf errbuf = STRBUF_INIT;\n-\tint parse_error;\n \n \tif (!filter_options->filter_spec.buf)\n \t\tBUG(\"filter_options not properly initialized\");\n \n \tif (!filter_options->choice) {\n+\t\tif (gently_parse_list_objects_filter(filter_options, arg, &errbuf))\n+\t\t\tdie(\"%s\", errbuf.buf);\n \t\tstrbuf_addstr(&filter_options->filter_spec, arg);\n-\n-\t\tparse_error = gently_parse_list_objects_filter(\n-\t\t\tfilter_options, arg, &errbuf);\n \t} else {\n \t\tstruct list_objects_filter_options *sub;\n \n@@ -271,18 +269,17 @@ void parse_list_objects_filter(\n \t\t */\n \t\ttransform_to_combine_type(filter_options);\n \n-\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n-\t\tfilter_spec_append_urlencode(filter_options, arg);\n \t\tALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,\n \t\t\t filter_options->sub_alloc);\n \t\tsub = &filter_options->sub[filter_options->sub_nr - 1];\n \n \t\tlist_objects_filter_init(sub);\n-\t\tparse_error = gently_parse_list_objects_filter(sub, arg,\n-\t\t\t\t\t\t\t &errbuf);\n+\t\tif (gently_parse_list_objects_filter(sub, arg, &errbuf))\n+\t\t\tdie(\"%s\", errbuf.buf);\n+\n+\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n+\t\tfilter_spec_append_urlencode(filter_options, arg);\n \t}\n-\tif (parse_error)\n-\t\tdie(\"%s\", errbuf.buf);\n }\n \n int opt_parse_list_objects_filter(const struct option *opt,\ndiff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\nindex 0387f35a326..71e38491fa8 100755\n--- a/t/t6112-rev-list-filters-objects.sh\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -5,6 +5,7 @@ test_description='git rev-list using object filtering'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Test the blob:none filter.\n-- \n2.47.0.dirty\n\n"},{"id":"505419","messageId":"87a5f1d3fb.fsf@iotcl.com","threadId":"62312","inReplyTo":"5d5f6867f918460001f62aaa78f24cf3cbe53a3c.1728624670.git.ps@pks.im","subject":"Re: [PATCH 07/21] pretty: clear signature check","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-10-18T12:02:48Z","receivedAt":"2024-10-18T12:03:01Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> The signature check in of the formatting context is never getting\n> released. Fix this to plug the resulting memory leak.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> pretty.c | 1 +\n> t/t4202-log.sh | 1 +\n> t/t7031-verify-tag-signed-ssh.sh | 1 +\n> t/t7510-signed-commit.sh | 1 +\n> t/t7528-signed-commit-ssh.sh | 1 +\n> 5 files changed, 5 insertions(+)\n>\n> diff --git a/pretty.c b/pretty.c\n> index 6403e268900..098378720a4 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -2032,6 +2032,7 @@ void repo_format_commit_message(struct repository *r,\n> \n> \tfree(context.commit_encoding);\n> \trepo_unuse_commit_buffer(r, commit, context.message);\n> +\tsignature_check_clear(&context.signature_check);\n\nI was having a very hard time finding where this gets allocated, and to\nbe honest, I still don't know for sure. I think in\ncheck_commit_signature() which is called by format_commit_one().\nIn \"[PATCH 20/21] builtin/merge: release outbut buffer after performing\nmerge\" you mention it's not obvious to the caller they need know about\nmemory they need to clean up, isn't that case here as well?\n\n--\nToon\n"},{"id":"505420","messageId":"878quld3e9.fsf@iotcl.com","threadId":"62312","inReplyTo":"ca5370d572d5750e5fb21c84d4a4134669e7e3c1.1728624670.git.ps@pks.im","subject":"Re: [PATCH 10/21] trailer: fix leaking trailer values","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-10-18T12:03:26Z","receivedAt":"2024-10-18T12:03:40Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> Fix leaking trailer values when replacing the value with a command or\n> when the token value is empty.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n>\n> diff --git a/trailer.c b/trailer.c\n> index 682d74505bf..5c0bfb735a9 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -1124,7 +1131,7 @@ void format_trailers(const struct process_trailer_options *opts,\n> \t\t\t * corresponding value).\n> \t\t\t */\n> \t\t\tif (opts->trim_empty && !strlen(item->value))\n> -\t\t\t\tcontinue;\n> +\t\t\t\tgoto next;\n\nWhile this is technically correct, I found it rather hard to understand\nwhat's happening. What do you think about instead turning the `if` below\nin an `else if` ?\n\n> \n> \t\t\tif (!opts->filter || opts->filter(&tok, opts->filter_data)) {\n> \t\t\t\tif (opts->separator && out->len != origlen)\n> @@ -1145,9 +1152,10 @@ void format_trailers(const struct process_trailer_options *opts,\n> \t\t\t\tif (!opts->separator)\n> \t\t\t\t\tstrbuf_addch(out, '\\n');\n> \t\t\t}\n> +\n> +next:\n> \t\t\tstrbuf_release(&tok);\n> \t\t\tstrbuf_release(&val);\n> -\n> \t\t} else if (!opts->only_trailers) {\n> \t\t\tif (opts->separator && out->len != origlen) {\n> \t\t\t\tstrbuf_addbuf(out, opts->separator);\n> -- \n> 2.47.0.dirty\n\n--\nToon\n"},{"id":"505421","messageId":"877ca5d3dn.fsf@iotcl.com","threadId":"62312","inReplyTo":"bc2206aa47e7e8be0642bb4540e7ddec22fbac91.1728624670.git.ps@pks.im","subject":"Re: [PATCH 20/21] builtin/merge: release outbut buffer after performing merge","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-10-18T12:03:48Z","receivedAt":"2024-10-18T12:04:00Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> The `obuf` member of `struct merge_options` is used to buffer output in\n> some cases. In order to not discard its allocated memory we only release\n> its contents in `merge_finalize()` when we're not currently recursing\n> into a subtree.\n>\n> This results in some situations where we seemingly do not release the\n> buffer reliably. We thus have calls to `strbuf_release()` for this\n> buffer scattered across the codebase. But we're missing one callsite in\n> git-merge(1), which causes a memory leak.\n>\n> We should ideally refactor this interface so that callers don't have to\n> know about any such internals. But for now, paper over the issue by\n> adding one more `strbuf_release()` call.\n\nShall we mark this as #leftoverbits?\n\n--\nToon\n"},{"id":"505422","messageId":"875xppd3ct.fsf@iotcl.com","threadId":"62312","inReplyTo":"6a2baf0d3e538e5f450c45c22248fbc3fefd77af.1728624670.git.ps@pks.im","subject":"Re: [PATCH 21/21] list-objects-filter-options: work around reported leak on error","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-10-18T12:04:18Z","receivedAt":"2024-10-18T12:04:31Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> This one is a little bit more curious. In t6112, we have a test that\n> exercises the `git rev-list --filter` option with invalid filters. We\n> execute git-rev-list(1) via `test_must_fail`, which means that we check\n> for leaks even though Git exits with an error code. This causes the\n> following leak:\n>\n> Direct leak of 27 byte(s) in 1 object(s) allocated from:\n> #0 0x5555555e6946 in realloc.part.0 lsan_interceptors.cpp.o\n> #1 0x5555558fb4b6 in xrealloc wrapper.c:137:8\n> #2 0x5555558b6e06 in strbuf_grow strbuf.c:112:2\n> #3 0x5555558b7550 in strbuf_add strbuf.c:311:2\n> #4 0x5555557c1a88 in strbuf_addstr strbuf.h:310:2\n> #5 0x5555557c1d4c in parse_list_objects_filter list-objects-filter-options.c:261:3\n> #6 0x555555885ead in handle_revision_pseudo_opt revision.c:2899:3\n> #7 0x555555884e20 in setup_revisions revision.c:3014:11\n> #8 0x5555556c4b42 in cmd_rev_list builtin/rev-list.c:588:9\n> #9 0x5555555ec5e3 in run_builtin git.c:483:11\n> #10 0x5555555eb1e4 in handle_builtin git.c:749:13\n> #11 0x5555555ec001 in run_argv git.c:819:4\n> #12 0x5555555eaf94 in cmd_main git.c:954:19\n> #13 0x5555556fd569 in main common-main.c:64:11\n> #14 0x7ffff7ca714d in __libc_start_call_main (.../lib/libc.so.6+0x2a14d)\n> #15 0x7ffff7ca7208 in __libc_start_main@GLIBC_2.2.5 (.../libc.so.6+0x2a208)\n> #16 0x5555555ad064 in _start (git+0x59064)\n>\n> This leak is valid, as we call `die()` and do not clean up the memory at\n> all. But what's curious is that this is the only leak reported, because\n> we don't clean up any other allocated memory, either, and I have no idea\n> why the leak sanitizer treats this buffer specially.\n>\n> In any case, we can work around the leak by shuffling things around a\n> bit. Instead of calling `gently_parse_list_objects_filter()` and dying\n> after we have modified the filter spec, we simply do so beforehand. Like\n> this we don't allocate the buffer in the error case, which makes the\n> reported leak go away.\n>\n> It's not pretty, but it manages to make t6112 leak free.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> list-objects-filter-options.c | 17 +++++++----------\n> t/t6112-rev-list-filters-objects.sh | 1 +\n> 2 files changed, 8 insertions(+), 10 deletions(-)\n>\n> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\n> index 00611107d20..fa72e81e4ad 100644\n> --- a/list-objects-filter-options.c\n> +++ b/list-objects-filter-options.c\n> @@ -252,16 +252,14 @@ void parse_list_objects_filter(\n> \tconst char *arg)\n> {\n> \tstruct strbuf errbuf = STRBUF_INIT;\n> -\tint parse_error;\n> \n> \tif (!filter_options->filter_spec.buf)\n> \t\tBUG(\"filter_options not properly initialized\");\n> \n> \tif (!filter_options->choice) {\n> +\t\tif (gently_parse_list_objects_filter(filter_options, arg, &errbuf))\n> +\t\t\tdie(\"%s\", errbuf.buf);\n> \t\tstrbuf_addstr(&filter_options->filter_spec, arg);\n> -\n> -\t\tparse_error = gently_parse_list_objects_filter(\n> -\t\t\tfilter_options, arg, &errbuf);\n> \t} else {\n> \t\tstruct list_objects_filter_options *sub;\n> \n> @@ -271,18 +269,17 @@ void parse_list_objects_filter(\n> \t\t */\n> \t\ttransform_to_combine_type(filter_options);\n> \n> -\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n> -\t\tfilter_spec_append_urlencode(filter_options, arg);\n> \t\tALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,\n> \t\t\t filter_options->sub_alloc);\n> \t\tsub = &filter_options->sub[filter_options->sub_nr - 1];\n> \n> \t\tlist_objects_filter_init(sub);\n> -\t\tparse_error = gently_parse_list_objects_filter(sub, arg,\n> -\t\t\t\t\t\t\t &errbuf);\n> +\t\tif (gently_parse_list_objects_filter(sub, arg, &errbuf))\n> +\t\t\tdie(\"%s\", errbuf.buf);\n\nDo we actually have a test hitting this code path? I wanted to figure\nout why `filter_options->sub` wasn't leaky (I assume that's what you're\ntalking about in your commit message), but I wasn't able to reproduce a\nscenario where we should die here.\n\n--\nToon\n"},{"id":"505471","messageId":"ZxLTfBEIEpvgM376@nand.local","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"Re: [PATCH 00/21] Memory leak fixes (pt.9)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-18T21:30:36Z","receivedAt":"2024-10-18T21:30:40Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Oct 11, 2024 at 07:32:04AM +0200, Patrick Steinhardt wrote:\n> Hi,\n>\n> here's another round of memory leak fixes. With this series we're down\n> to 10 test suites which are failing with the leak sanitizer. There are\n> two patch series in flight [1][2] that fix three more test suites, so in\n> total we're down to 7 test suites that we'll still have to fix up. So:\n> we're almost done!\n\nI was just looking through Toon's responses lower down in the thread,\nand realized that I never picked this one up.\n\nI think that makes sense... since this came on 11 October, which is when\nJunio signed off for vacation. I think that we each assumed that the\nother would (or had) pick it up.\n\nIn any case, this is now in my tree as 'ps/leakfixes-part-9'. Sorry for\nthe wait.\n\nThanks,\nTaylor\n"},{"id":"505592","messageId":"ZxYUfyh5j7EiVRdC@pks.im","threadId":"62312","inReplyTo":"87a5f1d3fb.fsf@iotcl.com","subject":"Re: [PATCH 07/21] pretty: clear signature check","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T08:44:53Z","receivedAt":"2024-10-21T08:44:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 02:02:48PM +0200, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> \n> > The signature check in of the formatting context is never getting\n> > released. Fix this to plug the resulting memory leak.\n> >\n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> > pretty.c | 1 +\n> > t/t4202-log.sh | 1 +\n> > t/t7031-verify-tag-signed-ssh.sh | 1 +\n> > t/t7510-signed-commit.sh | 1 +\n> > t/t7528-signed-commit-ssh.sh | 1 +\n> > 5 files changed, 5 insertions(+)\n> >\n> > diff --git a/pretty.c b/pretty.c\n> > index 6403e268900..098378720a4 100644\n> > --- a/pretty.c\n> > +++ b/pretty.c\n> > @@ -2032,6 +2032,7 @@ void repo_format_commit_message(struct repository *r,\n> > \n> > \tfree(context.commit_encoding);\n> > \trepo_unuse_commit_buffer(r, commit, context.message);\n> > +\tsignature_check_clear(&context.signature_check);\n> \n> I was having a very hard time finding where this gets allocated, and to\n> be honest, I still don't know for sure. I think in\n> check_commit_signature() which is called by format_commit_one().\n> In \"[PATCH 20/21] builtin/merge: release outbut buffer after performing\n> merge\" you mention it's not obvious to the caller they need know about\n> memory they need to clean up, isn't that case here as well?\n\nWell, I think it's clearer in this context, but hidden by the fact that\nwe pass around a wrapper-structure. But that's not really the problem of\nthe `struct signature_check` subsystem, but rather of \"pretty.c\".\n\nIn any case, the callchain is:\n\n - `format_commit_one()`\n - `check_commit_signature()`\n - `check_signature()`\n - `verify_signed_buffer()`, which is a function pointer depending on\n whether we verify a GPG, x509 or SSH signature.\n\nFrom my point of view, that callchain isn't all that important in this\ncontext. All we need to know is that we allocate the signature check\nstruct on our stack, and as it may get populated we have to clean it up,\nas well.\n\nPatrick\n"},{"id":"505593","messageId":"ZxYUiZ16qbPGi2S-@pks.im","threadId":"62312","inReplyTo":"878quld3e9.fsf@iotcl.com","subject":"Re: [PATCH 10/21] trailer: fix leaking trailer values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T08:44:57Z","receivedAt":"2024-10-21T08:45:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 02:03:26PM +0200, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> \n> > Fix leaking trailer values when replacing the value with a command or\n> > when the token value is empty.\n> >\n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> >\n> > diff --git a/trailer.c b/trailer.c\n> > index 682d74505bf..5c0bfb735a9 100644\n> > --- a/trailer.c\n> > +++ b/trailer.c\n> > @@ -1124,7 +1131,7 @@ void format_trailers(const struct process_trailer_options *opts,\n> > \t\t\t * corresponding value).\n> > \t\t\t */\n> > \t\t\tif (opts->trim_empty && !strlen(item->value))\n> > -\t\t\t\tcontinue;\n> > +\t\t\t\tgoto next;\n> \n> While this is technically correct, I found it rather hard to understand\n> what's happening. What do you think about instead turning the `if` below\n> in an `else if` ?\n\nAn even better idea is to lift the buffers outside of the loop. Like this\nwe don't have to reallocate them on every iteration and can easily\nrelease them once at the end of the function.\n\nPatrick\n"},{"id":"505594","messageId":"ZxYUjY39ihqUEjH-@pks.im","threadId":"62312","inReplyTo":"877ca5d3dn.fsf@iotcl.com","subject":"Re: [PATCH 20/21] builtin/merge: release outbut buffer after performing merge","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T08:45:01Z","receivedAt":"2024-10-21T08:45:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 02:03:48PM +0200, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> \n> > The `obuf` member of `struct merge_options` is used to buffer output in\n> > some cases. In order to not discard its allocated memory we only release\n> > its contents in `merge_finalize()` when we're not currently recursing\n> > into a subtree.\n> >\n> > This results in some situations where we seemingly do not release the\n> > buffer reliably. We thus have calls to `strbuf_release()` for this\n> > buffer scattered across the codebase. But we're missing one callsite in\n> > git-merge(1), which causes a memory leak.\n> >\n> > We should ideally refactor this interface so that callers don't have to\n> > know about any such internals. But for now, paper over the issue by\n> > adding one more `strbuf_release()` call.\n> \n> Shall we mark this as #leftoverbits?\n\nI guess it is now marked as such :)\n\nPatrick\n"},{"id":"505595","messageId":"ZxYUkKAm0iBT8f5L@pks.im","threadId":"62312","inReplyTo":"875xppd3ct.fsf@iotcl.com","subject":"Re: [PATCH 21/21] list-objects-filter-options: work around reported leak on error","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T08:45:04Z","receivedAt":"2024-10-21T08:45:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 02:04:18PM +0200, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> > diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\n> > index 00611107d20..fa72e81e4ad 100644\n> > --- a/list-objects-filter-options.c\n> > +++ b/list-objects-filter-options.c\n> > @@ -252,16 +252,14 @@ void parse_list_objects_filter(\n> > \tconst char *arg)\n> > {\n> > \tstruct strbuf errbuf = STRBUF_INIT;\n> > -\tint parse_error;\n> > \n> > \tif (!filter_options->filter_spec.buf)\n> > \t\tBUG(\"filter_options not properly initialized\");\n> > \n> > \tif (!filter_options->choice) {\n> > +\t\tif (gently_parse_list_objects_filter(filter_options, arg, &errbuf))\n> > +\t\t\tdie(\"%s\", errbuf.buf);\n> > \t\tstrbuf_addstr(&filter_options->filter_spec, arg);\n> > -\n> > -\t\tparse_error = gently_parse_list_objects_filter(\n> > -\t\t\tfilter_options, arg, &errbuf);\n> > \t} else {\n> > \t\tstruct list_objects_filter_options *sub;\n> > \n> > @@ -271,18 +269,17 @@ void parse_list_objects_filter(\n> > \t\t */\n> > \t\ttransform_to_combine_type(filter_options);\n> > \n> > -\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n> > -\t\tfilter_spec_append_urlencode(filter_options, arg);\n> > \t\tALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,\n> > \t\t\t filter_options->sub_alloc);\n> > \t\tsub = &filter_options->sub[filter_options->sub_nr - 1];\n> > \n> > \t\tlist_objects_filter_init(sub);\n> > -\t\tparse_error = gently_parse_list_objects_filter(sub, arg,\n> > -\t\t\t\t\t\t\t &errbuf);\n> > +\t\tif (gently_parse_list_objects_filter(sub, arg, &errbuf))\n> > +\t\t\tdie(\"%s\", errbuf.buf);\n> \n> Do we actually have a test hitting this code path? I wanted to figure\n> out why `filter_options->sub` wasn't leaky (I assume that's what you're\n> talking about in your commit message), but I wasn't able to reproduce a\n> scenario where we should die here.\n\nYou only refer to the second hunk, right? I couldn't find any code paths\nhitting this, so this is more of a \"Let's massage this code in the same\nway\" change. I don't want the next person to go through the same rabbit\nhole as I had to go through :)\n\nPatrick\n"},{"id":"505596","messageId":"ZxYUxBQCUk4RIHNd@pks.im","threadId":"62312","inReplyTo":"ZxLTfBEIEpvgM376@nand.local","subject":"Re: [PATCH 00/21] Memory leak fixes (pt.9)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T08:45:56Z","receivedAt":"2024-10-21T08:46:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 05:30:36PM -0400, Taylor Blau wrote:\n> On Fri, Oct 11, 2024 at 07:32:04AM +0200, Patrick Steinhardt wrote:\n> > Hi,\n> >\n> > here's another round of memory leak fixes. With this series we're down\n> > to 10 test suites which are failing with the leak sanitizer. There are\n> > two patch series in flight [1][2] that fix three more test suites, so in\n> > total we're down to 7 test suites that we'll still have to fix up. So:\n> > we're almost done!\n> \n> I was just looking through Toon's responses lower down in the thread,\n> and realized that I never picked this one up.\n> \n> I think that makes sense... since this came on 11 October, which is when\n> Junio signed off for vacation. I think that we each assumed that the\n> other would (or had) pick it up.\n> \n> In any case, this is now in my tree as 'ps/leakfixes-part-9'. Sorry for\n> the wait.\n\nNo worries, I'm happy that you keep things moving in the first place.\nThanks!\n\nPatrick\n"},{"id":"505598","messageId":"cover.1729502823.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH v2 00/22] Memory leak fixes (pt.9)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:27:47Z","receivedAt":"2024-10-21T09:27:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the second version of my 9th series of memory leak fixes.\n\nChanges compared to v1:\n\n - Split up the trailer fixes into two separate patches so that we can\n explain them standalone.\n\n - Adapt the second trailer memory leak fix to instead pull up the\n strbufs out of the loop such that we can reuse them. This makes the\n code flow more naturally and optimizes the loop.\n\nThanks!\n\nPatrick\n\nPatrick Steinhardt (22):\n builtin/ls-remote: plug leaking server options\n t/helper: fix leaks in \"reach\" test tool\n grep: fix leak in `grep_splice_or()`\n builtin/grep: fix leak with `--max-count=0`\n revision: fix leaking bloom filters\n diff-lib: fix leaking diffopts in `do_diff_cache()`\n pretty: clear signature check\n upload-pack: fix leaking URI protocols\n builtin/commit: fix leaking change data contents\n trailer: fix leaking trailer values\n trailer: fix leaking strbufs when formatting trailers\n builtin/commit: fix leaking cleanup config\n transport-helper: fix leaking import/export marks\n builtin/tag: fix leaking key ID on failure to sign\n combine-diff: fix leaking lost lines\n dir: release untracked cache data\n sparse-index: correctly free EWAH contents\n t/helper: stop re-initialization of `the_repository`\n t/helper: fix leaking buffer in \"dump-untracked-cache\"\n dir: fix leak when parsing \"status.showUntrackedFiles\"\n builtin/merge: release outbut buffer after performing merge\n list-objects-filter-options: work around reported leak on error\n\n builtin/commit.c | 26 +++++++++++++++++------\n builtin/grep.c | 13 +++++++++---\n builtin/ls-remote.c | 1 +\n builtin/merge.c | 1 +\n builtin/tag.c | 2 +-\n combine-diff.c | 2 +-\n diff-lib.c | 1 +\n dir.c | 12 +++++++++--\n grep.c | 1 +\n list-objects-filter-options.c | 17 ++++++---------\n pretty.c | 1 +\n revision.c | 5 +++++\n sparse-index.c | 7 ++++--\n t/helper/test-dump-untracked-cache.c | 2 ++\n t/helper/test-reach.c | 10 +++++++++\n t/helper/test-read-cache.c | 2 --\n t/t4038-diff-combined.sh | 1 +\n t/t4202-log.sh | 1 +\n t/t4216-log-bloom.sh | 1 +\n t/t5702-protocol-v2.sh | 1 +\n t/t5801-remote-helpers.sh | 1 +\n t/t6112-rev-list-filters-objects.sh | 1 +\n t/t6424-merge-unrelated-index-changes.sh | 1 +\n t/t6600-test-reach.sh | 1 +\n t/t7004-tag.sh | 1 +\n t/t7031-verify-tag-signed-ssh.sh | 1 +\n t/t7063-status-untracked-cache.sh | 1 +\n t/t7500-commit-template-squash-signoff.sh | 1 +\n t/t7502-commit-porcelain.sh | 1 +\n t/t7510-signed-commit.sh | 1 +\n t/t7513-interpret-trailers.sh | 1 +\n t/t7519-status-fsmonitor.sh | 1 +\n t/t7528-signed-commit-ssh.sh | 1 +\n t/t7610-mergetool.sh | 1 +\n t/t7810-grep.sh | 1 +\n trailer.c | 25 +++++++++++++++-------\n transport-helper.c | 2 ++\n upload-pack.c | 1 +\n 38 files changed, 115 insertions(+), 35 deletions(-)\n\nRange-diff against v1:\n 1: 89b66411354 = 1: 89b66411354 builtin/ls-remote: plug leaking server options\n 2: 1c42e194b20 = 2: 1c42e194b20 t/helper: fix leaks in \"reach\" test tool\n 3: cb4eee37b40 = 3: cb4eee37b40 grep: fix leak in `grep_splice_or()`\n 4: 6b2c8842ef5 = 4: 6b2c8842ef5 builtin/grep: fix leak with `--max-count=0`\n 5: 7527b31a28f = 5: 7527b31a28f revision: fix leaking bloom filters\n 6: 60af98cb2c7 = 6: 60af98cb2c7 diff-lib: fix leaking diffopts in `do_diff_cache()`\n 7: 5d5f6867f91 = 7: 5d5f6867f91 pretty: clear signature check\n 8: 0d503e40194 = 8: 0d503e40194 upload-pack: fix leaking URI protocols\n 9: 9f967dfe5d5 = 9: 9f967dfe5d5 builtin/commit: fix leaking change data contents\n -: ----------- > 10: e3ffd59123f trailer: fix leaking trailer values\n10: ca5370d572d ! 11: 5b851453bce trailer: fix leaking trailer values\n @@ Metadata\n Author: Patrick Steinhardt \n \n ## Commit message ##\n - trailer: fix leaking trailer values\n + trailer: fix leaking strbufs when formatting trailers\n \n - Fix leaking trailer values when replacing the value with a command or\n - when the token value is empty.\n + We are populating, but never releasing two string buffers in\n + `format_trailers()`, causing a memory leak. Plug this leak by lifting\n + those buffers outside of the loop and releasing them on function return.\n + This fixes the memory leaks, but also optimizes the loop as we don't\n + have to reallocate the buffers on every single iteration.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ t/t7513-interpret-trailers.sh\n # When we want one trailing space at the end of each line, let's use sed\n \n ## trailer.c ##\n -@@ trailer.c: static char *apply_command(struct conf_info *conf, const char *arg)\n - static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)\n +@@ trailer.c: void format_trailers(const struct process_trailer_options *opts,\n + \t\t struct list_head *trailers,\n + \t\t struct strbuf *out)\n {\n - \tif (arg_tok->conf.command || arg_tok->conf.cmd) {\n --\t\tconst char *arg;\n -+\t\tchar *value_to_free = NULL;\n -+\t\tchar *arg;\n -+\n - \t\tif (arg_tok->value && arg_tok->value[0]) {\n --\t\t\targ = arg_tok->value;\n -+\t\t\targ = (char *)arg_tok->value;\n - \t\t} else {\n - \t\t\tif (in_tok && in_tok->value)\n - \t\t\t\targ = xstrdup(in_tok->value);\n - \t\t\telse\n - \t\t\t\targ = xstrdup(\"\");\n -+\t\t\tvalue_to_free = arg_tok->value;\n - \t\t}\n -+\n - \t\targ_tok->value = apply_command(&arg_tok->conf, arg);\n --\t\tfree((char *)arg);\n -+\n -+\t\tfree(value_to_free);\n -+\t\tfree(arg);\n - \t}\n - }\n ++\tstruct strbuf tok = STRBUF_INIT;\n ++\tstruct strbuf val = STRBUF_INIT;\n + \tsize_t origlen = out->len;\n + \tstruct list_head *pos;\n + \tstruct trailer_item *item;\n \n -@@ trailer.c: void format_trailers(const struct process_trailer_options *opts,\n - \t\tif (item->token) {\n - \t\t\tstruct strbuf tok = STRBUF_INIT;\n - \t\t\tstruct strbuf val = STRBUF_INIT;\n +\n + \tlist_for_each(pos, trailers) {\n + \t\titem = list_entry(pos, struct trailer_item, list);\n + \t\tif (item->token) {\n +-\t\t\tstruct strbuf tok = STRBUF_INIT;\n +-\t\t\tstruct strbuf val = STRBUF_INIT;\n ++\t\t\tstrbuf_reset(&tok);\n \t\t\tstrbuf_addstr(&tok, item->token);\n ++\t\t\tstrbuf_reset(&val);\n \t\t\tstrbuf_addstr(&val, item->value);\n \n -@@ trailer.c: void format_trailers(const struct process_trailer_options *opts,\n - \t\t\t * corresponding value).\n - \t\t\t */\n - \t\t\tif (opts->trim_empty && !strlen(item->value))\n --\t\t\t\tcontinue;\n -+\t\t\t\tgoto next;\n - \n - \t\t\tif (!opts->filter || opts->filter(&tok, opts->filter_data)) {\n - \t\t\t\tif (opts->separator && out->len != origlen)\n + \t\t\t/*\n @@ trailer.c: void format_trailers(const struct process_trailer_options *opts,\n \t\t\t\tif (!opts->separator)\n \t\t\t\t\tstrbuf_addch(out, '\\n');\n \t\t\t}\n -+\n -+next:\n - \t\t\tstrbuf_release(&tok);\n - \t\t\tstrbuf_release(&val);\n +-\t\t\tstrbuf_release(&tok);\n +-\t\t\tstrbuf_release(&val);\n -\n \t\t} else if (!opts->only_trailers) {\n \t\t\tif (opts->separator && out->len != origlen) {\n \t\t\t\tstrbuf_addbuf(out, opts->separator);\n +@@ trailer.c: void format_trailers(const struct process_trailer_options *opts,\n + \t\t\t\tstrbuf_addch(out, '\\n');\n + \t\t}\n + \t}\n ++\n ++\tstrbuf_release(&tok);\n ++\tstrbuf_release(&val);\n + }\n + \n + void format_trailers_from_commit(const struct process_trailer_options *opts,\n11: 8ca4344ed86 = 12: 60c3f6146f3 builtin/commit: fix leaking cleanup config\n12: 16d969ed7b1 = 13: ea81cd8db86 transport-helper: fix leaking import/export marks\n13: 9e4bc5bf73f = 14: b700ab9079f builtin/tag: fix leaking key ID on failure to sign\n14: 8d305d9b1c8 = 15: 76bbcb3fe30 combine-diff: fix leaking lost lines\n15: f977a033cf4 = 16: d6b96a4012d dir: release untracked cache data\n16: 170cc61edaa = 17: 3aa6bac5079 sparse-index: correctly free EWAH contents\n17: 8e10ee844c6 = 18: 132fe750906 t/helper: stop re-initialization of `the_repository`\n18: 71fd1c76b8a = 19: b8b702eeb28 t/helper: fix leaking buffer in \"dump-untracked-cache\"\n19: 8ccc246432d = 20: ad309ac1b37 dir: fix leak when parsing \"status.showUntrackedFiles\"\n20: bc2206aa47e = 21: 5e243f9ee53 builtin/merge: release outbut buffer after performing merge\n21: 6a2baf0d3e5 = 22: b75376e3725 list-objects-filter-options: work around reported leak on error\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505599","messageId":"89b66411354437a1e3a97751f185889dffb84126.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 01/22] builtin/ls-remote: plug leaking server options","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:00Z","receivedAt":"2024-10-21T09:28:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The server options populated via `OPT_STRING_LIST()` is never cleared,\ncausing a memory leak. Plug it.\n\nThis leak is exposed by t5702, but plugging it alone does not make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/ls-remote.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex f723b3bf3bb..f333821b994 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -166,6 +166,7 @@ int cmd_ls_remote(int argc,\n \t\tstatus = 0; /* we found something */\n \t}\n \n+\tstring_list_clear(&server_options, 0);\n \tref_sorting_release(sorting);\n \tref_array_clear(&ref_array);\n \tif (transport_disconnect(transport))\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505600","messageId":"1c42e194b20637ef84c61b62e29fdd9047b1acfb.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 02/22] t/helper: fix leaks in \"reach\" test tool","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:03Z","receivedAt":"2024-10-21T09:28:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"reach\" test tool doesn't bother to clean up any of its allocated\nresources, causing various leaks. Plug them.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-reach.c | 10 ++++++++++\n t/t6600-test-reach.sh | 1 +\n 2 files changed, 11 insertions(+)\n\ndiff --git a/t/helper/test-reach.c b/t/helper/test-reach.c\nindex 995e382863a..84deee604ad 100644\n--- a/t/helper/test-reach.c\n+++ b/t/helper/test-reach.c\n@@ -127,10 +127,12 @@ int cmd__reach(int ac, const char **av)\n \t\t\texit(128);\n \t\tprintf(\"%s(A,X):\\n\", av[1]);\n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t} else if (!strcmp(av[1], \"reduce_heads\")) {\n \t\tstruct commit_list *list = reduce_heads(X);\n \t\tprintf(\"%s(X):\\n\", av[1]);\n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t} else if (!strcmp(av[1], \"can_all_from_reach\")) {\n \t\tprintf(\"%s(X,Y):%d\\n\", av[1], can_all_from_reach(X, Y, 1));\n \t} else if (!strcmp(av[1], \"can_all_from_reach_with_flag\")) {\n@@ -153,6 +155,7 @@ int cmd__reach(int ac, const char **av)\n \t\t\tfilter.with_commit_tag_algo = 0;\n \n \t\tprintf(\"%s(_,A,X,_):%d\\n\", av[1], commit_contains(&filter, A, X, &cache));\n+\t\tclear_contains_cache(&cache);\n \t} else if (!strcmp(av[1], \"get_reachable_subset\")) {\n \t\tconst int reachable_flag = 1;\n \t\tint i, count = 0;\n@@ -176,7 +179,14 @@ int cmd__reach(int ac, const char **av)\n \t\t\tdie(_(\"too many commits marked reachable\"));\n \n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t}\n \n+\tobject_array_clear(&X_obj);\n+\tstrbuf_release(&buf);\n+\tfree_commit_list(X);\n+\tfree_commit_list(Y);\n+\tfree(X_array);\n+\tfree(Y_array);\n \treturn 0;\n }\ndiff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh\nindex 2591f8b8b39..307deefed2c 100755\n--- a/t/t6600-test-reach.sh\n+++ b/t/t6600-test-reach.sh\n@@ -2,6 +2,7 @@\n \n test_description='basic commit reachability tests'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Construct a grid-like commit graph with points (x,y)\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505601","messageId":"cb4eee37b4024a451a385f94ad57e7278e542de7.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 03/22] grep: fix leak in `grep_splice_or()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:09Z","receivedAt":"2024-10-21T09:28:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `grep_splice_or()` we search for the next `TRUE` node in our tree of\ngrep exrpessions and replace it with the given new expression. But we\ndon't free the old node, which causes a memory leak. Plug it.\n\nThis leak is exposed by t7810, but plugging it alone isn't sufficient to\nmake the test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n grep.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/grep.c b/grep.c\nindex 701e58de04e..e9337f32cbf 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -756,6 +756,7 @@ static struct grep_expr *grep_splice_or(struct grep_expr *x, struct grep_expr *y\n \t\tassert(x->node == GREP_NODE_OR);\n \t\tif (x->u.binary.right &&\n \t\t x->u.binary.right->node == GREP_NODE_TRUE) {\n+\t\t\tfree(x->u.binary.right);\n \t\t\tx->u.binary.right = y;\n \t\t\tbreak;\n \t\t}\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505602","messageId":"6b2c8842ef53a3e550aa3de7447c9c3d23bc6772.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 04/22] builtin/grep: fix leak with `--max-count=0`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:12Z","receivedAt":"2024-10-21T09:28:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When executing with `--max-count=0` we'll return early from git-grep(1)\nwithout performing any cleanup, which causes memory leaks. Plug these.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/grep.c | 13 ++++++++++---\n t/t7810-grep.sh | 1 +\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex f17d46a06e4..98b85c7fcac 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -906,6 +906,7 @@ int cmd_grep(int argc,\n \tint dummy;\n \tint use_index = 1;\n \tint allow_revs;\n+\tint ret;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"cached\", &cached,\n@@ -1172,8 +1173,10 @@ int cmd_grep(int argc,\n \t * Optimize out the case where the amount of matches is limited to zero.\n \t * We do this to keep results consistent with GNU grep(1).\n \t */\n-\tif (opt.max_count == 0)\n-\t\treturn 1;\n+\tif (opt.max_count == 0) {\n+\t\tret = 1;\n+\t\tgoto out;\n+\t}\n \n \tif (show_in_pager) {\n \t\tif (num_threads > 1)\n@@ -1267,10 +1270,14 @@ int cmd_grep(int argc,\n \t\thit |= wait_all();\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\n+\n+\tret = !hit;\n+\n+out:\n \tclear_pathspec(&pathspec);\n \tstring_list_clear(&path_list, 0);\n \tfree_grep_patterns(&opt);\n \tobject_array_clear(&list);\n \tfree_repos();\n-\treturn !hit;\n+\treturn ret;\n }\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex af2cf2f78ab..9e7681f0831 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -9,6 +9,7 @@ test_description='git grep various.\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_invalid_grep_expression() {\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505603","messageId":"7527b31a28f4c9d2e689c9749b6f9f0615ba1261.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 05/22] revision: fix leaking bloom filters","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:15Z","receivedAt":"2024-10-21T09:28:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The memory allocated by `prepare_to_use_bloom_filter()` is not released\nby `release_revisions()`, causing a memory leak. Plug it.\n\nSigned-off-by: Patrick Steinhardt \n---\n revision.c | 5 +++++\n t/t4216-log-bloom.sh | 1 +\n 2 files changed, 6 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex f5f5b84f2b0..8df75b82249 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3227,6 +3227,11 @@ void release_revisions(struct rev_info *revs)\n \tclear_decoration(&revs->treesame, free);\n \tline_log_free(revs);\n \toidset_clear(&revs->missing_commits);\n+\n+\tfor (int i = 0; i < revs->bloom_keys_nr; i++)\n+\t\tclear_bloom_key(&revs->bloom_keys[i]);\n+\tFREE_AND_NULL(revs->bloom_keys);\n+\trevs->bloom_keys_nr = 0;\n }\n \n static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)\ndiff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh\nindex 3f163dc3969..8d22338f6aa 100755\n--- a/t/t4216-log-bloom.sh\n+++ b/t/t4216-log-bloom.sh\n@@ -4,6 +4,7 @@ test_description='git log for a path with Bloom filters'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-chunk.sh\n \n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505604","messageId":"60af98cb2c7752edc7cd5c5fe8173dc5b2522a7b.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 06/22] diff-lib: fix leaking diffopts in `do_diff_cache()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:18Z","receivedAt":"2024-10-21T09:28:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `do_diff_cache()` we initialize a new `rev_info` and then overwrite\nits `diffopt` with a user-provided set of options. This can leak memory\nbecause `repo_init_revisions()` may end up allocating memory for the\n`diffopt` itself depending on the configuration. And as that field is\noverwritten we won't ever free that.\n\nPlug the memory leak by releasing the diffopts before we overwrite them.\n\nSigned-off-by: Patrick Steinhardt \n---\n diff-lib.c | 1 +\n t/t7610-mergetool.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 6b14b959629..3cf353946f5 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -661,6 +661,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)\n \n \trepo_init_revisions(opt->repo, &revs, NULL);\n \tcopy_pathspec(&revs.prune_data, &opt->pathspec);\n+\tdiff_free(&revs.diffopt);\n \trevs.diffopt = *opt;\n \trevs.diffopt.no_free = 1;\n \ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 22b3a85b3e9..5c5e79e9905 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -10,6 +10,7 @@ Testing basic merge tool invocation'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # All the mergetool test work by checking out a temporary branch based\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505605","messageId":"5d5f6867f918460001f62aaa78f24cf3cbe53a3c.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 07/22] pretty: clear signature check","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:20Z","receivedAt":"2024-10-21T09:28:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The signature check in of the formatting context is never getting\nreleased. Fix this to plug the resulting memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n pretty.c | 1 +\n t/t4202-log.sh | 1 +\n t/t7031-verify-tag-signed-ssh.sh | 1 +\n t/t7510-signed-commit.sh | 1 +\n t/t7528-signed-commit-ssh.sh | 1 +\n 5 files changed, 5 insertions(+)\n\ndiff --git a/pretty.c b/pretty.c\nindex 6403e268900..098378720a4 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -2032,6 +2032,7 @@ void repo_format_commit_message(struct repository *r,\n \n \tfree(context.commit_encoding);\n \trepo_unuse_commit_buffer(r, commit, context.message);\n+\tsignature_check_clear(&context.signature_check);\n }\n \n static void pp_header(struct pretty_print_context *pp,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 51f7beb59f8..35bec4089a3 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -5,6 +5,7 @@ test_description='git log'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n . \"$TEST_DIRECTORY/lib-terminal.sh\"\ndiff --git a/t/t7031-verify-tag-signed-ssh.sh b/t/t7031-verify-tag-signed-ssh.sh\nindex 20913b37134..2ee62c07293 100755\n--- a/t/t7031-verify-tag-signed-ssh.sh\n+++ b/t/t7031-verify-tag-signed-ssh.sh\n@@ -4,6 +4,7 @@ test_description='signed tag tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 0d2dd29fe6a..eb229082e40 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -4,6 +4,7 @@ test_description='signed commit tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex 065f7806362..68e18856b66 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -4,6 +4,7 @@ test_description='ssh signed commit tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505606","messageId":"0d503e40194c5d1912c7dd8a23f685546c490027.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 08/22] upload-pack: fix leaking URI protocols","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:23Z","receivedAt":"2024-10-21T09:28:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We don't clear `struct upload_pack::uri_protocols`, which causes a\nmemory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5702-protocol-v2.sh | 1 +\n upload-pack.c | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1ef540f73d3..ed55a2f7f95 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Test protocol v2 with 'git://' transport\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 6d6e0f9f980..b4a59c3518b 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -166,6 +166,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)\n \tobject_array_clear(&data->extra_edge_obj);\n \tlist_objects_filter_release(&data->filter_options);\n \tstring_list_clear(&data->allowed_filters, 0);\n+\tstring_list_clear(&data->uri_protocols, 0);\n \n \tfree((char *)data->pack_objects_hook);\n }\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505607","messageId":"9f967dfe5d55ca7150bf3e118279388290f7d28c.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 09/22] builtin/commit: fix leaking change data contents","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:29Z","receivedAt":"2024-10-21T09:28:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While we free the worktree change data, we never free its contents. Fix\nthis.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/commit.c | 9 ++++++++-\n t/t7500-commit-template-squash-signoff.sh | 1 +\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8db4e9df0c9..18a55bd1b91 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -728,6 +728,13 @@ static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n \trepo_unuse_commit_buffer(the_repository, commit, buffer);\n }\n \n+static void change_data_free(void *util, const char *str UNUSED)\n+{\n+\tstruct wt_status_change_data *d = util;\n+\tfree(d->rename_source);\n+\tfree(d);\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t struct commit *current_head,\n \t\t\t struct wt_status *s,\n@@ -991,7 +998,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\ts->use_color = 0;\n \t\tcommittable = run_status(s->fp, index_file, prefix, 1, s);\n \t\ts->use_color = saved_color_setting;\n-\t\tstring_list_clear(&s->change, 1);\n+\t\tstring_list_clear_func(&s->change, change_data_free);\n \t} else {\n \t\tstruct object_id oid;\n \t\tconst char *parent = \"HEAD\";\ndiff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\nindex 4dca8d97a77..379d3ed3413 100755\n--- a/t/t7500-commit-template-squash-signoff.sh\n+++ b/t/t7500-commit-template-squash-signoff.sh\n@@ -7,6 +7,7 @@ test_description='git commit\n \n Tests for template, signoff, squash and -F functions.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/lib-rebase.sh\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505608","messageId":"e3ffd59123f23f53c0bee930ef7602acf2d800c2.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 10/22] trailer: fix leaking trailer values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:32Z","receivedAt":"2024-10-21T09:28:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Fix leaking trailer values when replacing the value with a command or\nwhen the token value is empty.\n\nThis leak is exposed by t7513, but plugging it does not make the whole\ntest suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n trailer.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/trailer.c b/trailer.c\nindex 682d74505bf..f1eca6d5d15 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -249,17 +249,23 @@ static char *apply_command(struct conf_info *conf, const char *arg)\n static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)\n {\n \tif (arg_tok->conf.command || arg_tok->conf.cmd) {\n-\t\tconst char *arg;\n+\t\tchar *value_to_free = NULL;\n+\t\tchar *arg;\n+\n \t\tif (arg_tok->value && arg_tok->value[0]) {\n-\t\t\targ = arg_tok->value;\n+\t\t\targ = (char *)arg_tok->value;\n \t\t} else {\n \t\t\tif (in_tok && in_tok->value)\n \t\t\t\targ = xstrdup(in_tok->value);\n \t\t\telse\n \t\t\t\targ = xstrdup(\"\");\n+\t\t\tvalue_to_free = arg_tok->value;\n \t\t}\n+\n \t\targ_tok->value = apply_command(&arg_tok->conf, arg);\n-\t\tfree((char *)arg);\n+\n+\t\tfree(value_to_free);\n+\t\tfree(arg);\n \t}\n }\n \n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505609","messageId":"5b851453bcea945f95c3f29138e510d8448e96e6.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 11/22] trailer: fix leaking strbufs when formatting trailers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:35Z","receivedAt":"2024-10-21T09:28:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We are populating, but never releasing two string buffers in\n`format_trailers()`, causing a memory leak. Plug this leak by lifting\nthose buffers outside of the loop and releasing them on function return.\nThis fixes the memory leaks, but also optimizes the loop as we don't\nhave to reallocate the buffers on every single iteration.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t7513-interpret-trailers.sh | 1 +\n trailer.c | 13 ++++++++-----\n 2 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 0f7d8938d98..38d6ccaa001 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -5,6 +5,7 @@\n \n test_description='git interpret-trailers'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # When we want one trailing space at the end of each line, let's use sed\ndiff --git a/trailer.c b/trailer.c\nindex f1eca6d5d15..24e4e56fdf8 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1111,16 +1111,19 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t struct list_head *trailers,\n \t\t struct strbuf *out)\n {\n+\tstruct strbuf tok = STRBUF_INIT;\n+\tstruct strbuf val = STRBUF_INIT;\n \tsize_t origlen = out->len;\n \tstruct list_head *pos;\n \tstruct trailer_item *item;\n \n+\n \tlist_for_each(pos, trailers) {\n \t\titem = list_entry(pos, struct trailer_item, list);\n \t\tif (item->token) {\n-\t\t\tstruct strbuf tok = STRBUF_INIT;\n-\t\t\tstruct strbuf val = STRBUF_INIT;\n+\t\t\tstrbuf_reset(&tok);\n \t\t\tstrbuf_addstr(&tok, item->token);\n+\t\t\tstrbuf_reset(&val);\n \t\t\tstrbuf_addstr(&val, item->value);\n \n \t\t\t/*\n@@ -1151,9 +1154,6 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t\t\tif (!opts->separator)\n \t\t\t\t\tstrbuf_addch(out, '\\n');\n \t\t\t}\n-\t\t\tstrbuf_release(&tok);\n-\t\t\tstrbuf_release(&val);\n-\n \t\t} else if (!opts->only_trailers) {\n \t\t\tif (opts->separator && out->len != origlen) {\n \t\t\t\tstrbuf_addbuf(out, opts->separator);\n@@ -1165,6 +1165,9 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t\t\tstrbuf_addch(out, '\\n');\n \t\t}\n \t}\n+\n+\tstrbuf_release(&tok);\n+\tstrbuf_release(&val);\n }\n \n void format_trailers_from_commit(const struct process_trailer_options *opts,\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505610","messageId":"60c3f6146f3d254b80e3b3c8ef3c248b4f9af7f8.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 12/22] builtin/commit: fix leaking cleanup config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:38Z","receivedAt":"2024-10-21T09:28:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The cleanup string set by the config is leaking when it is being\noverridden by an option. Fix this by tracking these via two separate\nvariables such that we can free the old value.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/commit.c | 17 ++++++++++++-----\n t/t7502-commit-porcelain.sh | 1 +\n 2 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 18a55bd1b91..71d674138c9 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -135,7 +135,7 @@ static struct strvec trailer_args = STRVEC_INIT;\n * is specified explicitly.\n */\n static enum commit_msg_cleanup_mode cleanup_mode;\n-static char *cleanup_arg;\n+static char *cleanup_config;\n \n static enum commit_whence whence;\n static int use_editor = 1, include_status = 1;\n@@ -1387,8 +1387,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (0 <= edit_flag)\n \t\tuse_editor = edit_flag;\n \n-\tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n-\n \thandle_untracked_files_arg(s);\n \n \tif (all && argc > 0)\n@@ -1636,8 +1634,10 @@ static int git_commit_config(const char *k, const char *v,\n \t\tinclude_status = git_config_bool(k, v);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(k, \"commit.cleanup\"))\n-\t\treturn git_config_string(&cleanup_arg, k, v);\n+\tif (!strcmp(k, \"commit.cleanup\")) {\n+\t\tFREE_AND_NULL(cleanup_config);\n+\t\treturn git_config_string(&cleanup_config, k, v);\n+\t}\n \tif (!strcmp(k, \"commit.gpgsign\")) {\n \t\tsign_commit = git_config_bool(k, v) ? \"\" : NULL;\n \t\treturn 0;\n@@ -1658,6 +1658,7 @@ int cmd_commit(int argc,\n \t struct repository *repo UNUSED)\n {\n \tstatic struct wt_status s;\n+\tstatic const char *cleanup_arg = NULL;\n \tstatic struct option builtin_commit_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress summary after successful commit\")),\n \t\tOPT__VERBOSE(&verbose, N_(\"show diff in commit message template\")),\n@@ -1757,6 +1758,12 @@ int cmd_commit(int argc,\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n+\tif (cleanup_arg) {\n+\t\tfree(cleanup_config);\n+\t\tcleanup_config = xstrdup(cleanup_arg);\n+\t}\n+\tcleanup_mode = get_cleanup_mode(cleanup_config, use_editor);\n+\n \tif (dry_run)\n \t\treturn dry_run_commit(argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argv, prefix, current_head, 0);\ndiff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\nindex b37e2018a74..84f1ff52b67 100755\n--- a/t/t7502-commit-porcelain.sh\n+++ b/t/t7502-commit-porcelain.sh\n@@ -5,6 +5,7 @@ test_description='git commit porcelain-ish'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n commit_msg_is () {\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505611","messageId":"ea81cd8db863c4421411442dd68be1d2ed1dccb5.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 13/22] transport-helper: fix leaking import/export marks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:40Z","receivedAt":"2024-10-21T09:28:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Fix leaking import and export marks for transport helpers.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5801-remote-helpers.sh | 1 +\n transport-helper.c | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex d21877150ed..d4882288a30 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -8,6 +8,7 @@ test_description='Test remote-helper import and export commands'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex 013ec79dc9c..bc27653cdee 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -399,6 +399,8 @@ static int release_helper(struct transport *transport)\n \tint res = 0;\n \tstruct helper_data *data = transport->data;\n \trefspec_clear(&data->rs);\n+\tfree(data->import_marks);\n+\tfree(data->export_marks);\n \tres = disconnect_helper(transport);\n \tfree(transport->data);\n \treturn res;\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505612","messageId":"b700ab9079ff0f84269dfbb5e6b6aa7af67bd903.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 14/22] builtin/tag: fix leaking key ID on failure to sign","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:43Z","receivedAt":"2024-10-21T09:28:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We do not free the key ID when signing a tag fails. Do so by using\nthe common exit path.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/tag.c | 2 +-\n t/t7004-tag.sh | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 93d10d59157..c37c0a68fda 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -164,7 +164,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,\n \tint ret = -1;\n \n \tif (sign_buffer(buffer, &sig, keyid))\n-\t\treturn -1;\n+\t\tgoto out;\n \n \tif (compat) {\n \t\tconst struct git_hash_algo *algo = the_repository->hash_algo;\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b1316e62f46..42b3327e69b 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -10,6 +10,7 @@ Tests for operations with tags.'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505613","messageId":"76bbcb3fe301fe273578a71849f99953ea94695c.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 15/22] combine-diff: fix leaking lost lines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:48Z","receivedAt":"2024-10-21T09:28:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `cnt` variable tracks the number of lines in a patch diff. It can\nhappen though that there are no newlines, in which case we'd still end\nup allocating our array of `sline`s. In fact, we always allocate it with\n`cnt + 2` entries. But when we loop through the array to clear it at the\nend of this function we only loop until `lno < cnt`, and thus we may not\nend up releasing whatever the two extra `sline`s contain.\n\nPlug this memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n combine-diff.c | 2 +-\n t/t4038-diff-combined.sh | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex f6b624dc288..3c6d9507fec 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1220,7 +1220,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t}\n \tfree(result);\n \n-\tfor (lno = 0; lno < cnt; lno++) {\n+\tfor (lno = 0; lno < cnt + 2; lno++) {\n \t\tif (sline[lno].lost) {\n \t\t\tstruct lline *ll = sline[lno].lost;\n \t\t\twhile (ll) {\ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex 2ce26e585c9..00190802d83 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -5,6 +5,7 @@ test_description='combined diff'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-diff.sh\n \n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505614","messageId":"d6b96a4012dca23cae73ce680759783cc49868b3.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 16/22] dir: release untracked cache data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:50Z","receivedAt":"2024-10-21T09:28:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several cases where we invalidate untracked cache directory\nentries where we do not free the underlying data, but reset the number\nof entries. This causes us to leak memory because `free_untracked()`\nwill not iterate over any potential entries which we still had in the\narray.\n\nFix this issue by freeing old entries. The leak is exposed by t7519, but\nplugging it alone does not make the whole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n dir.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex e3ddd5b5296..cb9782fa11f 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1056,6 +1056,8 @@ static void do_invalidate_gitignore(struct untracked_cache_dir *dir)\n {\n \tint i;\n \tdir->valid = 0;\n+\tfor (size_t i = 0; i < dir->untracked_nr; i++)\n+\t\tfree(dir->untracked[i]);\n \tdir->untracked_nr = 0;\n \tfor (i = 0; i < dir->dirs_nr; i++)\n \t\tdo_invalidate_gitignore(dir->dirs[i]);\n@@ -1083,6 +1085,8 @@ static void invalidate_directory(struct untracked_cache *uc,\n \t\tuc->dir_invalidated++;\n \n \tdir->valid = 0;\n+\tfor (size_t i = 0; i < dir->untracked_nr; i++)\n+\t\tfree(dir->untracked[i]);\n \tdir->untracked_nr = 0;\n \tfor (i = 0; i < dir->dirs_nr; i++)\n \t\tdir->dirs[i]->recurse = 0;\n@@ -3573,6 +3577,8 @@ static void write_one_dir(struct untracked_cache_dir *untracked,\n \t * for safety..\n \t */\n \tif (!untracked->valid) {\n+\t\tfor (size_t i = 0; i < untracked->untracked_nr; i++)\n+\t\t\tfree(untracked->untracked[i]);\n \t\tuntracked->untracked_nr = 0;\n \t\tuntracked->check_only = 0;\n \t}\n@@ -3905,6 +3911,8 @@ static void invalidate_one_directory(struct untracked_cache *uc,\n {\n \tuc->dir_invalidated++;\n \tucd->valid = 0;\n+\tfor (size_t i = 0; i < ucd->untracked_nr; i++)\n+\t\tfree(ucd->untracked[i]);\n \tucd->untracked_nr = 0;\n }\n \n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505615","messageId":"3aa6bac50793b2453e5933a5125bb8e146e3f569.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 17/22] sparse-index: correctly free EWAH contents","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:53Z","receivedAt":"2024-10-21T09:28:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While we free the `fsmonitor_dirty` member of `struct index_state`, we\ndo not free the contents of that EWAH. Do so by using `ewah_free()`\ninstead of `FREE_AND_NULL()`.\n\nThis leak is exposed by t7519, but plugging it alone does not make the\ntest suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n sparse-index.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 3d7f2164e25..2107840bfc5 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -2,6 +2,7 @@\n \n #include \"git-compat-util.h\"\n #include \"environment.h\"\n+#include \"ewah/ewok.h\"\n #include \"gettext.h\"\n #include \"name-hash.h\"\n #include \"read-cache-ll.h\"\n@@ -242,7 +243,8 @@ int convert_to_sparse(struct index_state *istate, int flags)\n \tcache_tree_update(istate, 0);\n \n \tistate->fsmonitor_has_run_once = 0;\n-\tFREE_AND_NULL(istate->fsmonitor_dirty);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n \tFREE_AND_NULL(istate->fsmonitor_last_update);\n \n \tistate->sparse_index = INDEX_COLLAPSED;\n@@ -438,7 +440,8 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \tistate->cache_nr = full->cache_nr;\n \tistate->cache_alloc = full->cache_alloc;\n \tistate->fsmonitor_has_run_once = 0;\n-\tFREE_AND_NULL(istate->fsmonitor_dirty);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n \tFREE_AND_NULL(istate->fsmonitor_last_update);\n \n \tstrbuf_release(&base);\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505616","messageId":"132fe75090656ee05198512b95acfa6eaa33f3e2.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 18/22] t/helper: stop re-initialization of `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:56Z","receivedAt":"2024-10-21T09:29:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While \"common-main.c\" already initializes `the_repository` for us, we do\nso a second time in the \"read-cache\" test helper. This causes a memory\nleak because the old repository's contents isn't released.\n\nStop calling `initialize_repository()` to plug this leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-read-cache.c | 2 --\n t/t7519-status-fsmonitor.sh | 1 +\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex d285c656bd3..e277dde8e71 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -11,8 +11,6 @@ int cmd__read_cache(int argc, const char **argv)\n \tint i, cnt = 1;\n \tconst char *name = NULL;\n \n-\tinitialize_repository(the_repository);\n-\n \tif (argc > 1 && skip_prefix(argv[1], \"--print-and-refresh=\", &name)) {\n \t\targc--;\n \t\targv++;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 7ee69ecdd4a..0f88a58a819 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -2,6 +2,7 @@\n \n test_description='git status with file system watcher'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Note, after \"git reset --hard HEAD\" no extensions exist other than 'TREE'\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505617","messageId":"b8b702eeb28b384bc359c77964509fa105645eec.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 19/22] t/helper: fix leaking buffer in \"dump-untracked-cache\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:28:58Z","receivedAt":"2024-10-21T09:29:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never release the local `struct strbuf base` buffer, thus leaking\nmemory. Fix this leak.\n\nThis leak is exposed by t7063, but plugging it alone does not make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-dump-untracked-cache.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/helper/test-dump-untracked-cache.c b/t/helper/test-dump-untracked-cache.c\nindex 4f010d53249..b2e70837a90 100644\n--- a/t/helper/test-dump-untracked-cache.c\n+++ b/t/helper/test-dump-untracked-cache.c\n@@ -68,5 +68,7 @@ int cmd__dump_untracked_cache(int ac UNUSED, const char **av UNUSED)\n \tprintf(\"flags %08x\\n\", uc->dir_flags);\n \tif (uc->root)\n \t\tdump(uc->root, &base);\n+\n+\tstrbuf_release(&base);\n \treturn 0;\n }\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505618","messageId":"ad309ac1b372f72c775390adaf0706df7d39b320.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 20/22] dir: fix leak when parsing \"status.showUntrackedFiles\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:29:01Z","receivedAt":"2024-10-21T09:29:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We use `repo_config_get_string()` to read \"status.showUntrackedFiles\"\nfrom the config subsystem. This function allocates the result, but we\nnever free the result after parsing it.\n\nThe value never leaves the scope of the calling function, so refactor it\nto instead use `repo_config_get_string_tmp()`, which does not hand over\nownership to the caller.\n\nSigned-off-by: Patrick Steinhardt \n---\n dir.c | 4 ++--\n t/t7063-status-untracked-cache.sh | 1 +\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex cb9782fa11f..7f35a3e3175 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2872,14 +2872,14 @@ static void set_untracked_ident(struct untracked_cache *uc)\n static unsigned new_untracked_cache_flags(struct index_state *istate)\n {\n \tstruct repository *repo = istate->repo;\n-\tchar *val;\n+\tconst char *val;\n \n \t/*\n \t * This logic is coordinated with the setting of these flags in\n \t * wt-status.c#wt_status_collect_untracked(), and the evaluation\n \t * of the config setting in commit.c#git_status_config()\n \t */\n-\tif (!repo_config_get_string(repo, \"status.showuntrackedfiles\", &val) &&\n+\tif (!repo_config_get_string_tmp(repo, \"status.showuntrackedfiles\", &val) &&\n \t !strcmp(val, \"all\"))\n \t\treturn 0;\n \ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex 8929ef481f9..13fea7931cd 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -5,6 +5,7 @@ test_description='test untracked cache'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # On some filesystems (e.g. FreeBSD's ext2 and ufs) directory mtime\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505619","messageId":"5e243f9ee530da417beda9230f65ccf18ae21a9e.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 21/22] builtin/merge: release outbut buffer after performing merge","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:29:07Z","receivedAt":"2024-10-21T09:29:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `obuf` member of `struct merge_options` is used to buffer output in\nsome cases. In order to not discard its allocated memory we only release\nits contents in `merge_finalize()` when we're not currently recursing\ninto a subtree.\n\nThis results in some situations where we seemingly do not release the\nbuffer reliably. We thus have calls to `strbuf_release()` for this\nbuffer scattered across the codebase. But we're missing one callsite in\ngit-merge(1), which causes a memory leak.\n\nWe should ideally refactor this interface so that callers don't have to\nknow about any such internals. But for now, paper over the issue by\nadding one more `strbuf_release()` call.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/merge.c | 1 +\n t/t6424-merge-unrelated-index-changes.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 84d0f3604bc..51038eaca84 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -754,6 +754,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\t\tclean = merge_recursive(&o, head, remoteheads->item,\n \t\t\t\t\t\treversed, &result);\n \t\tfree_commit_list(reversed);\n+\t\tstrbuf_release(&o.obuf);\n \n \t\tif (clean < 0) {\n \t\t\trollback_lock_file(&lock);\ndiff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh\nindex 7677c5f08d0..a7ea8acb845 100755\n--- a/t/t6424-merge-unrelated-index-changes.sh\n+++ b/t/t6424-merge-unrelated-index-changes.sh\n@@ -2,6 +2,7 @@\n \n test_description=\"merges with unrelated index changes\"\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Testcase for some simple merges\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505620","messageId":"b75376e3725bb22d0253792f586671975fa44cc0.1729502824.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"[PATCH v2 22/22] list-objects-filter-options: work around reported leak on error","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T09:29:09Z","receivedAt":"2024-10-21T09:29:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"This one is a little bit more curious. In t6112, we have a test that\nexercises the `git rev-list --filter` option with invalid filters. We\nexecute git-rev-list(1) via `test_must_fail`, which means that we check\nfor leaks even though Git exits with an error code. This causes the\nfollowing leak:\n\n Direct leak of 27 byte(s) in 1 object(s) allocated from:\n #0 0x5555555e6946 in realloc.part.0 lsan_interceptors.cpp.o\n #1 0x5555558fb4b6 in xrealloc wrapper.c:137:8\n #2 0x5555558b6e06 in strbuf_grow strbuf.c:112:2\n #3 0x5555558b7550 in strbuf_add strbuf.c:311:2\n #4 0x5555557c1a88 in strbuf_addstr strbuf.h:310:2\n #5 0x5555557c1d4c in parse_list_objects_filter list-objects-filter-options.c:261:3\n #6 0x555555885ead in handle_revision_pseudo_opt revision.c:2899:3\n #7 0x555555884e20 in setup_revisions revision.c:3014:11\n #8 0x5555556c4b42 in cmd_rev_list builtin/rev-list.c:588:9\n #9 0x5555555ec5e3 in run_builtin git.c:483:11\n #10 0x5555555eb1e4 in handle_builtin git.c:749:13\n #11 0x5555555ec001 in run_argv git.c:819:4\n #12 0x5555555eaf94 in cmd_main git.c:954:19\n #13 0x5555556fd569 in main common-main.c:64:11\n #14 0x7ffff7ca714d in __libc_start_call_main (.../lib/libc.so.6+0x2a14d)\n #15 0x7ffff7ca7208 in __libc_start_main@GLIBC_2.2.5 (.../libc.so.6+0x2a208)\n #16 0x5555555ad064 in _start (git+0x59064)\n\nThis leak is valid, as we call `die()` and do not clean up the memory at\nall. But what's curious is that this is the only leak reported, because\nwe don't clean up any other allocated memory, either, and I have no idea\nwhy the leak sanitizer treats this buffer specially.\n\nIn any case, we can work around the leak by shuffling things around a\nbit. Instead of calling `gently_parse_list_objects_filter()` and dying\nafter we have modified the filter spec, we simply do so beforehand. Like\nthis we don't allocate the buffer in the error case, which makes the\nreported leak go away.\n\nIt's not pretty, but it manages to make t6112 leak free.\n\nSigned-off-by: Patrick Steinhardt \n---\n list-objects-filter-options.c | 17 +++++++----------\n t/t6112-rev-list-filters-objects.sh | 1 +\n 2 files changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 00611107d20..fa72e81e4ad 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -252,16 +252,14 @@ void parse_list_objects_filter(\n \tconst char *arg)\n {\n \tstruct strbuf errbuf = STRBUF_INIT;\n-\tint parse_error;\n \n \tif (!filter_options->filter_spec.buf)\n \t\tBUG(\"filter_options not properly initialized\");\n \n \tif (!filter_options->choice) {\n+\t\tif (gently_parse_list_objects_filter(filter_options, arg, &errbuf))\n+\t\t\tdie(\"%s\", errbuf.buf);\n \t\tstrbuf_addstr(&filter_options->filter_spec, arg);\n-\n-\t\tparse_error = gently_parse_list_objects_filter(\n-\t\t\tfilter_options, arg, &errbuf);\n \t} else {\n \t\tstruct list_objects_filter_options *sub;\n \n@@ -271,18 +269,17 @@ void parse_list_objects_filter(\n \t\t */\n \t\ttransform_to_combine_type(filter_options);\n \n-\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n-\t\tfilter_spec_append_urlencode(filter_options, arg);\n \t\tALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,\n \t\t\t filter_options->sub_alloc);\n \t\tsub = &filter_options->sub[filter_options->sub_nr - 1];\n \n \t\tlist_objects_filter_init(sub);\n-\t\tparse_error = gently_parse_list_objects_filter(sub, arg,\n-\t\t\t\t\t\t\t &errbuf);\n+\t\tif (gently_parse_list_objects_filter(sub, arg, &errbuf))\n+\t\t\tdie(\"%s\", errbuf.buf);\n+\n+\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n+\t\tfilter_spec_append_urlencode(filter_options, arg);\n \t}\n-\tif (parse_error)\n-\t\tdie(\"%s\", errbuf.buf);\n }\n \n int opt_parse_list_objects_filter(const struct option *opt,\ndiff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\nindex 0387f35a326..71e38491fa8 100755\n--- a/t/t6112-rev-list-filters-objects.sh\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -5,6 +5,7 @@ test_description='git rev-list using object filtering'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Test the blob:none filter.\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505622","messageId":"8beb4948-7a6b-4c32-b768-d3ecf04407e2@app.fastmail.com","threadId":"62312","inReplyTo":"5e243f9ee530da417beda9230f65ccf18ae21a9e.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 21/22] builtin/merge: release outbut buffer after performing merge","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-10-21T09:41:04Z","receivedAt":"2024-10-21T09:41:25Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"> Re: [PATCH v2 21/22] builtin/merge: release outbut buffer after performing merge\n\ns/outbut/output/\n"},{"id":"505623","messageId":"fbbcea49-82aa-4cd6-b288-88b9e95530de@app.fastmail.com","threadId":"62312","inReplyTo":"cb4eee37b4024a451a385f94ad57e7278e542de7.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 03/22] grep: fix leak in `grep_splice_or()`","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-10-21T09:42:10Z","receivedAt":"2024-10-21T09:42:32Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Oct 21, 2024, at 11:28, Patrick Steinhardt wrote:\n> In `grep_splice_or()` we search for the next `TRUE` node in our tree of\n> grep exrpessions and replace it with the given new expression. But we\n\ns/exrpessions/expressions/\n"},{"id":"505624","messageId":"015d0995-c7db-45c9-9ad5-d4ea75d8f792@app.fastmail.com","threadId":"62312","inReplyTo":"60af98cb2c7752edc7cd5c5fe8173dc5b2522a7b.1728624670.git.ps@pks.im","subject":"Re: [PATCH 06/21] diff-lib: fix leaking diffopts in `do_diff_cache()`","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-10-21T09:46:23Z","receivedAt":"2024-10-21T09:46:45Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Fri, Oct 11, 2024, at 07:32, Patrick Steinhardt wrote:\n> In `do_diff_cache()` we initialize a new `rev_info` and then overwrite\n> its `diffopt` with a user-provided set of options. This can leak memory\n> because `repo_init_revisions()` may end up allocating memory for the\n> `diffopt` itself depending on the configuration. And as that field is\n\ns/as that/since that/\n\n“since” communicates causality better.\n\n> overwritten we won't ever free that.\n\ns/free that/free it/\n\n-- \nKristoffer Haugsbakk\n"},{"id":"505626","messageId":"c1d3d038-4d5d-4904-b2c1-47b98d20a2df@app.fastmail.com","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"Re: [PATCH v2 00/22] Memory leak fixes (pt.9)","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-10-21T09:54:52Z","receivedAt":"2024-10-21T09:55:14Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Oct 21, 2024, at 11:27, Patrick Steinhardt wrote:\n> Hi,\n>\n> this is the second version of my 9th series of memory leak fixes.\n\nI can’t even imagine the amount of effort it takes to plug all these\nleaks in parallel with the stuff that everyone else is doing in the\ncode. Nice work. :)\n\nOf course all the comments that I left just now are nitpicks.\n\n-- \nKristoffer Haugsbakk\n"},{"id":"505649","messageId":"ZxYuo2ISSGRI8_J5@pks.im","threadId":"62312","inReplyTo":"c1d3d038-4d5d-4904-b2c1-47b98d20a2df@app.fastmail.com","subject":"Re: [PATCH v2 00/22] Memory leak fixes (pt.9)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T10:36:25Z","receivedAt":"2024-10-21T10:36:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 21, 2024 at 11:54:52AM +0200, Kristoffer Haugsbakk wrote:\n> On Mon, Oct 21, 2024, at 11:27, Patrick Steinhardt wrote:\n> > Hi,\n> >\n> > this is the second version of my 9th series of memory leak fixes.\n> \n> I can’t even imagine the amount of effort it takes to plug all these\n> leaks in parallel with the stuff that everyone else is doing in the\n> code. Nice work. :)\n> \n> Of course all the comments that I left just now are nitpicks.\n\nThanks! I've queued all fixes locally, but will wait a bit before\nsending them out so that other reviewers have a chance to chime in\nfirst.\n\nPatrick\n"},{"id":"505761","messageId":"ZxbAhyilmPR7jF39@nand.local","threadId":"62312","inReplyTo":"5b851453bcea945f95c3f29138e510d8448e96e6.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 11/22] trailer: fix leaking strbufs when formatting trailers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-21T20:58:47Z","receivedAt":"2024-10-21T20:58:50Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 21, 2024 at 11:28:35AM +0200, Patrick Steinhardt wrote:\n> We are populating, but never releasing two string buffers in\n> `format_trailers()`, causing a memory leak. Plug this leak by lifting\n> those buffers outside of the loop and releasing them on function return.\n> This fixes the memory leaks, but also optimizes the loop as we don't\n> have to reallocate the buffers on every single iteration.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> t/t7513-interpret-trailers.sh | 1 +\n> trailer.c | 13 ++++++++-----\n> 2 files changed, 9 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\n> index 0f7d8938d98..38d6ccaa001 100755\n> --- a/t/t7513-interpret-trailers.sh\n> +++ b/t/t7513-interpret-trailers.sh\n> @@ -5,6 +5,7 @@\n>\n> test_description='git interpret-trailers'\n>\n> +TEST_PASSES_SANITIZE_LEAK=true\n> . ./test-lib.sh\n>\n> # When we want one trailing space at the end of each line, let's use sed\n> diff --git a/trailer.c b/trailer.c\n> index f1eca6d5d15..24e4e56fdf8 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -1111,16 +1111,19 @@ void format_trailers(const struct process_trailer_options *opts,\n> \t\t struct list_head *trailers,\n> \t\t struct strbuf *out)\n> {\n> +\tstruct strbuf tok = STRBUF_INIT;\n> +\tstruct strbuf val = STRBUF_INIT;\n> \tsize_t origlen = out->len;\n> \tstruct list_head *pos;\n> \tstruct trailer_item *item;\n>\n> +\n> \tlist_for_each(pos, trailers) {\n> \t\titem = list_entry(pos, struct trailer_item, list);\n> \t\tif (item->token) {\n> -\t\t\tstruct strbuf tok = STRBUF_INIT;\n> -\t\t\tstruct strbuf val = STRBUF_INIT;\n> +\t\t\tstrbuf_reset(&tok);\n> \t\t\tstrbuf_addstr(&tok, item->token);\n> +\t\t\tstrbuf_reset(&val);\n> \t\t\tstrbuf_addstr(&val, item->value);\n\nI have a vague preference towards writing this as:\n\n strbuf_reset(&tok);\n strbuf_reset(&val);\n strbuf_addstr(&tok, item->token);\n strbuf_addstr(&val, item->value);\n\nto make clear(er) to the reader that, OK, we are resetting both of these\nbuffers before using them at the beginning of the loop. To me it reads a\nlittle clearer seeing both strbuf_reset() calls one right after the\nother.\n\nI don't feel all that strongly about it, but I also see that you have a\ncouple of small changes you're sitting on from Kristoffer's review, so I\nfigured I'd throw it out there anyway as we are expecting a new round to\naddress those.\n\nThanks,\nTaylor\n"},{"id":"506094","messageId":"87plnobp0k.fsf@iotcl.com","threadId":"62312","inReplyTo":"ZxYuo2ISSGRI8_J5@pks.im","subject":"Re: [PATCH v2 00/22] Memory leak fixes (pt.9)","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-10-25T07:49:47Z","receivedAt":"2024-10-25T07:50:10Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> Thanks! I've queued all fixes locally, but will wait a bit before\n> sending them out so that other reviewers have a chance to chime in\n> first.\n\nI've got nothing more to add.\n\n--\nToon\n"},{"id":"506566","messageId":"qxtstkkch5toz6t7jdknwkdzrlkfu3b7gtw3wpg2iuqsmliqlq@w2qz65ujwj4r","threadId":"62312","inReplyTo":"89b66411354437a1e3a97751f185889dffb84126.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 01/22] builtin/ls-remote: plug leaking server options","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-04T22:10:37Z","receivedAt":"2024-11-04T22:12:18Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> The server options populated via `OPT_STRING_LIST()` is never cleared,\n\ns/is/are/\n\n> causing a memory leak. Plug it.\n> \n> This leak is exposed by t5702, but plugging it alone does not make the\n> whole test suite pass.\n> \n> Signed-off-by: Patrick Steinhardt \n"},{"id":"506568","messageId":"oujqbaznoam2vbvw3vzkg47axqisfutihrc6dtfl5ij7iufn47@7e73cxaz4pna","threadId":"62312","inReplyTo":"5d5f6867f918460001f62aaa78f24cf3cbe53a3c.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 07/22] pretty: clear signature check","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-04T22:15:14Z","receivedAt":"2024-11-04T22:16:52Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> The signature check in of the formatting context is never getting\n\ns/in of/in/\n\n> released. Fix this to plug the resulting memory leak.\n> \n> Signed-off-by: Patrick Steinhardt \n> ---\n> pretty.c | 1 +\n> t/t4202-log.sh | 1 +\n> t/t7031-verify-tag-signed-ssh.sh | 1 +\n> t/t7510-signed-commit.sh | 1 +\n> t/t7528-signed-commit-ssh.sh | 1 +\n> 5 files changed, 5 insertions(+)\n> \n> diff --git a/pretty.c b/pretty.c\n> index 6403e268900..098378720a4 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -2032,6 +2032,7 @@ void repo_format_commit_message(struct repository *r,\n> \n> \tfree(context.commit_encoding);\n> \trepo_unuse_commit_buffer(r, commit, context.message);\n> +\tsignature_check_clear(&context.signature_check);\n\nInitially I was thinking it might be nice to see the\n`format_commit_context` cleanup extracted to its own function, but being\nthat it only has a single use internal to \"pretty.c\" it probably isn't\nworth it.\n\n> }\n> \n> static void pp_header(struct pretty_print_context *pp,\n"},{"id":"506569","messageId":"e007decf-0156-44b2-bf76-66a91593d233@app.fastmail.com","threadId":"62312","inReplyTo":"qxtstkkch5toz6t7jdknwkdzrlkfu3b7gtw3wpg2iuqsmliqlq@w2qz65ujwj4r","subject":"Re: [PATCH v2 01/22] builtin/ls-remote: plug leaking server options","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-11-04T22:18:23Z","receivedAt":"2024-11-04T22:18:46Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Nov 4, 2024, at 23:10, Justin Tobler wrote:\n> On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n>> The server options populated via `OPT_STRING_LIST()` is never cleared,\n>\n> s/is/are/\n\nI guess “is” was chosen because “the list is”.\n\n The [list] populated via `OPT_STRING_LIST()` is never cleared,\n\n-- \nKristoffer Haugsbakk\n"},{"id":"506570","messageId":"5jdz52uqizr4zigq7nefgbmchqgtj2tkgwm7gxrbzfhe6oojpf@qjhdgoi2qvgg","threadId":"62312","inReplyTo":"9f967dfe5d55ca7150bf3e118279388290f7d28c.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 09/22] builtin/commit: fix leaking change data contents","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-04T22:21:15Z","receivedAt":"2024-11-04T22:22:54Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> While we free the worktree change data, we never free its contents. Fix\n> this.\n\nAh, so this worktree change data was part of the string list and was\nbeing freed, but it also had memory allocations itself that were not.\nMakes sense :)\n\n> \n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/commit.c | 9 ++++++++-\n> t/t7500-commit-template-squash-signoff.sh | 1 +\n> 2 files changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 8db4e9df0c9..18a55bd1b91 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -728,6 +728,13 @@ static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n> \trepo_unuse_commit_buffer(the_repository, commit, buffer);\n> }\n> \n> +static void change_data_free(void *util, const char *str UNUSED)\n> +{\n> +\tstruct wt_status_change_data *d = util;\n> +\tfree(d->rename_source);\n> +\tfree(d);\n> +}\n> +\n> static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\t\t struct commit *current_head,\n> \t\t\t struct wt_status *s,\n> @@ -991,7 +998,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\ts->use_color = 0;\n> \t\tcommittable = run_status(s->fp, index_file, prefix, 1, s);\n> \t\ts->use_color = saved_color_setting;\n> -\t\tstring_list_clear(&s->change, 1);\n> +\t\tstring_list_clear_func(&s->change, change_data_free);\n> \t} else {\n> \t\tstruct object_id oid;\n> \t\tconst char *parent = \"HEAD\";\n> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\n> index 4dca8d97a77..379d3ed3413 100755\n> --- a/t/t7500-commit-template-squash-signoff.sh\n> +++ b/t/t7500-commit-template-squash-signoff.sh\n> @@ -7,6 +7,7 @@ test_description='git commit\n> \n> Tests for template, signoff, squash and -F functions.'\n> \n> +TEST_PASSES_SANITIZE_LEAK=true\n> . ./test-lib.sh\n> \n> . \"$TEST_DIRECTORY\"/lib-rebase.sh\n> -- \n> 2.47.0.72.gef8ce8f3d4.dirty\n> \n> \n"},{"id":"506571","messageId":"wh227vmiokssq532pqfefzxlbt6ylonwza3sxumgohpwuojxbv@k2ujsz5upvop","threadId":"62312","inReplyTo":"e3ffd59123f23f53c0bee930ef7602acf2d800c2.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 10/22] trailer: fix leaking trailer values","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-04T22:25:38Z","receivedAt":"2024-11-04T22:27:17Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> Fix leaking trailer values when replacing the value with a command or\n> when the token value is empty.\n> \n> This leak is exposed by t7513, but plugging it does not make the whole\n> test suite pass.\n> \n> Signed-off-by: Patrick Steinhardt \n> ---\n> trailer.c | 12 +++++++++---\n> 1 file changed, 9 insertions(+), 3 deletions(-)\n> \n> diff --git a/trailer.c b/trailer.c\n> index 682d74505bf..f1eca6d5d15 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -249,17 +249,23 @@ static char *apply_command(struct conf_info *conf, const char *arg)\n> static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)\n> {\n> \tif (arg_tok->conf.command || arg_tok->conf.cmd) {\n> -\t\tconst char *arg;\n> +\t\tchar *value_to_free = NULL;\n> +\t\tchar *arg;\n> +\n> \t\tif (arg_tok->value && arg_tok->value[0]) {\n> -\t\t\targ = arg_tok->value;\n> +\t\t\targ = (char *)arg_tok->value;\n\nNaive question, is this cast not redundant? From looking at `struct\narg_item`, `value` already looks to be this type.\n\n> \t\t} else {\n> \t\t\tif (in_tok && in_tok->value)\n> \t\t\t\targ = xstrdup(in_tok->value);\n> \t\t\telse\n> \t\t\t\targ = xstrdup(\"\");\n> +\t\t\tvalue_to_free = arg_tok->value;\n> \t\t}\n> +\n> \t\targ_tok->value = apply_command(&arg_tok->conf, arg);\n> -\t\tfree((char *)arg);\n> +\n> +\t\tfree(value_to_free);\n> +\t\tfree(arg);\n> \t}\n> }\n> \n> -- \n> 2.47.0.72.gef8ce8f3d4.dirty\n> \n> \n"},{"id":"506572","messageId":"ye7hagpn5xmtlndgk5usx7i4xrg2s32eyukcy6fapxitye24ws@ddrg5zrnxkn3","threadId":"62312","inReplyTo":"5b851453bcea945f95c3f29138e510d8448e96e6.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 11/22] trailer: fix leaking strbufs when formatting trailers","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-04T22:31:34Z","receivedAt":"2024-11-04T22:33:12Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> We are populating, but never releasing two string buffers in\n> `format_trailers()`, causing a memory leak. Plug this leak by lifting\n> those buffers outside of the loop and releasing them on function return.\n> This fixes the memory leaks, but also optimizes the loop as we don't\n> have to reallocate the buffers on every single iteration.\n\nI see that we were previously calling `strbuf_release()` inside of the\nloop. In practice were these never hit? Or just sometimes skipped my the\n\"continue\" in the loop? The commit message makes it sound like the\nformer.\n\n"},{"id":"506575","messageId":"rbjmoxqrfpcpz2b5oqxcchuwpbvw4uz4ps4j7ctkd4ux5h4xwj@n4mngoog4ffd","threadId":"62312","inReplyTo":"76bbcb3fe301fe273578a71849f99953ea94695c.1729502824.git.ps@pks.im","subject":"Re: [PATCH v2 15/22] combine-diff: fix leaking lost lines","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-04T22:43:38Z","receivedAt":"2024-11-04T22:45:19Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> The `cnt` variable tracks the number of lines in a patch diff. It can\n> happen though that there are no newlines, in which case we'd still end\n> up allocating our array of `sline`s. In fact, we always allocate it with\n> `cnt + 2` entries. But when we loop through the array to clear it at the\n> end of this function we only loop until `lno < cnt`, and thus we may not\n> end up releasing whatever the two extra `sline`s contain.\n> \n> Plug this memory leak.\n> \n> Signed-off-by: Patrick Steinhardt \n> ---\n> combine-diff.c | 2 +-\n> t/t4038-diff-combined.sh | 1 +\n> 2 files changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/combine-diff.c b/combine-diff.c\n> index f6b624dc288..3c6d9507fec 100644\n> --- a/combine-diff.c\n> +++ b/combine-diff.c\n> @@ -1220,7 +1220,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n> \t}\n> \tfree(result);\n> \n> -\tfor (lno = 0; lno < cnt; lno++) {\n> +\tfor (lno = 0; lno < cnt + 2; lno++) {\n\nFrom looking only at the code, its not very obvious to me where the \"+2\"\ncomes from. Not really worth a reroll, but it might be nice to leave a\nnote with some context.\n\n> \t\tif (sline[lno].lost) {\n> \t\t\tstruct lline *ll = sline[lno].lost;\n> \t\t\twhile (ll) {\n> diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\n> index 2ce26e585c9..00190802d83 100755\n> --- a/t/t4038-diff-combined.sh\n> +++ b/t/t4038-diff-combined.sh\n> @@ -5,6 +5,7 @@ test_description='combined diff'\n> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n> \n> +TEST_PASSES_SANITIZE_LEAK=true\n> . ./test-lib.sh\n> . \"$TEST_DIRECTORY\"/lib-diff.sh\n> \n> -- \n> 2.47.0.72.gef8ce8f3d4.dirty\n> \n> \n"},{"id":"506576","messageId":"v4cfsfnmvqhedtsoykax4wkq2ug6ke4c2rrtbwmtanmc2fonzj@4il4rzkuyffa","threadId":"62312","inReplyTo":"cover.1729502823.git.ps@pks.im","subject":"Re: [PATCH v2 00/22] Memory leak fixes (pt.9)","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-04T22:46:57Z","receivedAt":"2024-11-04T22:48:35Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/10/21 11:27AM, Patrick Steinhardt wrote:\n> Hi,\n> \n> this is the second version of my 9th series of memory leak fixes.\n> \n> Changes compared to v1:\n> \n> - Split up the trailer fixes into two separate patches so that we can\n> explain them standalone.\n> \n> - Adapt the second trailer memory leak fix to instead pull up the\n> strbufs out of the loop such that we can reuse them. This makes the\n> code flow more naturally and optimizes the loop.\n> \n> Thanks!\n> \n> Patrick\n\nI've reviewed all the patches in this version and left a few\nnon-blocking thoughts. Overall, this version looks good to me though. :)\n\n-Justin\n"},{"id":"506603","messageId":"ZymzIZpQV8FHKpVI@pks.im","threadId":"62312","inReplyTo":"wh227vmiokssq532pqfefzxlbt6ylonwza3sxumgohpwuojxbv@k2ujsz5upvop","subject":"Re: [PATCH v2 10/22] trailer: fix leaking trailer values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T05:54:46Z","receivedAt":"2024-11-05T05:54:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 04, 2024 at 04:25:38PM -0600, Justin Tobler wrote:\n> On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> > Fix leaking trailer values when replacing the value with a command or\n> > when the token value is empty.\n> > \n> > This leak is exposed by t7513, but plugging it does not make the whole\n> > test suite pass.\n> > \n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> > trailer.c | 12 +++++++++---\n> > 1 file changed, 9 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/trailer.c b/trailer.c\n> > index 682d74505bf..f1eca6d5d15 100644\n> > --- a/trailer.c\n> > +++ b/trailer.c\n> > @@ -249,17 +249,23 @@ static char *apply_command(struct conf_info *conf, const char *arg)\n> > static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)\n> > {\n> > \tif (arg_tok->conf.command || arg_tok->conf.cmd) {\n> > -\t\tconst char *arg;\n> > +\t\tchar *value_to_free = NULL;\n> > +\t\tchar *arg;\n> > +\n> > \t\tif (arg_tok->value && arg_tok->value[0]) {\n> > -\t\t\targ = arg_tok->value;\n> > +\t\t\targ = (char *)arg_tok->value;\n> \n> Naive question, is this cast not redundant? From looking at `struct\n> arg_item`, `value` already looks to be this type.\n\nIndeed it is, good catch!\n\nPatrick\n"},{"id":"506604","messageId":"ZymzLJ4p7AK_V92K@pks.im","threadId":"62312","inReplyTo":"ye7hagpn5xmtlndgk5usx7i4xrg2s32eyukcy6fapxitye24ws@ddrg5zrnxkn3","subject":"Re: [PATCH v2 11/22] trailer: fix leaking strbufs when formatting trailers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T05:54:52Z","receivedAt":"2024-11-05T05:55:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 04, 2024 at 04:31:34PM -0600, Justin Tobler wrote:\n> On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> > We are populating, but never releasing two string buffers in\n> > `format_trailers()`, causing a memory leak. Plug this leak by lifting\n> > those buffers outside of the loop and releasing them on function return.\n> > This fixes the memory leaks, but also optimizes the loop as we don't\n> > have to reallocate the buffers on every single iteration.\n> \n> I see that we were previously calling `strbuf_release()` inside of the\n> loop. In practice were these never hit? Or just sometimes skipped my the\n> \"continue\" in the loop? The commit message makes it sound like the\n> former.\n\nTrue, \"never\" is definitely too strong of a wording here. Will adapt,\nthanks!\n\nPatrick\n"},{"id":"506605","messageId":"ZymzN5TQK5cpGw5Z@pks.im","threadId":"62312","inReplyTo":"rbjmoxqrfpcpz2b5oqxcchuwpbvw4uz4ps4j7ctkd4ux5h4xwj@n4mngoog4ffd","subject":"Re: [PATCH v2 15/22] combine-diff: fix leaking lost lines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T05:55:03Z","receivedAt":"2024-11-05T05:55:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 04, 2024 at 04:43:38PM -0600, Justin Tobler wrote:\n> On 24/10/21 11:28AM, Patrick Steinhardt wrote:\n> > The `cnt` variable tracks the number of lines in a patch diff. It can\n> > happen though that there are no newlines, in which case we'd still end\n> > up allocating our array of `sline`s. In fact, we always allocate it with\n> > `cnt + 2` entries. But when we loop through the array to clear it at the\n> > end of this function we only loop until `lno < cnt`, and thus we may not\n> > end up releasing whatever the two extra `sline`s contain.\n> > \n> > Plug this memory leak.\n> > \n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> > combine-diff.c | 2 +-\n> > t/t4038-diff-combined.sh | 1 +\n> > 2 files changed, 2 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/combine-diff.c b/combine-diff.c\n> > index f6b624dc288..3c6d9507fec 100644\n> > --- a/combine-diff.c\n> > +++ b/combine-diff.c\n> > @@ -1220,7 +1220,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n> > \t}\n> > \tfree(result);\n> > \n> > -\tfor (lno = 0; lno < cnt; lno++) {\n> > +\tfor (lno = 0; lno < cnt + 2; lno++) {\n> \n> From looking only at the code, its not very obvious to me where the \"+2\"\n> comes from. Not really worth a reroll, but it might be nice to leave a\n> note with some context.\n\nThe code is quite convoluted and hard to read, agreed. In any case, we\ncall `CALLOC_ARRAY(sline, st_add(cnt, 2))`, and as a comment further up\nmentions:\n\n /* Even p_lno[cnt+1] is valid -- that is for the end line number for\n * deletion hunk at the end.\n */\n\nThis explains the +1. The second +1 seems to never be populated and acts\nmore like a sentinel value.\n\nI don't really think it makes a ton of sense to explain why the sline\narray is overallocated when freeing it, and we already do explain it.\nBut I'll touch up the commit message a bit and sneak in a fix of the\nabove comment to be properly formatted, which also gives the necessary\ncontext when reading the diff, only.\n\nPatrick\n"},{"id":"506606","messageId":"ZymzPdjKy9AFBuvM@pks.im","threadId":"62312","inReplyTo":"v4cfsfnmvqhedtsoykax4wkq2ug6ke4c2rrtbwmtanmc2fonzj@4il4rzkuyffa","subject":"Re: [PATCH v2 00/22] Memory leak fixes (pt.9)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T05:55:09Z","receivedAt":"2024-11-05T05:55:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 04, 2024 at 04:46:57PM -0600, Justin Tobler wrote:\n> On 24/10/21 11:27AM, Patrick Steinhardt wrote:\n> > Hi,\n> > \n> > this is the second version of my 9th series of memory leak fixes.\n> > \n> > Changes compared to v1:\n> > \n> > - Split up the trailer fixes into two separate patches so that we can\n> > explain them standalone.\n> > \n> > - Adapt the second trailer memory leak fix to instead pull up the\n> > strbufs out of the loop such that we can reuse them. This makes the\n> > code flow more naturally and optimizes the loop.\n> > \n> > Thanks!\n> > \n> > Patrick\n> \n> I've reviewed all the patches in this version and left a few\n> non-blocking thoughts. Overall, this version looks good to me though. :)\n\nThanks for your review!\n\nPatrick\n"},{"id":"506607","messageId":"cover.1730786195.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1728624670.git.ps@pks.im","subject":"[PATCH v3 00/22] Memory leak fixes (pt.9)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:16:40Z","receivedAt":"2024-11-05T06:16:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the third revision of my 9th series of memory leak fixes.\nChanges compared to v2:\n\n - Remove an unnecessary cast.\n\n - Fix a duplicate newline.\n\n - Polish a couple of commit messages.\n\nThanks!\n\nPatrick\n\nPatrick Steinhardt (22):\n builtin/ls-remote: plug leaking server options\n t/helper: fix leaks in \"reach\" test tool\n grep: fix leak in `grep_splice_or()`\n builtin/grep: fix leak with `--max-count=0`\n revision: fix leaking bloom filters\n diff-lib: fix leaking diffopts in `do_diff_cache()`\n pretty: clear signature check\n upload-pack: fix leaking URI protocols\n builtin/commit: fix leaking change data contents\n trailer: fix leaking trailer values\n trailer: fix leaking strbufs when formatting trailers\n builtin/commit: fix leaking cleanup config\n transport-helper: fix leaking import/export marks\n builtin/tag: fix leaking key ID on failure to sign\n combine-diff: fix leaking lost lines\n dir: release untracked cache data\n sparse-index: correctly free EWAH contents\n t/helper: stop re-initialization of `the_repository`\n t/helper: fix leaking buffer in \"dump-untracked-cache\"\n dir: fix leak when parsing \"status.showUntrackedFiles\"\n builtin/merge: release output buffer after performing merge\n list-objects-filter-options: work around reported leak on error\n\n builtin/commit.c | 26 +++++++++++++++++------\n builtin/grep.c | 13 +++++++++---\n builtin/ls-remote.c | 1 +\n builtin/merge.c | 1 +\n builtin/tag.c | 2 +-\n combine-diff.c | 5 +++--\n diff-lib.c | 1 +\n dir.c | 12 +++++++++--\n grep.c | 1 +\n list-objects-filter-options.c | 17 ++++++---------\n pretty.c | 1 +\n revision.c | 5 +++++\n sparse-index.c | 7 ++++--\n t/helper/test-dump-untracked-cache.c | 2 ++\n t/helper/test-reach.c | 10 +++++++++\n t/helper/test-read-cache.c | 2 --\n t/t4038-diff-combined.sh | 1 +\n t/t4202-log.sh | 1 +\n t/t4216-log-bloom.sh | 1 +\n t/t5702-protocol-v2.sh | 1 +\n t/t5801-remote-helpers.sh | 1 +\n t/t6112-rev-list-filters-objects.sh | 1 +\n t/t6424-merge-unrelated-index-changes.sh | 1 +\n t/t6600-test-reach.sh | 1 +\n t/t7004-tag.sh | 1 +\n t/t7031-verify-tag-signed-ssh.sh | 1 +\n t/t7063-status-untracked-cache.sh | 1 +\n t/t7500-commit-template-squash-signoff.sh | 1 +\n t/t7502-commit-porcelain.sh | 1 +\n t/t7510-signed-commit.sh | 1 +\n t/t7513-interpret-trailers.sh | 1 +\n t/t7519-status-fsmonitor.sh | 1 +\n t/t7528-signed-commit-ssh.sh | 1 +\n t/t7610-mergetool.sh | 1 +\n t/t7810-grep.sh | 1 +\n trailer.c | 22 +++++++++++++------\n transport-helper.c | 2 ++\n upload-pack.c | 1 +\n 38 files changed, 115 insertions(+), 35 deletions(-)\n\nRange-diff against v2:\n 1: 89b66411354 ! 1: fbb9e68e2f2 builtin/ls-remote: plug leaking server options\n @@ Metadata\n ## Commit message ##\n builtin/ls-remote: plug leaking server options\n \n - The server options populated via `OPT_STRING_LIST()` is never cleared,\n - causing a memory leak. Plug it.\n + The list of server options populated via `OPT_STRING_LIST()` is never\n + cleared, causing a memory leak. Plug it.\n \n This leak is exposed by t5702, but plugging it alone does not make the\n whole test suite pass.\n 2: 1c42e194b20 = 2: 17136f4bdb3 t/helper: fix leaks in \"reach\" test tool\n 3: cb4eee37b40 ! 3: 74b21470194 grep: fix leak in `grep_splice_or()`\n @@ Commit message\n grep: fix leak in `grep_splice_or()`\n \n In `grep_splice_or()` we search for the next `TRUE` node in our tree of\n - grep exrpessions and replace it with the given new expression. But we\n + grep expressions and replace it with the given new expression. But we\n don't free the old node, which causes a memory leak. Plug it.\n \n This leak is exposed by t7810, but plugging it alone isn't sufficient to\n 4: 6b2c8842ef5 = 4: d716f93169a builtin/grep: fix leak with `--max-count=0`\n 5: 7527b31a28f = 5: aeb8a19d28d revision: fix leaking bloom filters\n 6: 60af98cb2c7 ! 6: 24d9d9b1358 diff-lib: fix leaking diffopts in `do_diff_cache()`\n @@ Commit message\n In `do_diff_cache()` we initialize a new `rev_info` and then overwrite\n its `diffopt` with a user-provided set of options. This can leak memory\n because `repo_init_revisions()` may end up allocating memory for the\n - `diffopt` itself depending on the configuration. And as that field is\n - overwritten we won't ever free that.\n + `diffopt` itself depending on the configuration. And since that field is\n + overwritten we won't ever free it.\n \n Plug the memory leak by releasing the diffopts before we overwrite them.\n \n 7: 5d5f6867f91 ! 7: 58ebef7e757 pretty: clear signature check\n @@ Metadata\n ## Commit message ##\n pretty: clear signature check\n \n - The signature check in of the formatting context is never getting\n - released. Fix this to plug the resulting memory leak.\n + The signature check in the formatting context is never getting released.\n + Fix this to plug the resulting memory leak.\n \n Signed-off-by: Patrick Steinhardt \n \n 8: 0d503e40194 = 8: c5813079d44 upload-pack: fix leaking URI protocols\n 9: 9f967dfe5d5 = 9: f66fa4e0e25 builtin/commit: fix leaking change data contents\n10: e3ffd59123f ! 10: dac63bae39e trailer: fix leaking trailer values\n @@ trailer.c: static char *apply_command(struct conf_info *conf, const char *arg)\n +\t\tchar *arg;\n +\n \t\tif (arg_tok->value && arg_tok->value[0]) {\n --\t\t\targ = arg_tok->value;\n -+\t\t\targ = (char *)arg_tok->value;\n + \t\t\targ = arg_tok->value;\n \t\t} else {\n - \t\t\tif (in_tok && in_tok->value)\n +@@ trailer.c: static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg\n \t\t\t\targ = xstrdup(in_tok->value);\n \t\t\telse\n \t\t\t\targ = xstrdup(\"\");\n11: 5b851453bce ! 11: 82269e5d5be trailer: fix leaking strbufs when formatting trailers\n @@ Metadata\n ## Commit message ##\n trailer: fix leaking strbufs when formatting trailers\n \n - We are populating, but never releasing two string buffers in\n - `format_trailers()`, causing a memory leak. Plug this leak by lifting\n - those buffers outside of the loop and releasing them on function return.\n - This fixes the memory leaks, but also optimizes the loop as we don't\n - have to reallocate the buffers on every single iteration.\n + When formatting trailer lines we iterate through each of the trailers\n + and munge their respective token/value pairs according to the trailer\n + options. When formatting a trailer that has its `item->token` pointer\n + set we perform the munging in two local buffers. In the case where we\n + figure out that the value is empty and `trim_empty` is set we just skip\n + over the trailer item. But the buffers are local to the loop and we\n + don't release their contents, leading to a memory leak.\n +\n + Plug this leak by lifting the buffers outside of the loop and releasing\n + them on function return. This fixes the memory leaks, but also optimizes\n + the loop as we don't have to reallocate the buffers on every single\n + iteration.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ trailer.c: void format_trailers(const struct process_trailer_options *opts,\n \tsize_t origlen = out->len;\n \tstruct list_head *pos;\n \tstruct trailer_item *item;\n - \n -+\n +@@ trailer.c: void format_trailers(const struct process_trailer_options *opts,\n \tlist_for_each(pos, trailers) {\n \t\titem = list_entry(pos, struct trailer_item, list);\n \t\tif (item->token) {\n12: 60c3f6146f3 = 12: a627e03702e builtin/commit: fix leaking cleanup config\n13: ea81cd8db86 = 13: 40e0c2a2cac transport-helper: fix leaking import/export marks\n14: b700ab9079f = 14: ffa5d9eae7e builtin/tag: fix leaking key ID on failure to sign\n15: 76bbcb3fe30 ! 15: 70dd0cb6933 combine-diff: fix leaking lost lines\n @@ Commit message\n The `cnt` variable tracks the number of lines in a patch diff. It can\n happen though that there are no newlines, in which case we'd still end\n up allocating our array of `sline`s. In fact, we always allocate it with\n - `cnt + 2` entries. But when we loop through the array to clear it at the\n - end of this function we only loop until `lno < cnt`, and thus we may not\n - end up releasing whatever the two extra `sline`s contain.\n + `cnt + 2` entries: one extra entry for the deletion hunk at the end, and\n + another entry that we don't seem to ever populate at all but acts as a\n + kind of sentinel value.\n \n - Plug this memory leak.\n + When we loop through the array to clear it at the end of this function\n + we only loop until `lno < cnt`, and thus we may not end up releasing\n + whatever the two extra `sline`s contain. While that shouldn't matter for\n + the sentinel value, it does matter for the extra deletion hunk sline.\n + Regardless of that, plug this memory leak by releasing both extra\n + entries, which makes the logic a bit easier to reason about.\n +\n + While at it, fix the formatting of a local comment, which incidentally\n + also provides the necessary context for why we overallocate the `sline`\n + array.\n \n Signed-off-by: Patrick Steinhardt \n \n ## combine-diff.c ##\n +@@ combine-diff.c: static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n + \tresult_file.ptr = result;\n + \tresult_file.size = result_size;\n + \n +-\t/* Even p_lno[cnt+1] is valid -- that is for the end line number\n ++\t/*\n ++\t * Even p_lno[cnt+1] is valid -- that is for the end line number\n + \t * for deletion hunk at the end.\n + \t */\n + \tCALLOC_ARRAY(sline[0].p_lno, st_mult(st_add(cnt, 2), num_parent));\n @@ combine-diff.c: static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t}\n \tfree(result);\n16: d6b96a4012d = 16: b248f266a91 dir: release untracked cache data\n17: 3aa6bac5079 = 17: 76e9a6d5792 sparse-index: correctly free EWAH contents\n18: 132fe750906 = 18: 70f16486d77 t/helper: stop re-initialization of `the_repository`\n19: b8b702eeb28 = 19: f056d31ca50 t/helper: fix leaking buffer in \"dump-untracked-cache\"\n20: ad309ac1b37 = 20: 714c9286e7a dir: fix leak when parsing \"status.showUntrackedFiles\"\n21: 5e243f9ee53 ! 21: 0ff65c1213b builtin/merge: release outbut buffer after performing merge\n @@ Metadata\n Author: Patrick Steinhardt \n \n ## Commit message ##\n - builtin/merge: release outbut buffer after performing merge\n + builtin/merge: release output buffer after performing merge\n \n The `obuf` member of `struct merge_options` is used to buffer output in\n some cases. In order to not discard its allocated memory we only release\n22: b75376e3725 = 22: d9e122bb5db list-objects-filter-options: work around reported leak on error\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506608","messageId":"fbb9e68e2f2039204a47b25c7a8d385cca04afbc.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 01/22] builtin/ls-remote: plug leaking server options","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:16:43Z","receivedAt":"2024-11-05T06:16:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The list of server options populated via `OPT_STRING_LIST()` is never\ncleared, causing a memory leak. Plug it.\n\nThis leak is exposed by t5702, but plugging it alone does not make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/ls-remote.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex f723b3bf3bb..f333821b994 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -166,6 +166,7 @@ int cmd_ls_remote(int argc,\n \t\tstatus = 0; /* we found something */\n \t}\n \n+\tstring_list_clear(&server_options, 0);\n \tref_sorting_release(sorting);\n \tref_array_clear(&ref_array);\n \tif (transport_disconnect(transport))\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506609","messageId":"17136f4bdb39cb1ccbcc06ec4867d7606fce2202.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 02/22] t/helper: fix leaks in \"reach\" test tool","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:16:46Z","receivedAt":"2024-11-05T06:16:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"reach\" test tool doesn't bother to clean up any of its allocated\nresources, causing various leaks. Plug them.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-reach.c | 10 ++++++++++\n t/t6600-test-reach.sh | 1 +\n 2 files changed, 11 insertions(+)\n\ndiff --git a/t/helper/test-reach.c b/t/helper/test-reach.c\nindex 995e382863a..84deee604ad 100644\n--- a/t/helper/test-reach.c\n+++ b/t/helper/test-reach.c\n@@ -127,10 +127,12 @@ int cmd__reach(int ac, const char **av)\n \t\t\texit(128);\n \t\tprintf(\"%s(A,X):\\n\", av[1]);\n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t} else if (!strcmp(av[1], \"reduce_heads\")) {\n \t\tstruct commit_list *list = reduce_heads(X);\n \t\tprintf(\"%s(X):\\n\", av[1]);\n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t} else if (!strcmp(av[1], \"can_all_from_reach\")) {\n \t\tprintf(\"%s(X,Y):%d\\n\", av[1], can_all_from_reach(X, Y, 1));\n \t} else if (!strcmp(av[1], \"can_all_from_reach_with_flag\")) {\n@@ -153,6 +155,7 @@ int cmd__reach(int ac, const char **av)\n \t\t\tfilter.with_commit_tag_algo = 0;\n \n \t\tprintf(\"%s(_,A,X,_):%d\\n\", av[1], commit_contains(&filter, A, X, &cache));\n+\t\tclear_contains_cache(&cache);\n \t} else if (!strcmp(av[1], \"get_reachable_subset\")) {\n \t\tconst int reachable_flag = 1;\n \t\tint i, count = 0;\n@@ -176,7 +179,14 @@ int cmd__reach(int ac, const char **av)\n \t\t\tdie(_(\"too many commits marked reachable\"));\n \n \t\tprint_sorted_commit_ids(list);\n+\t\tfree_commit_list(list);\n \t}\n \n+\tobject_array_clear(&X_obj);\n+\tstrbuf_release(&buf);\n+\tfree_commit_list(X);\n+\tfree_commit_list(Y);\n+\tfree(X_array);\n+\tfree(Y_array);\n \treturn 0;\n }\ndiff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh\nindex 2591f8b8b39..307deefed2c 100755\n--- a/t/t6600-test-reach.sh\n+++ b/t/t6600-test-reach.sh\n@@ -2,6 +2,7 @@\n \n test_description='basic commit reachability tests'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Construct a grid-like commit graph with points (x,y)\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506610","messageId":"74b214701942b6412965b53b3aec4580ef6fd242.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 03/22] grep: fix leak in `grep_splice_or()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:16:50Z","receivedAt":"2024-11-05T06:16:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `grep_splice_or()` we search for the next `TRUE` node in our tree of\ngrep expressions and replace it with the given new expression. But we\ndon't free the old node, which causes a memory leak. Plug it.\n\nThis leak is exposed by t7810, but plugging it alone isn't sufficient to\nmake the test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n grep.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/grep.c b/grep.c\nindex 701e58de04e..e9337f32cbf 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -756,6 +756,7 @@ static struct grep_expr *grep_splice_or(struct grep_expr *x, struct grep_expr *y\n \t\tassert(x->node == GREP_NODE_OR);\n \t\tif (x->u.binary.right &&\n \t\t x->u.binary.right->node == GREP_NODE_TRUE) {\n+\t\t\tfree(x->u.binary.right);\n \t\t\tx->u.binary.right = y;\n \t\t\tbreak;\n \t\t}\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506611","messageId":"d716f93169a5a7c2352684998f75e85cf70b9224.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 04/22] builtin/grep: fix leak with `--max-count=0`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:16:52Z","receivedAt":"2024-11-05T06:17:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When executing with `--max-count=0` we'll return early from git-grep(1)\nwithout performing any cleanup, which causes memory leaks. Plug these.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/grep.c | 13 ++++++++++---\n t/t7810-grep.sh | 1 +\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex f17d46a06e4..98b85c7fcac 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -906,6 +906,7 @@ int cmd_grep(int argc,\n \tint dummy;\n \tint use_index = 1;\n \tint allow_revs;\n+\tint ret;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"cached\", &cached,\n@@ -1172,8 +1173,10 @@ int cmd_grep(int argc,\n \t * Optimize out the case where the amount of matches is limited to zero.\n \t * We do this to keep results consistent with GNU grep(1).\n \t */\n-\tif (opt.max_count == 0)\n-\t\treturn 1;\n+\tif (opt.max_count == 0) {\n+\t\tret = 1;\n+\t\tgoto out;\n+\t}\n \n \tif (show_in_pager) {\n \t\tif (num_threads > 1)\n@@ -1267,10 +1270,14 @@ int cmd_grep(int argc,\n \t\thit |= wait_all();\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\n+\n+\tret = !hit;\n+\n+out:\n \tclear_pathspec(&pathspec);\n \tstring_list_clear(&path_list, 0);\n \tfree_grep_patterns(&opt);\n \tobject_array_clear(&list);\n \tfree_repos();\n-\treturn !hit;\n+\treturn ret;\n }\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex af2cf2f78ab..9e7681f0831 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -9,6 +9,7 @@ test_description='git grep various.\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_invalid_grep_expression() {\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506612","messageId":"aeb8a19d28d28e0eb1a80b0969eff7621f381a9c.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 05/22] revision: fix leaking bloom filters","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:16:58Z","receivedAt":"2024-11-05T06:17:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The memory allocated by `prepare_to_use_bloom_filter()` is not released\nby `release_revisions()`, causing a memory leak. Plug it.\n\nSigned-off-by: Patrick Steinhardt \n---\n revision.c | 5 +++++\n t/t4216-log-bloom.sh | 1 +\n 2 files changed, 6 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex f5f5b84f2b0..8df75b82249 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3227,6 +3227,11 @@ void release_revisions(struct rev_info *revs)\n \tclear_decoration(&revs->treesame, free);\n \tline_log_free(revs);\n \toidset_clear(&revs->missing_commits);\n+\n+\tfor (int i = 0; i < revs->bloom_keys_nr; i++)\n+\t\tclear_bloom_key(&revs->bloom_keys[i]);\n+\tFREE_AND_NULL(revs->bloom_keys);\n+\trevs->bloom_keys_nr = 0;\n }\n \n static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)\ndiff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh\nindex 3f163dc3969..8d22338f6aa 100755\n--- a/t/t4216-log-bloom.sh\n+++ b/t/t4216-log-bloom.sh\n@@ -4,6 +4,7 @@ test_description='git log for a path with Bloom filters'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-chunk.sh\n \n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506613","messageId":"24d9d9b1358d55dd304aea064515b61830a12b21.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 06/22] diff-lib: fix leaking diffopts in `do_diff_cache()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:00Z","receivedAt":"2024-11-05T06:17:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `do_diff_cache()` we initialize a new `rev_info` and then overwrite\nits `diffopt` with a user-provided set of options. This can leak memory\nbecause `repo_init_revisions()` may end up allocating memory for the\n`diffopt` itself depending on the configuration. And since that field is\noverwritten we won't ever free it.\n\nPlug the memory leak by releasing the diffopts before we overwrite them.\n\nSigned-off-by: Patrick Steinhardt \n---\n diff-lib.c | 1 +\n t/t7610-mergetool.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 6b14b959629..3cf353946f5 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -661,6 +661,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)\n \n \trepo_init_revisions(opt->repo, &revs, NULL);\n \tcopy_pathspec(&revs.prune_data, &opt->pathspec);\n+\tdiff_free(&revs.diffopt);\n \trevs.diffopt = *opt;\n \trevs.diffopt.no_free = 1;\n \ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 22b3a85b3e9..5c5e79e9905 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -10,6 +10,7 @@ Testing basic merge tool invocation'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # All the mergetool test work by checking out a temporary branch based\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506614","messageId":"58ebef7e757803bd7c2292f0494386464f2c581d.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 07/22] pretty: clear signature check","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:03Z","receivedAt":"2024-11-05T06:17:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The signature check in the formatting context is never getting released.\nFix this to plug the resulting memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n pretty.c | 1 +\n t/t4202-log.sh | 1 +\n t/t7031-verify-tag-signed-ssh.sh | 1 +\n t/t7510-signed-commit.sh | 1 +\n t/t7528-signed-commit-ssh.sh | 1 +\n 5 files changed, 5 insertions(+)\n\ndiff --git a/pretty.c b/pretty.c\nindex 6403e268900..098378720a4 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -2032,6 +2032,7 @@ void repo_format_commit_message(struct repository *r,\n \n \tfree(context.commit_encoding);\n \trepo_unuse_commit_buffer(r, commit, context.message);\n+\tsignature_check_clear(&context.signature_check);\n }\n \n static void pp_header(struct pretty_print_context *pp,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 51f7beb59f8..35bec4089a3 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -5,6 +5,7 @@ test_description='git log'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n . \"$TEST_DIRECTORY/lib-terminal.sh\"\ndiff --git a/t/t7031-verify-tag-signed-ssh.sh b/t/t7031-verify-tag-signed-ssh.sh\nindex 20913b37134..2ee62c07293 100755\n--- a/t/t7031-verify-tag-signed-ssh.sh\n+++ b/t/t7031-verify-tag-signed-ssh.sh\n@@ -4,6 +4,7 @@ test_description='signed tag tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 0d2dd29fe6a..eb229082e40 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -4,6 +4,7 @@ test_description='signed commit tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\ndiff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh\nindex 065f7806362..68e18856b66 100755\n--- a/t/t7528-signed-commit-ssh.sh\n+++ b/t/t7528-signed-commit-ssh.sh\n@@ -4,6 +4,7 @@ test_description='ssh signed commit tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506615","messageId":"c5813079d44ff8959f21b7b8b19414ada475c133.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 08/22] upload-pack: fix leaking URI protocols","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:06Z","receivedAt":"2024-11-05T06:17:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We don't clear `struct upload_pack::uri_protocols`, which causes a\nmemory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5702-protocol-v2.sh | 1 +\n upload-pack.c | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex 1ef540f73d3..ed55a2f7f95 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Test protocol v2 with 'git://' transport\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 6d6e0f9f980..b4a59c3518b 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -166,6 +166,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)\n \tobject_array_clear(&data->extra_edge_obj);\n \tlist_objects_filter_release(&data->filter_options);\n \tstring_list_clear(&data->allowed_filters, 0);\n+\tstring_list_clear(&data->uri_protocols, 0);\n \n \tfree((char *)data->pack_objects_hook);\n }\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506616","messageId":"f66fa4e0e25b0696c0b52ebc765a5dff18c8597e.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 09/22] builtin/commit: fix leaking change data contents","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:09Z","receivedAt":"2024-11-05T06:17:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While we free the worktree change data, we never free its contents. Fix\nthis.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/commit.c | 9 ++++++++-\n t/t7500-commit-template-squash-signoff.sh | 1 +\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8db4e9df0c9..18a55bd1b91 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -728,6 +728,13 @@ static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n \trepo_unuse_commit_buffer(the_repository, commit, buffer);\n }\n \n+static void change_data_free(void *util, const char *str UNUSED)\n+{\n+\tstruct wt_status_change_data *d = util;\n+\tfree(d->rename_source);\n+\tfree(d);\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t struct commit *current_head,\n \t\t\t struct wt_status *s,\n@@ -991,7 +998,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\ts->use_color = 0;\n \t\tcommittable = run_status(s->fp, index_file, prefix, 1, s);\n \t\ts->use_color = saved_color_setting;\n-\t\tstring_list_clear(&s->change, 1);\n+\t\tstring_list_clear_func(&s->change, change_data_free);\n \t} else {\n \t\tstruct object_id oid;\n \t\tconst char *parent = \"HEAD\";\ndiff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\nindex 4dca8d97a77..379d3ed3413 100755\n--- a/t/t7500-commit-template-squash-signoff.sh\n+++ b/t/t7500-commit-template-squash-signoff.sh\n@@ -7,6 +7,7 @@ test_description='git commit\n \n Tests for template, signoff, squash and -F functions.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/lib-rebase.sh\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506617","messageId":"dac63bae39e01e612621b36b698d13d06a21a2ae.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 10/22] trailer: fix leaking trailer values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:12Z","receivedAt":"2024-11-05T06:17:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Fix leaking trailer values when replacing the value with a command or\nwhen the token value is empty.\n\nThis leak is exposed by t7513, but plugging it does not make the whole\ntest suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n trailer.c | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/trailer.c b/trailer.c\nindex 682d74505bf..6bafe92b326 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -249,7 +249,9 @@ static char *apply_command(struct conf_info *conf, const char *arg)\n static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)\n {\n \tif (arg_tok->conf.command || arg_tok->conf.cmd) {\n-\t\tconst char *arg;\n+\t\tchar *value_to_free = NULL;\n+\t\tchar *arg;\n+\n \t\tif (arg_tok->value && arg_tok->value[0]) {\n \t\t\targ = arg_tok->value;\n \t\t} else {\n@@ -257,9 +259,13 @@ static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg\n \t\t\t\targ = xstrdup(in_tok->value);\n \t\t\telse\n \t\t\t\targ = xstrdup(\"\");\n+\t\t\tvalue_to_free = arg_tok->value;\n \t\t}\n+\n \t\targ_tok->value = apply_command(&arg_tok->conf, arg);\n-\t\tfree((char *)arg);\n+\n+\t\tfree(value_to_free);\n+\t\tfree(arg);\n \t}\n }\n \n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506618","messageId":"82269e5d5be012f24f52504986a1eb043b9dc17a.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 11/22] trailer: fix leaking strbufs when formatting trailers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:17Z","receivedAt":"2024-11-05T06:17:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When formatting trailer lines we iterate through each of the trailers\nand munge their respective token/value pairs according to the trailer\noptions. When formatting a trailer that has its `item->token` pointer\nset we perform the munging in two local buffers. In the case where we\nfigure out that the value is empty and `trim_empty` is set we just skip\nover the trailer item. But the buffers are local to the loop and we\ndon't release their contents, leading to a memory leak.\n\nPlug this leak by lifting the buffers outside of the loop and releasing\nthem on function return. This fixes the memory leaks, but also optimizes\nthe loop as we don't have to reallocate the buffers on every single\niteration.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t7513-interpret-trailers.sh | 1 +\n trailer.c | 12 +++++++-----\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 0f7d8938d98..38d6ccaa001 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -5,6 +5,7 @@\n \n test_description='git interpret-trailers'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # When we want one trailing space at the end of each line, let's use sed\ndiff --git a/trailer.c b/trailer.c\nindex 6bafe92b326..8ba350404d4 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1111,6 +1111,8 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t struct list_head *trailers,\n \t\t struct strbuf *out)\n {\n+\tstruct strbuf tok = STRBUF_INIT;\n+\tstruct strbuf val = STRBUF_INIT;\n \tsize_t origlen = out->len;\n \tstruct list_head *pos;\n \tstruct trailer_item *item;\n@@ -1118,9 +1120,9 @@ void format_trailers(const struct process_trailer_options *opts,\n \tlist_for_each(pos, trailers) {\n \t\titem = list_entry(pos, struct trailer_item, list);\n \t\tif (item->token) {\n-\t\t\tstruct strbuf tok = STRBUF_INIT;\n-\t\t\tstruct strbuf val = STRBUF_INIT;\n+\t\t\tstrbuf_reset(&tok);\n \t\t\tstrbuf_addstr(&tok, item->token);\n+\t\t\tstrbuf_reset(&val);\n \t\t\tstrbuf_addstr(&val, item->value);\n \n \t\t\t/*\n@@ -1151,9 +1153,6 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t\t\tif (!opts->separator)\n \t\t\t\t\tstrbuf_addch(out, '\\n');\n \t\t\t}\n-\t\t\tstrbuf_release(&tok);\n-\t\t\tstrbuf_release(&val);\n-\n \t\t} else if (!opts->only_trailers) {\n \t\t\tif (opts->separator && out->len != origlen) {\n \t\t\t\tstrbuf_addbuf(out, opts->separator);\n@@ -1165,6 +1164,9 @@ void format_trailers(const struct process_trailer_options *opts,\n \t\t\t\tstrbuf_addch(out, '\\n');\n \t\t}\n \t}\n+\n+\tstrbuf_release(&tok);\n+\tstrbuf_release(&val);\n }\n \n void format_trailers_from_commit(const struct process_trailer_options *opts,\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506619","messageId":"a627e03702e184f038607b78ec28085020b60ec3.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 12/22] builtin/commit: fix leaking cleanup config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:20Z","receivedAt":"2024-11-05T06:17:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The cleanup string set by the config is leaking when it is being\noverridden by an option. Fix this by tracking these via two separate\nvariables such that we can free the old value.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/commit.c | 17 ++++++++++++-----\n t/t7502-commit-porcelain.sh | 1 +\n 2 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 18a55bd1b91..71d674138c9 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -135,7 +135,7 @@ static struct strvec trailer_args = STRVEC_INIT;\n * is specified explicitly.\n */\n static enum commit_msg_cleanup_mode cleanup_mode;\n-static char *cleanup_arg;\n+static char *cleanup_config;\n \n static enum commit_whence whence;\n static int use_editor = 1, include_status = 1;\n@@ -1387,8 +1387,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (0 <= edit_flag)\n \t\tuse_editor = edit_flag;\n \n-\tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n-\n \thandle_untracked_files_arg(s);\n \n \tif (all && argc > 0)\n@@ -1636,8 +1634,10 @@ static int git_commit_config(const char *k, const char *v,\n \t\tinclude_status = git_config_bool(k, v);\n \t\treturn 0;\n \t}\n-\tif (!strcmp(k, \"commit.cleanup\"))\n-\t\treturn git_config_string(&cleanup_arg, k, v);\n+\tif (!strcmp(k, \"commit.cleanup\")) {\n+\t\tFREE_AND_NULL(cleanup_config);\n+\t\treturn git_config_string(&cleanup_config, k, v);\n+\t}\n \tif (!strcmp(k, \"commit.gpgsign\")) {\n \t\tsign_commit = git_config_bool(k, v) ? \"\" : NULL;\n \t\treturn 0;\n@@ -1658,6 +1658,7 @@ int cmd_commit(int argc,\n \t struct repository *repo UNUSED)\n {\n \tstatic struct wt_status s;\n+\tstatic const char *cleanup_arg = NULL;\n \tstatic struct option builtin_commit_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress summary after successful commit\")),\n \t\tOPT__VERBOSE(&verbose, N_(\"show diff in commit message template\")),\n@@ -1757,6 +1758,12 @@ int cmd_commit(int argc,\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n+\tif (cleanup_arg) {\n+\t\tfree(cleanup_config);\n+\t\tcleanup_config = xstrdup(cleanup_arg);\n+\t}\n+\tcleanup_mode = get_cleanup_mode(cleanup_config, use_editor);\n+\n \tif (dry_run)\n \t\treturn dry_run_commit(argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argv, prefix, current_head, 0);\ndiff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\nindex b37e2018a74..84f1ff52b67 100755\n--- a/t/t7502-commit-porcelain.sh\n+++ b/t/t7502-commit-porcelain.sh\n@@ -5,6 +5,7 @@ test_description='git commit porcelain-ish'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n commit_msg_is () {\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506620","messageId":"40e0c2a2cacaf2e6879b262c43321728af25579e.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 13/22] transport-helper: fix leaking import/export marks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:23Z","receivedAt":"2024-11-05T06:17:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Fix leaking import and export marks for transport helpers.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5801-remote-helpers.sh | 1 +\n transport-helper.c | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex d21877150ed..d4882288a30 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -8,6 +8,7 @@ test_description='Test remote-helper import and export commands'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex 013ec79dc9c..bc27653cdee 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -399,6 +399,8 @@ static int release_helper(struct transport *transport)\n \tint res = 0;\n \tstruct helper_data *data = transport->data;\n \trefspec_clear(&data->rs);\n+\tfree(data->import_marks);\n+\tfree(data->export_marks);\n \tres = disconnect_helper(transport);\n \tfree(transport->data);\n \treturn res;\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506621","messageId":"ffa5d9eae7ec13cfca12199274e79c1e2de57a90.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 14/22] builtin/tag: fix leaking key ID on failure to sign","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:26Z","receivedAt":"2024-11-05T06:17:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We do not free the key ID when signing a tag fails. Do so by using\nthe common exit path.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/tag.c | 2 +-\n t/t7004-tag.sh | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 93d10d59157..c37c0a68fda 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -164,7 +164,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,\n \tint ret = -1;\n \n \tif (sign_buffer(buffer, &sig, keyid))\n-\t\treturn -1;\n+\t\tgoto out;\n \n \tif (compat) {\n \t\tconst struct git_hash_algo *algo = the_repository->hash_algo;\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b1316e62f46..42b3327e69b 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -10,6 +10,7 @@ Tests for operations with tags.'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-gpg.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506622","messageId":"70dd0cb6933c31064f6084bb5a51794a7691b154.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 15/22] combine-diff: fix leaking lost lines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:28Z","receivedAt":"2024-11-05T06:17:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `cnt` variable tracks the number of lines in a patch diff. It can\nhappen though that there are no newlines, in which case we'd still end\nup allocating our array of `sline`s. In fact, we always allocate it with\n`cnt + 2` entries: one extra entry for the deletion hunk at the end, and\nanother entry that we don't seem to ever populate at all but acts as a\nkind of sentinel value.\n\nWhen we loop through the array to clear it at the end of this function\nwe only loop until `lno < cnt`, and thus we may not end up releasing\nwhatever the two extra `sline`s contain. While that shouldn't matter for\nthe sentinel value, it does matter for the extra deletion hunk sline.\nRegardless of that, plug this memory leak by releasing both extra\nentries, which makes the logic a bit easier to reason about.\n\nWhile at it, fix the formatting of a local comment, which incidentally\nalso provides the necessary context for why we overallocate the `sline`\narray.\n\nSigned-off-by: Patrick Steinhardt \n---\n combine-diff.c | 5 +++--\n t/t4038-diff-combined.sh | 1 +\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex f6b624dc288..33d0ed70975 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1185,7 +1185,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \tresult_file.ptr = result;\n \tresult_file.size = result_size;\n \n-\t/* Even p_lno[cnt+1] is valid -- that is for the end line number\n+\t/*\n+\t * Even p_lno[cnt+1] is valid -- that is for the end line number\n \t * for deletion hunk at the end.\n \t */\n \tCALLOC_ARRAY(sline[0].p_lno, st_mult(st_add(cnt, 2), num_parent));\n@@ -1220,7 +1221,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t}\n \tfree(result);\n \n-\tfor (lno = 0; lno < cnt; lno++) {\n+\tfor (lno = 0; lno < cnt + 2; lno++) {\n \t\tif (sline[lno].lost) {\n \t\t\tstruct lline *ll = sline[lno].lost;\n \t\t\twhile (ll) {\ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex 2ce26e585c9..00190802d83 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -5,6 +5,7 @@ test_description='combined diff'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-diff.sh\n \n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506623","messageId":"b248f266a91480a84744b241496894309e5a51ca.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 16/22] dir: release untracked cache data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:35Z","receivedAt":"2024-11-05T06:17:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several cases where we invalidate untracked cache directory\nentries where we do not free the underlying data, but reset the number\nof entries. This causes us to leak memory because `free_untracked()`\nwill not iterate over any potential entries which we still had in the\narray.\n\nFix this issue by freeing old entries. The leak is exposed by t7519, but\nplugging it alone does not make the whole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n dir.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex e3ddd5b5296..cb9782fa11f 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1056,6 +1056,8 @@ static void do_invalidate_gitignore(struct untracked_cache_dir *dir)\n {\n \tint i;\n \tdir->valid = 0;\n+\tfor (size_t i = 0; i < dir->untracked_nr; i++)\n+\t\tfree(dir->untracked[i]);\n \tdir->untracked_nr = 0;\n \tfor (i = 0; i < dir->dirs_nr; i++)\n \t\tdo_invalidate_gitignore(dir->dirs[i]);\n@@ -1083,6 +1085,8 @@ static void invalidate_directory(struct untracked_cache *uc,\n \t\tuc->dir_invalidated++;\n \n \tdir->valid = 0;\n+\tfor (size_t i = 0; i < dir->untracked_nr; i++)\n+\t\tfree(dir->untracked[i]);\n \tdir->untracked_nr = 0;\n \tfor (i = 0; i < dir->dirs_nr; i++)\n \t\tdir->dirs[i]->recurse = 0;\n@@ -3573,6 +3577,8 @@ static void write_one_dir(struct untracked_cache_dir *untracked,\n \t * for safety..\n \t */\n \tif (!untracked->valid) {\n+\t\tfor (size_t i = 0; i < untracked->untracked_nr; i++)\n+\t\t\tfree(untracked->untracked[i]);\n \t\tuntracked->untracked_nr = 0;\n \t\tuntracked->check_only = 0;\n \t}\n@@ -3905,6 +3911,8 @@ static void invalidate_one_directory(struct untracked_cache *uc,\n {\n \tuc->dir_invalidated++;\n \tucd->valid = 0;\n+\tfor (size_t i = 0; i < ucd->untracked_nr; i++)\n+\t\tfree(ucd->untracked[i]);\n \tucd->untracked_nr = 0;\n }\n \n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506624","messageId":"76e9a6d57927cb8e7363b3af20d9b194ab258b21.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 17/22] sparse-index: correctly free EWAH contents","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:38Z","receivedAt":"2024-11-05T06:17:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While we free the `fsmonitor_dirty` member of `struct index_state`, we\ndo not free the contents of that EWAH. Do so by using `ewah_free()`\ninstead of `FREE_AND_NULL()`.\n\nThis leak is exposed by t7519, but plugging it alone does not make the\ntest suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n sparse-index.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 3d7f2164e25..2107840bfc5 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -2,6 +2,7 @@\n \n #include \"git-compat-util.h\"\n #include \"environment.h\"\n+#include \"ewah/ewok.h\"\n #include \"gettext.h\"\n #include \"name-hash.h\"\n #include \"read-cache-ll.h\"\n@@ -242,7 +243,8 @@ int convert_to_sparse(struct index_state *istate, int flags)\n \tcache_tree_update(istate, 0);\n \n \tistate->fsmonitor_has_run_once = 0;\n-\tFREE_AND_NULL(istate->fsmonitor_dirty);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n \tFREE_AND_NULL(istate->fsmonitor_last_update);\n \n \tistate->sparse_index = INDEX_COLLAPSED;\n@@ -438,7 +440,8 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)\n \tistate->cache_nr = full->cache_nr;\n \tistate->cache_alloc = full->cache_alloc;\n \tistate->fsmonitor_has_run_once = 0;\n-\tFREE_AND_NULL(istate->fsmonitor_dirty);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n \tFREE_AND_NULL(istate->fsmonitor_last_update);\n \n \tstrbuf_release(&base);\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506625","messageId":"70f16486d77ec69874ee4f36cc1d89631deae3fd.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 18/22] t/helper: stop re-initialization of `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:40Z","receivedAt":"2024-11-05T06:17:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While \"common-main.c\" already initializes `the_repository` for us, we do\nso a second time in the \"read-cache\" test helper. This causes a memory\nleak because the old repository's contents isn't released.\n\nStop calling `initialize_repository()` to plug this leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-read-cache.c | 2 --\n t/t7519-status-fsmonitor.sh | 1 +\n 2 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex d285c656bd3..e277dde8e71 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -11,8 +11,6 @@ int cmd__read_cache(int argc, const char **argv)\n \tint i, cnt = 1;\n \tconst char *name = NULL;\n \n-\tinitialize_repository(the_repository);\n-\n \tif (argc > 1 && skip_prefix(argv[1], \"--print-and-refresh=\", &name)) {\n \t\targc--;\n \t\targv++;\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 7ee69ecdd4a..0f88a58a819 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -2,6 +2,7 @@\n \n test_description='git status with file system watcher'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Note, after \"git reset --hard HEAD\" no extensions exist other than 'TREE'\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506626","messageId":"f056d31ca50a739c64b142f4f0b455326c283c14.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 19/22] t/helper: fix leaking buffer in \"dump-untracked-cache\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:43Z","receivedAt":"2024-11-05T06:17:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never release the local `struct strbuf base` buffer, thus leaking\nmemory. Fix this leak.\n\nThis leak is exposed by t7063, but plugging it alone does not make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-dump-untracked-cache.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/helper/test-dump-untracked-cache.c b/t/helper/test-dump-untracked-cache.c\nindex 4f010d53249..b2e70837a90 100644\n--- a/t/helper/test-dump-untracked-cache.c\n+++ b/t/helper/test-dump-untracked-cache.c\n@@ -68,5 +68,7 @@ int cmd__dump_untracked_cache(int ac UNUSED, const char **av UNUSED)\n \tprintf(\"flags %08x\\n\", uc->dir_flags);\n \tif (uc->root)\n \t\tdump(uc->root, &base);\n+\n+\tstrbuf_release(&base);\n \treturn 0;\n }\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506627","messageId":"714c9286e7a466f5693cb9ab70177f46d51afe04.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 20/22] dir: fix leak when parsing \"status.showUntrackedFiles\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:46Z","receivedAt":"2024-11-05T06:17:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We use `repo_config_get_string()` to read \"status.showUntrackedFiles\"\nfrom the config subsystem. This function allocates the result, but we\nnever free the result after parsing it.\n\nThe value never leaves the scope of the calling function, so refactor it\nto instead use `repo_config_get_string_tmp()`, which does not hand over\nownership to the caller.\n\nSigned-off-by: Patrick Steinhardt \n---\n dir.c | 4 ++--\n t/t7063-status-untracked-cache.sh | 1 +\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex cb9782fa11f..7f35a3e3175 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2872,14 +2872,14 @@ static void set_untracked_ident(struct untracked_cache *uc)\n static unsigned new_untracked_cache_flags(struct index_state *istate)\n {\n \tstruct repository *repo = istate->repo;\n-\tchar *val;\n+\tconst char *val;\n \n \t/*\n \t * This logic is coordinated with the setting of these flags in\n \t * wt-status.c#wt_status_collect_untracked(), and the evaluation\n \t * of the config setting in commit.c#git_status_config()\n \t */\n-\tif (!repo_config_get_string(repo, \"status.showuntrackedfiles\", &val) &&\n+\tif (!repo_config_get_string_tmp(repo, \"status.showuntrackedfiles\", &val) &&\n \t !strcmp(val, \"all\"))\n \t\treturn 0;\n \ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex 8929ef481f9..13fea7931cd 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -5,6 +5,7 @@ test_description='test untracked cache'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # On some filesystems (e.g. FreeBSD's ext2 and ufs) directory mtime\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506628","messageId":"0ff65c1213b7034c35d80796800d29de5ee74b52.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 21/22] builtin/merge: release output buffer after performing merge","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:49Z","receivedAt":"2024-11-05T06:18:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `obuf` member of `struct merge_options` is used to buffer output in\nsome cases. In order to not discard its allocated memory we only release\nits contents in `merge_finalize()` when we're not currently recursing\ninto a subtree.\n\nThis results in some situations where we seemingly do not release the\nbuffer reliably. We thus have calls to `strbuf_release()` for this\nbuffer scattered across the codebase. But we're missing one callsite in\ngit-merge(1), which causes a memory leak.\n\nWe should ideally refactor this interface so that callers don't have to\nknow about any such internals. But for now, paper over the issue by\nadding one more `strbuf_release()` call.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/merge.c | 1 +\n t/t6424-merge-unrelated-index-changes.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 84d0f3604bc..51038eaca84 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -754,6 +754,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\t\tclean = merge_recursive(&o, head, remoteheads->item,\n \t\t\t\t\t\treversed, &result);\n \t\tfree_commit_list(reversed);\n+\t\tstrbuf_release(&o.obuf);\n \n \t\tif (clean < 0) {\n \t\t\trollback_lock_file(&lock);\ndiff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh\nindex 7677c5f08d0..a7ea8acb845 100755\n--- a/t/t6424-merge-unrelated-index-changes.sh\n+++ b/t/t6424-merge-unrelated-index-changes.sh\n@@ -2,6 +2,7 @@\n \n test_description=\"merges with unrelated index changes\"\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Testcase for some simple merges\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506629","messageId":"d9e122bb5db965cdc3d86f4ba869cd51a3aca800.1730786196.git.ps@pks.im","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"[PATCH v3 22/22] list-objects-filter-options: work around reported leak on error","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:17:54Z","receivedAt":"2024-11-05T06:18:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"This one is a little bit more curious. In t6112, we have a test that\nexercises the `git rev-list --filter` option with invalid filters. We\nexecute git-rev-list(1) via `test_must_fail`, which means that we check\nfor leaks even though Git exits with an error code. This causes the\nfollowing leak:\n\n Direct leak of 27 byte(s) in 1 object(s) allocated from:\n #0 0x5555555e6946 in realloc.part.0 lsan_interceptors.cpp.o\n #1 0x5555558fb4b6 in xrealloc wrapper.c:137:8\n #2 0x5555558b6e06 in strbuf_grow strbuf.c:112:2\n #3 0x5555558b7550 in strbuf_add strbuf.c:311:2\n #4 0x5555557c1a88 in strbuf_addstr strbuf.h:310:2\n #5 0x5555557c1d4c in parse_list_objects_filter list-objects-filter-options.c:261:3\n #6 0x555555885ead in handle_revision_pseudo_opt revision.c:2899:3\n #7 0x555555884e20 in setup_revisions revision.c:3014:11\n #8 0x5555556c4b42 in cmd_rev_list builtin/rev-list.c:588:9\n #9 0x5555555ec5e3 in run_builtin git.c:483:11\n #10 0x5555555eb1e4 in handle_builtin git.c:749:13\n #11 0x5555555ec001 in run_argv git.c:819:4\n #12 0x5555555eaf94 in cmd_main git.c:954:19\n #13 0x5555556fd569 in main common-main.c:64:11\n #14 0x7ffff7ca714d in __libc_start_call_main (.../lib/libc.so.6+0x2a14d)\n #15 0x7ffff7ca7208 in __libc_start_main@GLIBC_2.2.5 (.../libc.so.6+0x2a208)\n #16 0x5555555ad064 in _start (git+0x59064)\n\nThis leak is valid, as we call `die()` and do not clean up the memory at\nall. But what's curious is that this is the only leak reported, because\nwe don't clean up any other allocated memory, either, and I have no idea\nwhy the leak sanitizer treats this buffer specially.\n\nIn any case, we can work around the leak by shuffling things around a\nbit. Instead of calling `gently_parse_list_objects_filter()` and dying\nafter we have modified the filter spec, we simply do so beforehand. Like\nthis we don't allocate the buffer in the error case, which makes the\nreported leak go away.\n\nIt's not pretty, but it manages to make t6112 leak free.\n\nSigned-off-by: Patrick Steinhardt \n---\n list-objects-filter-options.c | 17 +++++++----------\n t/t6112-rev-list-filters-objects.sh | 1 +\n 2 files changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 00611107d20..fa72e81e4ad 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -252,16 +252,14 @@ void parse_list_objects_filter(\n \tconst char *arg)\n {\n \tstruct strbuf errbuf = STRBUF_INIT;\n-\tint parse_error;\n \n \tif (!filter_options->filter_spec.buf)\n \t\tBUG(\"filter_options not properly initialized\");\n \n \tif (!filter_options->choice) {\n+\t\tif (gently_parse_list_objects_filter(filter_options, arg, &errbuf))\n+\t\t\tdie(\"%s\", errbuf.buf);\n \t\tstrbuf_addstr(&filter_options->filter_spec, arg);\n-\n-\t\tparse_error = gently_parse_list_objects_filter(\n-\t\t\tfilter_options, arg, &errbuf);\n \t} else {\n \t\tstruct list_objects_filter_options *sub;\n \n@@ -271,18 +269,17 @@ void parse_list_objects_filter(\n \t\t */\n \t\ttransform_to_combine_type(filter_options);\n \n-\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n-\t\tfilter_spec_append_urlencode(filter_options, arg);\n \t\tALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,\n \t\t\t filter_options->sub_alloc);\n \t\tsub = &filter_options->sub[filter_options->sub_nr - 1];\n \n \t\tlist_objects_filter_init(sub);\n-\t\tparse_error = gently_parse_list_objects_filter(sub, arg,\n-\t\t\t\t\t\t\t &errbuf);\n+\t\tif (gently_parse_list_objects_filter(sub, arg, &errbuf))\n+\t\t\tdie(\"%s\", errbuf.buf);\n+\n+\t\tstrbuf_addch(&filter_options->filter_spec, '+');\n+\t\tfilter_spec_append_urlencode(filter_options, arg);\n \t}\n-\tif (parse_error)\n-\t\tdie(\"%s\", errbuf.buf);\n }\n \n int opt_parse_list_objects_filter(const struct option *opt,\ndiff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\nindex 0387f35a326..71e38491fa8 100755\n--- a/t/t6112-rev-list-filters-objects.sh\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -5,6 +5,7 @@ test_description='git rev-list using object filtering'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Test the blob:none filter.\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506633","messageId":"xmqq7c9itbq9.fsf@gitster.g","threadId":"62312","inReplyTo":"cover.1730786195.git.ps@pks.im","subject":"Re: [PATCH v3 00/22] Memory leak fixes (pt.9)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-05T06:51:10Z","receivedAt":"2024-11-05T06:51:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Changes compared to v2:\n>\n> - Remove an unnecessary cast.\n>\n> - Fix a duplicate newline.\n>\n> - Polish a couple of commit messages.\n>\n> Thanks!\n\nI spot checked the ones that did not change from v2 and the ones I\nchecked at all looked sensible. Perhaps this is now ready for\n'next'?\n\nThanks.\n"},{"id":"506634","messageId":"ZynAn6VGChtZX9Wb@pks.im","threadId":"62312","inReplyTo":"xmqq7c9itbq9.fsf@gitster.g","subject":"Re: [PATCH v3 00/22] Memory leak fixes (pt.9)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-05T06:52:15Z","receivedAt":"2024-11-05T06:52:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 04, 2024 at 10:51:10PM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > Changes compared to v2:\n> >\n> > - Remove an unnecessary cast.\n> >\n> > - Fix a duplicate newline.\n> >\n> > - Polish a couple of commit messages.\n> >\n> > Thanks!\n> \n> I spot checked the ones that did not change from v2 and the ones I\n> checked at all looked sensible. Perhaps this is now ready for\n> 'next'?\n\nFrom my point of view it should be ready, yes.\n\nPatrick\n"},{"id":"506666","messageId":"audhpto5cjvo5xo77is37cycws4kasgaquv53ppiewkzuj4xjw@tkw6axaoggos","threadId":"62312","inReplyTo":"ZynAn6VGChtZX9Wb@pks.im","subject":"Re: [PATCH v3 00/22] Memory leak fixes (pt.9)","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-05T15:27:19Z","receivedAt":"2024-11-05T15:28:57Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/11/05 07:52AM, Patrick Steinhardt wrote:\n> On Mon, Nov 04, 2024 at 10:51:10PM -0800, Junio C Hamano wrote:\n> > Patrick Steinhardt writes:\n> > \n> > > Changes compared to v2:\n> > >\n> > > - Remove an unnecessary cast.\n> > >\n> > > - Fix a duplicate newline.\n> > >\n> > > - Polish a couple of commit messages.\n> > >\n> > > Thanks!\n> > \n> > I spot checked the ones that did not change from v2 and the ones I\n> > checked at all looked sensible. Perhaps this is now ready for\n> > 'next'?\n> \n> From my point of view it should be ready, yes.\n\nFrom looking at the range-diff, this version looks good to me. :)\n\n-Justin\n"}]}