{"thread":{"id":"61850","subject":"[PATCH 00/23] Memory leak fixes (pt.3)","startedAt":"2024-07-26T12:14:00Z","lastAt":"2024-08-01T17:17:29Z","messageCount":72,"participants":["Patrick Steinhardt","Karthik Nayak","Junio C Hamano","Taylor Blau","Rubén Justo"],"isPatch":true,"patchVersion":1,"patchTotal":23},"messages":[{"id":"499413","messageId":"cover.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":null,"subject":"[PATCH 00/23] Memory leak fixes (pt.3)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:13:43Z","receivedAt":"2024-07-26T12:14:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nI originally wanted to hold off with sending out this series until v2.46\nwas out. But I saw that Junio sent out some patches which are plugging\nthe same leaks as I did, so I dedcided to send it out now to avoid some\nduplicated work.\n\nThere isn't really any structure to this series, I just happened to pick\nsome random test suites that fail with the leak checker enabled and then\nfixed those. Naturally, I've also got part 4 of this series of patch\nseries in the pipeline already :) As mentioned elsewhere, I hope to get\nthe number of failing test suites to zero this year. Let's see whether\nthis is realistic.\n\nPatrick\n\nPatrick Steinhardt (23):\n builtin/replay: plug leaking `advance_name` variable\n builtin/log: fix leaking branch name when creating cover letters\n builtin/describe: fix memory leak with `--contains=`\n builtin/describe: fix leaking array when running diff-index\n builtin/describe: fix trivial memory leak when describing blob\n builtin/name-rev: fix various trivial memory leaks\n builtin/submodule--helper: fix various trivial memory leaks\n builtin/ls-remote: fix leaking `pattern` strings\n builtin/remote: fix leaking strings in `branch_list`\n builtin/remote: fix various trivial memory leaks\n builtin/stash: fix various trivial memory leaks\n builtin/rev-parse: fix memory leak with `--parseopt`\n builtin/show-branch: fix several memory leaks\n builtin/credential-store: fix leaking credential\n builtin/rerere: fix various trivial memory leaks\n builtin/shortlog: fix various trivial memory leaks\n builtin/worktree: fix leaking derived branch names\n builtin/credential-cache: fix trivial leaks\n t/test-repository: fix leaking repository\n object-name: fix leaking commit list items\n entry: fix leaking pathnames during delayed checkout\n convert: fix leaking config strings\n commit-reach: fix trivial memory leak when computing reachability\n\n builtin/credential-cache.c | 9 ++++-\n builtin/credential-store.c | 1 +\n builtin/describe.c | 25 ++++++++++--\n builtin/log.c | 4 +-\n builtin/ls-remote.c | 11 ++++--\n builtin/name-rev.c | 6 ++-\n builtin/remote.c | 44 ++++++++++++++++-----\n builtin/replay.c | 20 +++++++---\n builtin/rerere.c | 8 +++-\n builtin/rev-parse.c | 5 ++-\n builtin/shortlog.c | 1 +\n builtin/show-branch.c | 52 +++++++++++++++++--------\n builtin/stash.c | 18 ++++++++-\n builtin/submodule--helper.c | 13 +++++--\n builtin/worktree.c | 7 ++--\n commit-reach.c | 1 +\n convert.c | 14 +++++--\n entry.c | 4 +-\n object-name.c | 26 ++++++++-----\n rerere.c | 9 ++++-\n t/helper/test-repository.c | 4 +-\n t/t0021-conversion.sh | 1 +\n t/t0301-credential-cache.sh | 2 +\n t/t0302-credential-store.sh | 2 +\n t/t0303-credential-external.sh | 1 +\n t/t1502-rev-parse-parseopt.sh | 2 +\n t/t1511-rev-parse-caret.sh | 1 +\n t/t2030-unresolve-info.sh | 1 +\n t/t2080-parallel-checkout-basics.sh | 1 +\n t/t2082-parallel-checkout-attributes.sh | 1 +\n t/t2400-worktree-add.sh | 1 +\n t/t2501-cwd-empty.sh | 1 +\n t/t3201-branch-contains.sh | 1 +\n t/t3202-show-branch.sh | 1 +\n t/t3206-range-diff.sh | 1 +\n t/t3650-replay-basics.sh | 1 +\n t/t3903-stash.sh | 1 +\n t/t3904-stash-patch.sh | 2 +\n t/t3905-stash-include-untracked.sh | 1 +\n t/t4200-rerere.sh | 1 +\n t/t4201-shortlog.sh | 1 +\n t/t5318-commit-graph.sh | 2 +\n t/t5512-ls-remote.sh | 1 +\n t/t5514-fetch-multiple.sh | 1 +\n t/t5520-pull.sh | 1 +\n t/t5528-push-default.sh | 1 +\n t/t5535-fetch-push-symref.sh | 1 +\n t/t5543-atomic-push.sh | 1 +\n t/t5570-git-daemon.sh | 1 +\n t/t6007-rev-list-cherry-pick-file.sh | 1 +\n t/t6010-merge-base.sh | 1 +\n t/t6120-describe.sh | 1 +\n t/t6133-pathspec-rev-dwim.sh | 2 +\n t/t7064-wtstatus-pv2.sh | 1 +\n t/t7400-submodule-basic.sh | 1 +\n t/t9902-completion.sh | 1 +\n t/t9903-bash-prompt.sh | 1 +\n 57 files changed, 251 insertions(+), 73 deletions(-)\n\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499414","messageId":"dd044eacc2b20c3e426f5825fdc30f4db6618052.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 01/23] builtin/replay: plug leaking `advance_name` variable","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:13:59Z","receivedAt":"2024-07-26T12:14:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `advance_name` variable can either contain a static string when\nparsed via the `--advance` command line option or it may be an allocated\nstring when set via `determine_replay_mode()`. Because we cannot be sure\nwhether it is allocated or not we just didn't free it at all, resulting\nin a memory leak.\n\nSplit up the variables such that we can track the static and allocated\nstrings separately and then free the allocated one to fix the memory\nleak.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/replay.c | 20 ++++++++++++++------\n t/t3650-replay-basics.sh | 1 +\n 2 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 0448326636..27b40eda98 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -151,7 +151,7 @@ static void get_ref_information(struct rev_cmdline_info *cmd_info,\n \n static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\t\t\t const char *onto_name,\n-\t\t\t\t const char **advance_name,\n+\t\t\t\t char **advance_name,\n \t\t\t\t struct commit **onto,\n \t\t\t\t struct strset **update_refs)\n {\n@@ -174,6 +174,7 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\t*onto = peel_committish(*advance_name);\n \t\tif (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name),\n \t\t\t &oid, &fullname, 0) == 1) {\n+\t\t\tfree(*advance_name);\n \t\t\t*advance_name = fullname;\n \t\t} else {\n \t\t\tdie(_(\"argument to --advance must be a reference\"));\n@@ -197,6 +198,7 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\tif (negative_refs_complete) {\n \t\t\tstruct hashmap_iter iter;\n \t\t\tstruct strmap_entry *entry;\n+\t\t\tconst char *last_key = NULL;\n \n \t\t\tif (rinfo.negative_refexprs == 0)\n \t\t\t\tdie(_(\"all positive revisions given must be references\"));\n@@ -208,8 +210,11 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\t\t/* Only one entry, but we have to loop to get it */\n \t\t\tstrset_for_each_entry(&rinfo.negative_refs,\n \t\t\t\t\t &iter, entry) {\n-\t\t\t\t*advance_name = entry->key;\n+\t\t\t\tlast_key = entry->key;\n \t\t\t}\n+\n+\t\t\tfree(*advance_name);\n+\t\t\t*advance_name = xstrdup_or_null(last_key);\n \t\t} else { /* positive_refs_complete */\n \t\t\tif (rinfo.negative_refexprs > 1)\n \t\t\t\tdie(_(\"cannot implicitly determine correct base for --onto\"));\n@@ -271,7 +276,8 @@ static struct commit *pick_regular_commit(struct commit *pickme,\n \n int cmd_replay(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *advance_name = NULL;\n+\tconst char *advance_name_opt = NULL;\n+\tchar *advance_name = NULL;\n \tstruct commit *onto = NULL;\n \tconst char *onto_name = NULL;\n \tint contained = 0;\n@@ -292,7 +298,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)\n \t\tNULL\n \t};\n \tstruct option replay_options[] = {\n-\t\tOPT_STRING(0, \"advance\", &advance_name,\n+\t\tOPT_STRING(0, \"advance\", &advance_name_opt,\n \t\t\t N_(\"branch\"),\n \t\t\t N_(\"make replay advance given branch\")),\n \t\tOPT_STRING(0, \"onto\", &onto_name,\n@@ -306,14 +312,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, replay_options, replay_usage,\n \t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);\n \n-\tif (!onto_name && !advance_name) {\n+\tif (!onto_name && !advance_name_opt) {\n \t\terror(_(\"option --onto or --advance is mandatory\"));\n \t\tusage_with_options(replay_usage, replay_options);\n \t}\n \n-\tif (advance_name && contained)\n+\tif (advance_name_opt && contained)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n \t\t \"--advance\", \"--contained\");\n+\tadvance_name = xstrdup_or_null(advance_name_opt);\n \n \trepo_init_revisions(the_repository, &revs, prefix);\n \n@@ -441,6 +448,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)\n \n cleanup:\n \trelease_revisions(&revs);\n+\tfree(advance_name);\n \n \t/* Return */\n \tif (ret < 0)\ndiff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh\nindex 389670262e..12bd3db4cb 100755\n--- a/t/t3650-replay-basics.sh\n+++ b/t/t3650-replay-basics.sh\n@@ -5,6 +5,7 @@ test_description='basic git replay 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 \n GIT_AUTHOR_NAME=author@name\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499415","messageId":"4daae88877f2355d6a3960883e7b3f15cf738cb5.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 02/23] builtin/log: fix leaking branch name when creating cover letters","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:14:10Z","receivedAt":"2024-07-26T12:14:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `make_cover_letter()` without a branch name, then we try to\nderive the branch name by calling `find_branch_name()`. But while this\nfunction returns an allocated string, we never free the result and thus\nhave a memory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/log.c | 4 +++-\n t/t3206-range-diff.sh | 1 +\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 4d4b60caa7..a73a767606 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1434,6 +1434,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \tint need_8bit_cte = 0;\n \tstruct pretty_print_context pp = {0};\n \tstruct commit *head = list[0];\n+\tchar *to_free = NULL;\n \n \tif (!cmit_fmt_is_mail(rev->commit_format))\n \t\tdie(_(\"cover letter needs email format\"));\n@@ -1455,7 +1456,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t}\n \n \tif (!branch_name)\n-\t\tbranch_name = find_branch_name(rev);\n+\t\tbranch_name = to_free = find_branch_name(rev);\n \n \tpp.fmt = CMIT_FMT_EMAIL;\n \tpp.date_mode.type = DATE_RFC2822;\n@@ -1466,6 +1467,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t encoding, need_8bit_cte, cfg);\n \tfprintf(rev->diffopt.file, \"%s\\n\", sb.buf);\n \n+\tfree(to_free);\n \tfree(pp.after_subject);\n \tstrbuf_release(&sb);\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex a767c3520e..973e20254b 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -5,6 +5,7 @@ test_description='range-diff 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 \n # Note that because of the range-diff's heuristics, test_commit does more\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499416","messageId":"08a12be13c2fed247d6086967e7a3f03fa6519e1.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 03/23] builtin/describe: fix memory leak with `--contains=`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:14:18Z","receivedAt":"2024-07-26T12:14:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `git describe --contains=`, we end up invoking\n`cmd_name_rev()` with some munged argv array. This array may contain\nallocated strings and furthermore will likely be modified by the called\nfunction. This results in two memory leaks:\n\n - First, we leak the array that we use to assemble the arguments.\n\n - Second, we leak the allocated strings that we may have put into the\n array.\n\nFix those leaks by creating a separate copy of the array that we can\nhand over to `cmd_name_rev()`. This allows us to free all strings\ncontained in the `strvec`, as the original vector will not be modified\nanymore.\n\nFurthermore, free both the `strvec` and the copied array to fix the\nfirst memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/describe.c | 18 +++++++++++++++++-\n 1 file changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex cf8edc4222..4c0980c675 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -619,6 +619,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \tif (contains) {\n \t\tstruct string_list_item *item;\n \t\tstruct strvec args;\n+\t\tconst char **argv_copy;\n+\t\tint ret;\n \n \t\tstrvec_init(&args);\n \t\tstrvec_pushl(&args, \"name-rev\",\n@@ -637,7 +639,21 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\tstrvec_pushv(&args, argv);\n \t\telse\n \t\t\tstrvec_push(&args, \"HEAD\");\n-\t\treturn cmd_name_rev(args.nr, args.v, prefix);\n+\n+\t\t/*\n+\t\t * `cmd_name_rev()` modifies the array, so we'd link its\n+\t\t * contained strings if we didn't do a copy here.\n+\t\t */\n+\t\tALLOC_ARRAY(argv_copy, args.nr + 1);\n+\t\tfor (size_t i = 0; i < args.nr; i++)\n+\t\t\targv_copy[i] = args.v[i];\n+\t\targv_copy[args.nr] = NULL;\n+\n+\t\tret = cmd_name_rev(args.nr, argv_copy, prefix);\n+\n+\t\tstrvec_clear(&args);\n+\t\tfree(argv_copy);\n+\t\treturn ret;\n \t}\n \n \thashmap_init(&names, commit_name_neq, NULL, 0);\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499417","messageId":"e8d86c01cf1316fabcec7dfacad55e1dc98baa7d.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 04/23] builtin/describe: fix leaking array when running diff-index","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:14:33Z","receivedAt":"2024-07-26T12:14:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When running git-describe(1) with `--dirty`, we will set up a `struct\nrev_info` with arguments for git-diff-index(1). The way we assmble the\narguments it causes two memory leaks though:\n\n - We never release the `struct strvec`.\n\n - `setup_revisions()` may end up removing some entries from the\n `strvec`, which we wouldn't free even if we released the struct.\n\nWhile we could plug those leaks, this is ultimately unnecessary as the\narguments we pass are part of a static array anyway. So instead,\nrefactor the code to drop the `struct strvec` and just pass this static\narray directly.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/describe.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 4c0980c675..3b61aa1baa 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -695,7 +695,6 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t} else if (dirty) {\n \t\t\tstruct lock_file index_lock = LOCK_INIT;\n \t\t\tstruct rev_info revs;\n-\t\t\tstruct strvec args = STRVEC_INIT;\n \t\t\tint fd;\n \n \t\t\tsetup_work_tree();\n@@ -710,8 +709,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\t\trepo_update_index_if_able(the_repository, &index_lock);\n \n \t\t\trepo_init_revisions(the_repository, &revs, prefix);\n-\t\t\tstrvec_pushv(&args, diff_index_args);\n-\t\t\tif (setup_revisions(args.nr, args.v, &revs, NULL) != 1)\n+\n+\t\t\tif (setup_revisions(ARRAY_SIZE(diff_index_args) - 1,\n+\t\t\t\t\t diff_index_args, &revs, NULL) != 1)\n \t\t\t\tBUG(\"malformed internal diff-index command line\");\n \t\t\trun_diff_index(&revs, 0);\n \n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499418","messageId":"8aba7434bd3350ea98ad055c3f0acd2045b96960.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 05/23] builtin/describe: fix trivial memory leak when describing blob","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:14:48Z","receivedAt":"2024-07-26T12:14:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never free the `struct strvec args` variable in `describe_blob()`,\nwhich thus causes a memory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/describe.c | 1 +\n t/t9903-bash-prompt.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 3b61aa1baa..aa5ec8d68a 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -529,6 +529,7 @@ static void describe_blob(struct object_id oid, struct strbuf *dst)\n \ttraverse_commit_list(&revs, process_commit, process_object, &pcd);\n \treset_revision_walk();\n \trelease_revisions(&revs);\n+\tstrvec_clear(&args);\n }\n \n static void describe(const char *arg, int last_one)\ndiff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh\nindex d667dda654..95e9955bca 100755\n--- a/t/t9903-bash-prompt.sh\n+++ b/t/t9903-bash-prompt.sh\n@@ -8,6 +8,7 @@ test_description='test git-specific bash prompt functions'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./lib-bash.sh\n \n . \"$GIT_BUILD_DIR/contrib/completion/git-prompt.sh\"\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499419","messageId":"088f730572f0f5aaa8f9640ea35a4687be9094ba.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 06/23] builtin/name-rev: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:14:55Z","receivedAt":"2024-07-26T12:15:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several structures that we don't release after\n`cmd_name_rev()` is done. Plug those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/name-rev.c | 6 ++++--\n t/t6007-rev-list-cherry-pick-file.sh | 1 +\n t/t6120-describe.sh | 1 +\n 3 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 70e9ec4e47..f62c0a36cb 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -677,7 +677,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t\t\t\t always, allow_undefined, data.name_only);\n \t}\n \n-\tUNLEAK(string_pool);\n-\tUNLEAK(revs);\n+\tstring_list_clear(&data.ref_filters, 0);\n+\tstring_list_clear(&data.exclude_filters, 0);\n+\tmem_pool_discard(&string_pool, 0);\n+\tobject_array_clear(&revs);\n \treturn 0;\n }\ndiff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh\nindex 6f3e543977..2d337d7287 100755\n--- a/t/t6007-rev-list-cherry-pick-file.sh\n+++ b/t/t6007-rev-list-cherry-pick-file.sh\n@@ -5,6 +5,7 @@ test_description='test git rev-list --cherry-pick -- file'\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 # A---B---D---F\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 79e0f19deb..05ed2510d9 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -14,6 +14,7 @@ test_description='test describe'\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 check_describe () {\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499420","messageId":"5220c91bda7bc766368c2925499e5d244a03697b.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 07/23] builtin/submodule--helper: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:15:09Z","receivedAt":"2024-07-26T12:15:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are multiple trivial memory leaks in the submodule helper. Fix\nthose.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/submodule--helper.c | 13 ++++++++++---\n t/t7400-submodule-basic.sh | 1 +\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f1218a1995..5ae06c3e0b 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2269,6 +2269,7 @@ static int is_tip_reachable(const char *path, const struct object_id *oid)\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf rev = STRBUF_INIT;\n \tchar *hex = oid_to_hex(oid);\n+\tint reachable;\n \n \tcp.git_cmd = 1;\n \tcp.dir = path;\n@@ -2278,9 +2279,12 @@ static int is_tip_reachable(const char *path, const struct object_id *oid)\n \tprepare_submodule_repo_env(&cp.env);\n \n \tif (capture_command(&cp, &rev, GIT_MAX_HEXSZ + 1) || rev.len)\n-\t\treturn 0;\n+\t\treachable = 0;\n+\telse\n+\t\treachable = 1;\n \n-\treturn 1;\n+\tstrbuf_release(&rev);\n+\treturn reachable;\n }\n \n static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n@@ -3135,6 +3139,7 @@ static void append_fetch_remotes(struct strbuf *msg, const char *git_dir_path)\n static int add_submodule(const struct add_data *add_data)\n {\n \tchar *submod_gitdir_path;\n+\tchar *depth_to_free = NULL;\n \tstruct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;\n \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n \tint ret = -1;\n@@ -3200,7 +3205,7 @@ static int add_submodule(const struct add_data *add_data)\n \t\t}\n \t\tclone_data.dissociate = add_data->dissociate;\n \t\tif (add_data->depth >= 0)\n-\t\t\tclone_data.depth = xstrfmt(\"%d\", add_data->depth);\n+\t\t\tclone_data.depth = depth_to_free = xstrfmt(\"%d\", add_data->depth);\n \n \t\tif (clone_submodule(&clone_data, &reference))\n \t\t\tgoto cleanup;\n@@ -3223,7 +3228,9 @@ static int add_submodule(const struct add_data *add_data)\n \t\t\tdie(_(\"unable to checkout submodule '%s'\"), add_data->sm_path);\n \t}\n \tret = 0;\n+\n cleanup:\n+\tfree(depth_to_free);\n \tstring_list_clear(&reference, 1);\n \treturn ret;\n }\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 981488885f..098d8833b6 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -12,6 +12,7 @@ subcommands of git submodule.\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_expect_success 'setup - enable local submodules' '\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499421","messageId":"d42152654bf91e90b8b417316f255746a3a75155.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 08/23] builtin/ls-remote: fix leaking `pattern` strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:15:24Z","receivedAt":"2024-07-26T12:15:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Users can pass patterns to git-ls-remote(1), which allows them to filter\nthe list of printed references. We assemble those patterns into an array\nand prefix them with \"*/\", but never free either the array nor the\nallocated strings.\n\nFix those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/ls-remote.c | 11 +++++++----\n t/t5535-fetch-push-symref.sh | 1 +\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex debf2d4f88..500f76fe4c 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -47,7 +47,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tint status = 0;\n \tint show_symref_target = 0;\n \tconst char *uploadpack = NULL;\n-\tconst char **pattern = NULL;\n+\tchar **pattern = NULL;\n \tstruct transport_ls_refs_options transport_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tint i;\n@@ -96,9 +96,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tif (argc > 1) {\n \t\tint i;\n \t\tCALLOC_ARRAY(pattern, argc);\n-\t\tfor (i = 1; i < argc; i++) {\n+\t\tfor (i = 1; i < argc; i++)\n \t\t\tpattern[i - 1] = xstrfmt(\"*/%s\", argv[i]);\n-\t\t}\n \t}\n \n \tif (flags & REF_TAGS)\n@@ -136,7 +135,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tstruct ref_array_item *item;\n \t\tif (!check_ref_type(ref, flags))\n \t\t\tcontinue;\n-\t\tif (!tail_match(pattern, ref->name))\n+\t\tif (!tail_match((const char **) pattern, ref->name))\n \t\t\tcontinue;\n \t\titem = ref_array_push(&ref_array, ref->name, &ref->old_oid);\n \t\titem->symref = xstrdup_or_null(ref->symref);\n@@ -158,5 +157,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tif (transport_disconnect(transport))\n \t\tstatus = 1;\n \ttransport_ls_refs_options_release(&transport_options);\n+\n+\tfor (i = 1; i < argc; i++)\n+\t\tfree(pattern[i - 1]);\n+\tfree(pattern);\n \treturn status;\n }\ndiff --git a/t/t5535-fetch-push-symref.sh b/t/t5535-fetch-push-symref.sh\nindex e8f6d233ff..7122af7fdb 100755\n--- a/t/t5535-fetch-push-symref.sh\n+++ b/t/t5535-fetch-push-symref.sh\n@@ -2,6 +2,7 @@\n \n test_description='avoiding conflicting update through symref aliasing'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499422","messageId":"6952fb2ff2cf453de39b63883a716a9f09cab7b7.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 09/23] builtin/remote: fix leaking strings in `branch_list`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:15:36Z","receivedAt":"2024-07-26T12:15:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct string_list branch_list` is declared as `NODUP`, which makes\nit not copy strings inserted into it. This causes memory leaks though,\nas this means it also won't be responsible for _freeing_ inserted\nstrings. Thus, every branch we add to this will leak.\n\nFix this by marking the list as `DUP` instead and free the local copy we\nhave of the variable.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/remote.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 08292498bd..303da7f73f 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -258,7 +258,7 @@ struct branch_info {\n \tchar *push_remote_name;\n };\n \n-static struct string_list branch_list = STRING_LIST_INIT_NODUP;\n+static struct string_list branch_list = STRING_LIST_INIT_DUP;\n \n static const char *abbrev_ref(const char *name, const char *prefix)\n {\n@@ -292,8 +292,8 @@ static int config_read_branches(const char *key, const char *value,\n \t\ttype = PUSH_REMOTE;\n \telse\n \t\treturn 0;\n-\tname = xmemdupz(key, key_len);\n \n+\tname = xmemdupz(key, key_len);\n \titem = string_list_insert(&branch_list, name);\n \n \tif (!item->util)\n@@ -337,6 +337,7 @@ static int config_read_branches(const char *key, const char *value,\n \t\tBUG(\"unexpected type=%d\", type);\n \t}\n \n+\tfree(name);\n \treturn 0;\n }\n \n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499423","messageId":"8bedcfdad80c19dbb98e1407e4cef985a066c714.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 10/23] builtin/remote: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:15:49Z","receivedAt":"2024-07-26T12:16:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are multiple trivial memory leaks in git-remote(1). Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/remote.c | 39 ++++++++++++++++++++++++++++++++-------\n t/t5512-ls-remote.sh | 1 +\n t/t5514-fetch-multiple.sh | 1 +\n t/t5520-pull.sh | 1 +\n t/t5528-push-default.sh | 1 +\n t/t5543-atomic-push.sh | 1 +\n t/t5570-git-daemon.sh | 1 +\n 7 files changed, 38 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 303da7f73f..9d54fddf8c 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -555,13 +555,16 @@ static int add_branch_for_removal(const char *refname,\n \trefspec.dst = (char *)refname;\n \tif (remote_find_tracking(branches->remote, &refspec))\n \t\treturn 0;\n+\tfree(refspec.src);\n \n \t/* don't delete a branch if another remote also uses it */\n \tfor (kr = branches->keep->list; kr; kr = kr->next) {\n \t\tmemset(&refspec, 0, sizeof(refspec));\n \t\trefspec.dst = (char *)refname;\n-\t\tif (!remote_find_tracking(kr->remote, &refspec))\n+\t\tif (!remote_find_tracking(kr->remote, &refspec)) {\n+\t\t\tfree(refspec.src);\n \t\t\treturn 0;\n+\t\t}\n \t}\n \n \t/* don't delete non-remote-tracking refs */\n@@ -668,7 +671,11 @@ static int config_read_push_default(const char *key, const char *value,\n static void handle_push_default(const char* old_name, const char* new_name)\n {\n \tstruct push_default_info push_default = {\n-\t\told_name, CONFIG_SCOPE_UNKNOWN, STRBUF_INIT, -1 };\n+\t\t.old_name = old_name,\n+\t\t.scope = CONFIG_SCOPE_UNKNOWN,\n+\t\t.origin = STRBUF_INIT,\n+\t\t.linenr = -1,\n+\t};\n \tgit_config(config_read_push_default, &push_default);\n \tif (push_default.scope >= CONFIG_SCOPE_COMMAND)\n \t\t; /* pass */\n@@ -688,6 +695,8 @@ static void handle_push_default(const char* old_name, const char* new_name)\n \t\t\tpush_default.origin.buf, push_default.linenr,\n \t\t\told_name);\n \t}\n+\n+\tstrbuf_release(&push_default.origin);\n }\n \n \n@@ -785,7 +794,7 @@ static int mv(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (!refspec_updated)\n-\t\treturn 0;\n+\t\tgoto out;\n \n \t/*\n \t * First remove symrefs, then rename the rest, finally create\n@@ -851,10 +860,15 @@ static int mv(int argc, const char **argv, const char *prefix)\n \t\tdisplay_progress(progress, ++refs_renamed_nr);\n \t}\n \tstop_progress(&progress);\n-\tstring_list_clear(&remote_branches, 1);\n \n \thandle_push_default(rename.old_name, rename.new_name);\n \n+out:\n+\tstring_list_clear(&remote_branches, 1);\n+\tstrbuf_release(&old_remote_context);\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&buf2);\n+\tstrbuf_release(&buf3);\n \treturn 0;\n }\n \n@@ -945,12 +959,21 @@ static int rm(int argc, const char **argv, const char *prefix)\n \n \tif (!result) {\n \t\tstrbuf_addf(&buf, \"remote.%s\", remote->name);\n-\t\tif (git_config_rename_section(buf.buf, NULL) < 1)\n-\t\t\treturn error(_(\"Could not remove config section '%s'\"), buf.buf);\n+\t\tif (git_config_rename_section(buf.buf, NULL) < 1) {\n+\t\t\tresult = error(_(\"Could not remove config section '%s'\"), buf.buf);\n+\t\t\tgoto out;\n+\t\t}\n \n \t\thandle_push_default(remote->name, NULL);\n \t}\n \n+out:\n+\tfor (struct known_remote *r = known_remotes.list; r;) {\n+\t\tstruct known_remote *next = r->next;\n+\t\tfree(r);\n+\t\tr = next;\n+\t}\n+\tstrbuf_release(&buf);\n \treturn result;\n }\n \n@@ -983,8 +1006,10 @@ static int append_ref_to_tracked_list(const char *refname,\n \n \tmemset(&refspec, 0, sizeof(refspec));\n \trefspec.dst = (char *)refname;\n-\tif (!remote_find_tracking(states->remote, &refspec))\n+\tif (!remote_find_tracking(states->remote, &refspec)) {\n \t\tstring_list_append(&states->tracked, abbrev_branch(refspec.src));\n+\t\tfree(refspec.src);\n+\t}\n \n \treturn 0;\n }\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 42e77eb5a9..d687d824d1 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -5,6 +5,7 @@ test_description='git ls-remote'\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 generate_references () {\ndiff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh\nindex 25772c85c5..579872c258 100755\n--- a/t/t5514-fetch-multiple.sh\n+++ b/t/t5514-fetch-multiple.sh\n@@ -5,6 +5,7 @@ test_description='fetch --all works correctly'\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 setup_repository () {\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 47534f1062..1098cbd0a1 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -5,6 +5,7 @@ test_description='pulling into void'\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 modify () {\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 14f7eced9a..bc2bada34c 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -4,6 +4,7 @@ test_description='check various push.default settings'\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_expect_success 'setup bare remotes' '\ndiff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh\nindex 04b47ad84a..479d103469 100755\n--- a/t/t5543-atomic-push.sh\n+++ b/t/t5543-atomic-push.sh\n@@ -5,6 +5,7 @@ test_description='pushing to a repository using the atomic push option'\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 mk_repo_pair () {\ndiff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh\nindex f9a9bf9503..c5f08b6799 100755\n--- a/t/t5570-git-daemon.sh\n+++ b/t/t5570-git-daemon.sh\n@@ -4,6 +4,7 @@ test_description='test fetching over git protocol'\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_DIRECTORY\"/lib-git-daemon.sh\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499424","messageId":"a4b3ca49c9724b79ca695d0db8e45be663a46434.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 11/23] builtin/stash: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:16:12Z","receivedAt":"2024-07-26T12:16:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are multiple trivial memory leaks in git-stash(1). Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/stash.c | 18 ++++++++++++++++--\n t/t2501-cwd-empty.sh | 1 +\n t/t3903-stash.sh | 1 +\n t/t3904-stash-patch.sh | 2 ++\n t/t3905-stash-include-untracked.sh | 1 +\n t/t7064-wtstatus-pv2.sh | 1 +\n 6 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 46b981c4dd..7f2f989b69 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1521,6 +1521,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \tstruct strbuf patch = STRBUF_INIT;\n \tstruct strbuf stash_msg_buf = STRBUF_INIT;\n \tstruct strbuf untracked_files = STRBUF_INIT;\n+\tstruct strbuf out = STRBUF_INIT;\n \n \tif (patch_mode && keep_index == -1)\n \t\tkeep_index = 1;\n@@ -1626,7 +1627,6 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \t\t\tstruct child_process cp_add = CHILD_PROCESS_INIT;\n \t\t\tstruct child_process cp_diff = CHILD_PROCESS_INIT;\n \t\t\tstruct child_process cp_apply = CHILD_PROCESS_INIT;\n-\t\t\tstruct strbuf out = STRBUF_INIT;\n \n \t\t\tcp_add.git_cmd = 1;\n \t\t\tstrvec_push(&cp_add.args, \"add\");\n@@ -1718,6 +1718,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \n done:\n \tstrbuf_release(&patch);\n+\tstrbuf_release(&out);\n \tfree_stash_info(&info);\n \tstrbuf_release(&stash_msg_buf);\n \tstrbuf_release(&untracked_files);\n@@ -1869,6 +1870,8 @@ int cmd_stash(int argc, const char **argv, const char *prefix)\n \t\tOPT_SUBCOMMAND_F(\"save\", &fn, save_stash, PARSE_OPT_NOCOMPLETE),\n \t\tOPT_END()\n \t};\n+\tconst char **args_copy;\n+\tint ret;\n \n \tgit_config(git_stash_config, NULL);\n \n@@ -1892,5 +1895,16 @@ int cmd_stash(int argc, const char **argv, const char *prefix)\n \t/* Assume 'stash push' */\n \tstrvec_push(&args, \"push\");\n \tstrvec_pushv(&args, argv);\n-\treturn !!push_stash(args.nr, args.v, prefix, 1);\n+\n+\t/*\n+\t * `push_stash()` ends up modifying the array, which causes memory\n+\t * leaks if we didn't copy the array here.\n+\t */\n+\tDUP_ARRAY(args_copy, args.v, args.nr);\n+\n+\tret = !!push_stash(args.nr, args_copy, prefix, 1);\n+\n+\tstrvec_clear(&args);\n+\tfree(args_copy);\n+\treturn ret;\n }\ndiff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh\nindex f6d8d7d03d..8af4e8cfe3 100755\n--- a/t/t2501-cwd-empty.sh\n+++ b/t/t2501-cwd-empty.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test handling of the current working directory becoming empty'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex a7f71f8126..e4c0937f61 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -8,6 +8,7 @@ test_description='Test git stash'\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-unique-files.sh\n \ndiff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\nindex 368fc2a6cc..aa5019fd6c 100755\n--- a/t/t3904-stash-patch.sh\n+++ b/t/t3904-stash-patch.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='stash -p'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./lib-patch-mode.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 1289ae3e07..a1733f45c3 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -5,6 +5,7 @@\n \n test_description='Test git stash --include-untracked'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'stash save --include-untracked some dirty working directory' '\ndiff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh\nindex 11884d2fc3..06c1301222 100755\n--- a/t/t7064-wtstatus-pv2.sh\n+++ b/t/t7064-wtstatus-pv2.sh\n@@ -4,6 +4,7 @@ test_description='git status --porcelain=v2\n \n This test exercises porcelain V2 output for git status.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n \n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499425","messageId":"e277222bd298560b0d7f10cb9b5d0eee5967ba87.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 12/23] builtin/rev-parse: fix memory leak with `--parseopt`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:16:27Z","receivedAt":"2024-07-26T12:16:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `--parseopt` mode allows shell scripts to have the same option\nparsing mode as we have in C builtins. It soaks up a set of option\ndescriptions via stdin and massages them into proper `struct option`s\nthat we can then use to parse a set of arguments.\n\nWe only partially free those options when done though, creating a memory\nleak. Interestingly, we only end up free'ing the first option's help,\nwhich is of course wrong.\n\nFix this by freeing all option's help fields as well as their `argh`\nfielids to plug this memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/rev-parse.c | 5 ++++-\n t/t1502-rev-parse-parseopt.sh | 2 ++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 2e64f5bda7..5845d3f59b 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -553,7 +553,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&sb);\n \tstrvec_clear(&longnames);\n \tstrvec_clear(&usage);\n-\tfree((char *) opts->help);\n+\tfor (size_t i = 0; i < opts_nr; i++) {\n+\t\tfree((char *) opts[i].help);\n+\t\tfree((char *) opts[i].argh);\n+\t}\n \tfree(opts);\n \treturn 0;\n }\ndiff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh\nindex b754b9fd74..5eaa6428c4 100755\n--- a/t/t1502-rev-parse-parseopt.sh\n+++ b/t/t1502-rev-parse-parseopt.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test git rev-parse --parseopt'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n check_invalid_long_option () {\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499426","messageId":"fd857efa9f95d5adfc237b7b21a922b6e55c2865.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 13/23] builtin/show-branch: fix several memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:16:51Z","receivedAt":"2024-07-26T12:17:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several memory leaks in git-show-branch(1). Fix them.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/show-branch.c | 52 +++++++++++++++++++++++++++++-------------\n t/t3202-show-branch.sh | 1 +\n t/t6010-merge-base.sh | 1 +\n 3 files changed, 38 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex d72f4cb98d..7d797a880c 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -502,14 +502,14 @@ static int rev_is_head(const char *head, const char *name)\n \treturn !strcmp(head, name);\n }\n \n-static int show_merge_base(struct commit_list *seen, int num_rev)\n+static int show_merge_base(const struct commit_list *seen, int num_rev)\n {\n \tint all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);\n \tint all_revs = all_mask & ~((1u << REV_SHIFT) - 1);\n \tint exit_status = 1;\n \n-\twhile (seen) {\n-\t\tstruct commit *commit = pop_commit(&seen);\n+\tfor (const struct commit_list *s = seen; s; s = s->next) {\n+\t\tstruct commit *commit = s->item;\n \t\tint flags = commit->object.flags & all_mask;\n \t\tif (!(flags & UNINTERESTING) &&\n \t\t ((flags & all_revs) == all_revs)) {\n@@ -635,7 +635,7 @@ static int parse_reflog_param(const struct option *opt, const char *arg,\n int cmd_show_branch(int ac, const char **av, const char *prefix)\n {\n \tstruct commit *rev[MAX_REVS], *commit;\n-\tchar *reflog_msg[MAX_REVS];\n+\tchar *reflog_msg[MAX_REVS] = {0};\n \tstruct commit_list *list = NULL, *seen = NULL;\n \tunsigned int rev_mask[MAX_REVS];\n \tint num_rev, i, extra = 0;\n@@ -692,6 +692,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t parse_reflog_param),\n \t\tOPT_END()\n \t};\n+\tconst char **args_copy = NULL;\n+\tint ret;\n \n \tinit_commit_name_slab(&name_slab);\n \n@@ -699,8 +701,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \t/* If nothing is specified, try the default first */\n \tif (ac == 1 && default_args.nr) {\n+\t\tDUP_ARRAY(args_copy, default_args.v, default_args.nr);\n \t\tac = default_args.nr;\n-\t\tav = default_args.v;\n+\t\tav = args_copy;\n \t}\n \n \tac = parse_options(ac, av, prefix, builtin_show_branch_options,\n@@ -780,7 +783,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t}\n \n \t\tfor (i = 0; i < reflog; i++) {\n-\t\t\tchar *logmsg;\n+\t\t\tchar *logmsg = NULL;\n \t\t\tchar *nth_desc;\n \t\t\tconst char *msg;\n \t\t\tchar *end;\n@@ -790,6 +793,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\tif (read_ref_at(get_main_ref_store(the_repository),\n \t\t\t\t\tref, flags, 0, base + i, &oid, &logmsg,\n \t\t\t\t\t×tamp, &tz, NULL)) {\n+\t\t\t\tfree(logmsg);\n \t\t\t\treflog = i;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -842,7 +846,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \tif (!ref_name_cnt) {\n \t\tfprintf(stderr, \"No revs to be shown.\\n\");\n-\t\texit(0);\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \tfor (num_rev = 0; ref_name[num_rev]; num_rev++) {\n@@ -879,11 +884,15 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \tcommit_list_sort_by_date(&seen);\n \n-\tif (merge_base)\n-\t\treturn show_merge_base(seen, num_rev);\n+\tif (merge_base) {\n+\t\tret = show_merge_base(seen, num_rev);\n+\t\tgoto out;\n+\t}\n \n-\tif (independent)\n-\t\treturn show_independent(rev, num_rev, rev_mask);\n+\tif (independent) {\n+\t\tret = show_independent(rev, num_rev, rev_mask);\n+\t\tgoto out;\n+\t}\n \n \t/* Show list; --more=-1 means list-only */\n \tif (1 < num_rev || extra < 0) {\n@@ -919,8 +928,10 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\tputchar('\\n');\n \t\t}\n \t}\n-\tif (extra < 0)\n-\t\texit(0);\n+\tif (extra < 0) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n \n \t/* Sort topologically */\n \tsort_in_topological_order(&seen, sort_order);\n@@ -932,8 +943,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \tall_mask = ((1u << (REV_SHIFT + num_rev)) - 1);\n \tall_revs = all_mask & ~((1u << REV_SHIFT) - 1);\n \n-\twhile (seen) {\n-\t\tstruct commit *commit = pop_commit(&seen);\n+\tfor (struct commit_list *l = seen; l; l = l->next) {\n+\t\tstruct commit *commit = l->item;\n \t\tint this_flag = commit->object.flags;\n \t\tint is_merge_point = ((this_flag & all_revs) == all_revs);\n \n@@ -973,6 +984,15 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\tif (shown_merge_point && --extra < 0)\n \t\t\tbreak;\n \t}\n+\n+\tret = 0;\n+\n+out:\n+\tfor (size_t i = 0; i < ARRAY_SIZE(reflog_msg); i++)\n+\t\tfree(reflog_msg[i]);\n+\tfree_commit_list(seen);\n+\tfree_commit_list(list);\n+\tfree(args_copy);\n \tfree(head);\n-\treturn 0;\n+\treturn ret;\n }\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex a1139f79e2..3b6dad0c46 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -2,6 +2,7 @@\n \n test_description='test show-branch'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'error descriptions on empty repository' '\ndiff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\nindex 44c726ea39..f96ea82e78 100755\n--- a/t/t6010-merge-base.sh\n+++ b/t/t6010-merge-base.sh\n@@ -6,6 +6,7 @@\n test_description='Merge base and parent list computation.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n M=1130000000\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499427","messageId":"ba4a883ae11a2b98eae499166520d5d3fe453768.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 14/23] builtin/credential-store: fix leaking credential","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:17:05Z","receivedAt":"2024-07-26T12:17:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never free credentials read by the credential store, leading to a\nmemory leak. Plug it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/credential-store.c | 1 +\n t/t0302-credential-store.sh | 2 ++\n t/t0303-credential-external.sh | 1 +\n 3 files changed, 4 insertions(+)\n\ndiff --git a/builtin/credential-store.c b/builtin/credential-store.c\nindex 494c809332..97968bfa1c 100644\n--- a/builtin/credential-store.c\n+++ b/builtin/credential-store.c\n@@ -218,5 +218,6 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)\n \t\t; /* Ignore unknown operation. */\n \n \tstring_list_clear(&fns, 0);\n+\tcredential_clear(&c);\n \treturn 0;\n }\ndiff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh\nindex 716bf1af9f..f83db659e2 100755\n--- a/t/t0302-credential-store.sh\n+++ b/t/t0302-credential-store.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='credential-store tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-credential.sh\n \ndiff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh\nindex 72ae405c3e..8aadbe86c4 100755\n--- a/t/t0303-credential-external.sh\n+++ b/t/t0303-credential-external.sh\n@@ -29,6 +29,7 @@ you can set GIT_TEST_CREDENTIAL_HELPER_SETUP to a sequence of shell\n commands.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-credential.sh\n \n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499428","messageId":"6d49645c0f82420388a8ca3522b48982fc2fe92e.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 15/23] builtin/rerere: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:17:15Z","receivedAt":"2024-07-26T12:17:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are multiple trivial memory leaks in git-rerere(1). Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/rerere.c | 8 +++++++-\n rerere.c | 9 +++++++--\n t/t2030-unresolve-info.sh | 1 +\n t/t4200-rerere.sh | 1 +\n 4 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex b2efc6f640..81b65ffa39 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -73,11 +73,17 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \n \tif (!strcmp(argv[0], \"forget\")) {\n \t\tstruct pathspec pathspec;\n+\t\tint ret;\n+\n \t\tif (argc < 2)\n \t\t\twarning(_(\"'git rerere forget' without paths is deprecated\"));\n \t\tparse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD,\n \t\t\t prefix, argv + 1);\n-\t\treturn rerere_forget(the_repository, &pathspec);\n+\n+\t\tret = rerere_forget(the_repository, &pathspec);\n+\n+\t\tclear_pathspec(&pathspec);\n+\t\treturn ret;\n \t}\n \n \tif (!strcmp(argv[0], \"clear\")) {\ndiff --git a/rerere.c b/rerere.c\nindex 3a3888cce2..525ed6cc1e 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1107,7 +1107,7 @@ static int rerere_forget_one_path(struct index_state *istate,\n \n int rerere_forget(struct repository *r, struct pathspec *pathspec)\n {\n-\tint i, fd;\n+\tint i, fd, ret;\n \tstruct string_list conflict = STRING_LIST_INIT_DUP;\n \tstruct string_list merge_rr = STRING_LIST_INIT_DUP;\n \n@@ -1132,7 +1132,12 @@ int rerere_forget(struct repository *r, struct pathspec *pathspec)\n \t\t\tcontinue;\n \t\trerere_forget_one_path(r->index, it->string, &merge_rr);\n \t}\n-\treturn write_rr(&merge_rr, fd);\n+\n+\tret = write_rr(&merge_rr, fd);\n+\n+\tstring_list_clear(&conflict, 0);\n+\tstring_list_clear(&merge_rr, 1);\n+\treturn ret;\n }\n \n /*\ndiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\nindex be3fcdde07..b3f6bc97b5 100755\n--- a/t/t2030-unresolve-info.sh\n+++ b/t/t2030-unresolve-info.sh\n@@ -5,6 +5,7 @@ test_description='undoing resolution'\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 check_resolve_undo () {\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex b0a3e84984..213b36fb96 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -25,6 +25,7 @@ test_description='git rerere\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_expect_success 'setup' '\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499429","messageId":"778e87221ac1d8cc581a8c3f3572810c68686451.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 16/23] builtin/shortlog: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:17:29Z","receivedAt":"2024-07-26T12:17:37Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There is a trivial memory leak in git-shortlog(1). Fix it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/shortlog.c | 1 +\n t/t4201-shortlog.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 5bde7c68c2..b529608c92 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -514,4 +514,5 @@ void shortlog_output(struct shortlog *log)\n \tstring_list_clear(&log->list, 1);\n \tclear_mailmap(&log->mailmap);\n \tstring_list_clear(&log->format, 0);\n+\tstring_list_clear(&log->trailers, 0);\n }\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex f698d0c9ad..c20c885724 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -9,6 +9,7 @@ test_description='git shortlog\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_expect_success 'setup' '\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499430","messageId":"8a649bf7ed38297c9a2d3ec856368603e31adb7a.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 17/23] builtin/worktree: fix leaking derived branch names","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:17:40Z","receivedAt":"2024-07-26T12:17:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several heuristics that git-worktree(1) uses to derive the\nname of the newly created branch when not given explicitly. These\nheuristics all allocate a new string, but we only end up freeing that\nstring in a subset of cases.\n\nFix the remaining cases where we didn't yet free the derived branch\nnames. While at it, also free `opt_track`, which is being populated via\nan `OPT_PASSTHRU()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/worktree.c | 7 ++++---\n t/t2400-worktree-add.sh | 1 +\n t/t9902-completion.sh | 1 +\n 3 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 1d51e54fcd..cec3ada6b0 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -769,7 +769,7 @@ static int add(int ac, const char **av, const char *prefix)\n \tchar *branch_to_free = NULL;\n \tchar *new_branch_to_free = NULL;\n \tconst char *new_branch = NULL;\n-\tconst char *opt_track = NULL;\n+\tchar *opt_track = NULL;\n \tconst char *lock_reason = NULL;\n \tint keep_locked = 0;\n \tint used_new_branch_options;\n@@ -846,7 +846,7 @@ static int add(int ac, const char **av, const char *prefix)\n \tif (opts.orphan && !new_branch) {\n \t\tint n;\n \t\tconst char *s = worktree_basename(path, &n);\n-\t\tnew_branch = xstrndup(s, n);\n+\t\tnew_branch = new_branch_to_free = xstrndup(s, n);\n \t} else if (opts.orphan) {\n \t\t; /* no-op */\n \t} else if (opts.detach) {\n@@ -875,7 +875,7 @@ static int add(int ac, const char **av, const char *prefix)\n \t\t\tremote = unique_tracking_name(branch, &oid, NULL);\n \t\t\tif (remote) {\n \t\t\t\tnew_branch = branch;\n-\t\t\t\tbranch = remote;\n+\t\t\t\tbranch = new_branch_to_free = remote;\n \t\t\t}\n \t\t}\n \n@@ -923,6 +923,7 @@ static int add(int ac, const char **av, const char *prefix)\n \n \tret = add_worktree(path, branch, &opts);\n \tfree(path);\n+\tfree(opt_track);\n \tfree(branch_to_free);\n \tfree(new_branch_to_free);\n \treturn ret;\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex ba320dc417..cfc4aeb179 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/lib-rebase.sh\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 932d5ad759..cc6aa9f0cd 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -16,6 +16,7 @@ test_untraceable=UnfortunatelyYes\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./lib-bash.sh\n \n complete ()\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499431","messageId":"2c7a3694903e54dcfa2eadb3b7fa7828da42f5d3.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 18/23] builtin/credential-cache: fix trivial leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:17:56Z","receivedAt":"2024-07-26T12:18:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are two trivial leaks in git-credential-cache(1):\n\n - We leak the child process in `spawn_daemon()`. As we do not call\n `finish_command()` and instead let the created process daemonize, we\n have to clear the process manually.\n\n - We do not free the computed socket path in case it wasn't given via\n `--socket=`.\n\nPlug both of these memory leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/credential-cache.c | 9 +++++++--\n t/t0301-credential-cache.sh | 2 ++\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\nindex 3db8df70a9..aaf2f8438b 100644\n--- a/builtin/credential-cache.c\n+++ b/builtin/credential-cache.c\n@@ -88,6 +88,8 @@ static void spawn_daemon(const char *socket)\n \t\tdie_errno(\"unable to read result code from cache daemon\");\n \tif (r != 3 || memcmp(buf, \"ok\\n\", 3))\n \t\tdie(\"cache daemon did not start: %.*s\", r, buf);\n+\n+\tchild_process_clear(&daemon);\n \tclose(daemon.out);\n }\n \n@@ -137,7 +139,8 @@ static void announce_capabilities(void)\n \n int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n {\n-\tchar *socket_path = NULL;\n+\tconst char *socket_path_arg = NULL;\n+\tchar *socket_path;\n \tint timeout = 900;\n \tconst char *op;\n \tconst char * const usage[] = {\n@@ -147,7 +150,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"timeout\", &timeout,\n \t\t\t \"number of seconds to cache credentials\"),\n-\t\tOPT_STRING(0, \"socket\", &socket_path, \"path\",\n+\t\tOPT_STRING(0, \"socket\", &socket_path_arg, \"path\",\n \t\t\t \"path of cache-daemon socket\"),\n \t\tOPT_END()\n \t};\n@@ -160,6 +163,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n \tif (!have_unix_sockets())\n \t\tdie(_(\"credential-cache unavailable; no unix socket support\"));\n \n+\tsocket_path = xstrdup_or_null(socket_path_arg);\n \tif (!socket_path)\n \t\tsocket_path = get_socket_path();\n \tif (!socket_path)\n@@ -176,6 +180,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n \telse\n \t\t; /* ignore unknown operation */\n \n+\tfree(socket_path);\n \treturn 0;\n }\n \ndiff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\nindex c10e35905e..5d5b64205f 100755\n--- a/t/t0301-credential-cache.sh\n+++ b/t/t0301-credential-cache.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='credential-cache tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-credential.sh\n \n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499432","messageId":"7564911d2ad5332ea1f70b51868585819e895b06.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 19/23] t/test-repository: fix leaking repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:18:14Z","receivedAt":"2024-07-26T12:18:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The test-repository test helper zeroes out `the_repository` such that it\ncan be sure that our codebase only ends up using the supplied repository\nthat we initialize in the respective helper functions. This does cause\nmemory leaks though as the data that `the_repository` has been holding\nonto is not referenced anymore.\n\nFix this by calling `repo_clear()` instead.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-repository.c | 4 ++--\n t/t5318-commit-graph.sh | 2 ++\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\nindex c6a074df3d..63c37de33d 100644\n--- a/t/helper/test-repository.c\n+++ b/t/helper/test-repository.c\n@@ -19,7 +19,7 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,\n \n \tsetup_git_env(gitdir);\n \n-\tmemset(the_repository, 0, sizeof(*the_repository));\n+\trepo_clear(the_repository);\n \n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\n@@ -49,7 +49,7 @@ static void test_get_commit_tree_in_graph(const char *gitdir,\n \n \tsetup_git_env(gitdir);\n \n-\tmemset(the_repository, 0, sizeof(*the_repository));\n+\trepo_clear(the_repository);\n \n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex a2b4442660..2916c07e3c 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='commit graph'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-chunk.sh\n \n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499433","messageId":"e3dbf9116baaf3cf519a9c3cb78026ede566e3f4.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 20/23] object-name: fix leaking commit list items","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:18:28Z","receivedAt":"2024-07-26T12:18:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `get_oid_oneline()`, we pass in a `struct commit_list` that\ngets modified by the function. This creates a weird situation where the\ncommit list may sometimes be empty after returning, but sometimes it\nwill continue to carry additional commits. In those cases the remainder\nof the list leaks.\n\nUltimately, the design where we only pass partial ownership to\n`get_oid_oneline()` feels shoddy. Refactor the code such that we only\npass a constant pointer to the list, creating a local copy as needed.\nCallers are thus always responsible for freeing the commit list, which\nthen allows us to plug a bunch of memory leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-name.c | 26 ++++++++++++++++----------\n t/t1511-rev-parse-caret.sh | 1 +\n t/t6133-pathspec-rev-dwim.sh | 2 ++\n 3 files changed, 19 insertions(+), 10 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 527b853ac4..240a93e7ce 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -27,7 +27,8 @@\n #include \"date.h\"\n #include \"object-file-convert.h\"\n \n-static int get_oid_oneline(struct repository *r, const char *, struct object_id *, struct commit_list *);\n+static int get_oid_oneline(struct repository *r, const char *, struct object_id *,\n+\t\t\t const struct commit_list *);\n \n typedef int (*disambiguate_hint_fn)(struct repository *, const struct object_id *, void *);\n \n@@ -1254,6 +1255,8 @@ static int peel_onion(struct repository *r, const char *name, int len,\n \t\tprefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));\n \t\tcommit_list_insert((struct commit *)o, &list);\n \t\tret = get_oid_oneline(r, prefix, oid, list);\n+\n+\t\tfree_commit_list(list);\n \t\tfree(prefix);\n \t\treturn ret;\n \t}\n@@ -1388,9 +1391,10 @@ static int handle_one_ref(const char *path, const struct object_id *oid,\n \n static int get_oid_oneline(struct repository *r,\n \t\t\t const char *prefix, struct object_id *oid,\n-\t\t\t struct commit_list *list)\n+\t\t\t const struct commit_list *list)\n {\n-\tstruct commit_list *backup = NULL, *l;\n+\tstruct commit_list *copy = NULL;\n+\tconst struct commit_list *l;\n \tint found = 0;\n \tint negative = 0;\n \tregex_t regex;\n@@ -1411,14 +1415,14 @@ static int get_oid_oneline(struct repository *r,\n \n \tfor (l = list; l; l = l->next) {\n \t\tl->item->object.flags |= ONELINE_SEEN;\n-\t\tcommit_list_insert(l->item, &backup);\n+\t\tcommit_list_insert(l->item, ©);\n \t}\n-\twhile (list) {\n+\twhile (copy) {\n \t\tconst char *p, *buf;\n \t\tstruct commit *commit;\n \t\tint matches;\n \n-\t\tcommit = pop_most_recent_commit(&list, ONELINE_SEEN);\n+\t\tcommit = pop_most_recent_commit(©, ONELINE_SEEN);\n \t\tif (!parse_object(r, &commit->object.oid))\n \t\t\tcontinue;\n \t\tbuf = repo_get_commit_buffer(r, commit, NULL);\n@@ -1433,10 +1437,9 @@ static int get_oid_oneline(struct repository *r,\n \t\t}\n \t}\n \tregfree(®ex);\n-\tfree_commit_list(list);\n-\tfor (l = backup; l; l = l->next)\n+\tfor (l = list; l; l = l->next)\n \t\tclear_commit_marks(l->item, ONELINE_SEEN);\n-\tfree_commit_list(backup);\n+\tfree_commit_list(copy);\n \treturn found ? 0 : -1;\n }\n \n@@ -2024,7 +2027,10 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n \t\t\trefs_for_each_ref(get_main_ref_store(repo), handle_one_ref, &cb);\n \t\t\trefs_head_ref(get_main_ref_store(repo), handle_one_ref, &cb);\n \t\t\tcommit_list_sort_by_date(&list);\n-\t\t\treturn get_oid_oneline(repo, name + 2, oid, list);\n+\t\t\tret = get_oid_oneline(repo, name + 2, oid, list);\n+\n+\t\t\tfree_commit_list(list);\n+\t\t\treturn ret;\n \t\t}\n \t\tif (namelen < 3 ||\n \t\t name[2] != ':' ||\ndiff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh\nindex 6ecfed86bc..e7e78a4028 100755\n--- a/t/t1511-rev-parse-caret.sh\n+++ b/t/t1511-rev-parse-caret.sh\n@@ -5,6 +5,7 @@ test_description='tests for ref^{stuff}'\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_expect_success 'setup' '\ndiff --git a/t/t6133-pathspec-rev-dwim.sh b/t/t6133-pathspec-rev-dwim.sh\nindex a290ffca0d..6dd4bbbf9f 100755\n--- a/t/t6133-pathspec-rev-dwim.sh\n+++ b/t/t6133-pathspec-rev-dwim.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test dwim of revs versus pathspecs in revision parser'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499434","messageId":"e426fd7ec5b3a93b4110d05fb4e3580c4f6e7d52.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 21/23] entry: fix leaking pathnames during delayed checkout","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:18:53Z","receivedAt":"2024-07-26T12:19:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When filtering files during delayed checkout, we pass a string list to\n`async_query_available_blobs()`. This list is initialized with NODUP,\nand thus inserted strings will not be owned by the list. In the latter\nfunction we then try to hand over ownership by passing an `xstrup()`'d\nvalue to `string_list_insert()`. But this is not how this works: a NODUP\nlist does not take ownership of allocated strings and will never free\nthem for the caller.\n\nFix this issue by initializing the list as `DUP` instead and dropping\nthe explicit call to `xstrdup()`. This is okay to do given that this is\nthe single callsite of `async_query_available_blobs()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n convert.c | 2 +-\n entry.c | 4 +++-\n t/t2080-parallel-checkout-basics.sh | 1 +\n t/t2082-parallel-checkout-attributes.sh | 1 +\n 4 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2..61a540e212 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -960,7 +960,7 @@ int async_query_available_blobs(const char *cmd, struct string_list *available_p\n \twhile ((line = packet_read_line(process->out, NULL))) {\n \t\tconst char *path;\n \t\tif (skip_prefix(line, \"pathname=\", &path))\n-\t\t\tstring_list_insert(available_paths, xstrdup(path));\n+\t\t\tstring_list_insert(available_paths, path);\n \t\telse\n \t\t\t; /* ignore unknown keys */\n \t}\ndiff --git a/entry.c b/entry.c\nindex e7ed440ce2..3143b9996b 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -191,7 +191,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)\n \t\tprogress = start_delayed_progress(_(\"Filtering content\"), dco->paths.nr);\n \twhile (dco->filters.nr > 0) {\n \t\tfor_each_string_list_item(filter, &dco->filters) {\n-\t\t\tstruct string_list available_paths = STRING_LIST_INIT_NODUP;\n+\t\t\tstruct string_list available_paths = STRING_LIST_INIT_DUP;\n \n \t\t\tif (!async_query_available_blobs(filter->string, &available_paths)) {\n \t\t\t\t/* Filter reported an error */\n@@ -245,6 +245,8 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)\n \t\t\t\t} else\n \t\t\t\t\terrs = 1;\n \t\t\t}\n+\n+\t\t\tstring_list_clear(&available_paths, 0);\n \t\t}\n \n \t\tfilter_string_list(&dco->filters, 0, string_is_not_null, NULL);\ndiff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh\nindex 5ffe1a41e2..59e5570cb2 100755\n--- a/t/t2080-parallel-checkout-basics.sh\n+++ b/t/t2080-parallel-checkout-basics.sh\n@@ -8,6 +8,7 @@ working tree.\n '\n \n TEST_NO_CREATE_REPO=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-parallel-checkout.sh\"\n \ndiff --git a/t/t2082-parallel-checkout-attributes.sh b/t/t2082-parallel-checkout-attributes.sh\nindex f3511cd43a..aec55496eb 100755\n--- a/t/t2082-parallel-checkout-attributes.sh\n+++ b/t/t2082-parallel-checkout-attributes.sh\n@@ -10,6 +10,7 @@ properly (without access to the index or attribute stack).\n '\n \n TEST_NO_CREATE_REPO=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-parallel-checkout.sh\"\n . \"$TEST_DIRECTORY/lib-encoding.sh\"\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499435","messageId":"8c2a19994fcda53cc1cb315c8ab87a36838d598b.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 22/23] convert: fix leaking config strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:19:12Z","receivedAt":"2024-07-26T12:19:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `read_convert_config()`, we end up reading some string values into\nvariables. We don't free any potentially-existing old values though,\nwhich will result in a memory leak in case the same key has been defined\nmultiple times.\n\nFix those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n convert.c | 12 +++++++++---\n t/t0021-conversion.sh | 1 +\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 61a540e212..92ce04c406 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1050,14 +1050,20 @@ static int read_convert_config(const char *var, const char *value,\n \t * The command-line will not be interpolated in any way.\n \t */\n \n-\tif (!strcmp(\"smudge\", key))\n+\tif (!strcmp(\"smudge\", key)) {\n+\t\tFREE_AND_NULL(drv->smudge);\n \t\treturn git_config_string(&drv->smudge, var, value);\n+\t}\n \n-\tif (!strcmp(\"clean\", key))\n+\tif (!strcmp(\"clean\", key)) {\n+\t\tFREE_AND_NULL(drv->clean);\n \t\treturn git_config_string(&drv->clean, var, value);\n+\t}\n \n-\tif (!strcmp(\"process\", key))\n+\tif (!strcmp(\"process\", key)) {\n+\t\tFREE_AND_NULL(drv->process);\n \t\treturn git_config_string(&drv->process, var, value);\n+\t}\n \n \tif (!strcmp(\"required\", key)) {\n \t\tdrv->required = git_config_bool(var, value);\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 0b4997022b..eeb2714d9d 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -5,6 +5,7 @@ test_description='blob conversion via gitattributes'\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-terminal.sh\n \n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499436","messageId":"60aef16aef5e5733d449b3e9fd1a8816b5d82c27.1721995576.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH 23/23] commit-reach: fix trivial memory leak when computing reachability","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-26T12:19:22Z","receivedAt":"2024-07-26T12:19:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We don't free the local `stack` commit list that we use to compute\nreachability of multiple commits at once. Do so.\n\nSigned-off-by: Patrick Steinhardt \n---\n commit-reach.c | 1 +\n t/t3201-branch-contains.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex dabc2972e4..02f8218b8e 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -1227,4 +1227,5 @@ void tips_reachable_from_bases(struct repository *r,\n done:\n \tfree(commits);\n \trepo_clear_commit_marks(r, SEEN);\n+\tfree_commit_list(stack);\n }\ndiff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh\nindex 800fc33165..6e587d27d7 100755\n--- a/t/t3201-branch-contains.sh\n+++ b/t/t3201-branch-contains.sh\n@@ -2,6 +2,7 @@\n \n test_description='branch --contains , --no-contains --merged, and --no-merged'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\n-- \n2.46.0.rc1.dirty\n\n"},{"id":"499588","messageId":"CAOLa=ZQ1zGe421HFqH4wCSMwc53H5eUr18j18yM0pNgD8DxR5g@mail.gmail.com","threadId":"61850","inReplyTo":"4daae88877f2355d6a3960883e7b3f15cf738cb5.1721995576.git.ps@pks.im","subject":"Re: [PATCH 02/23] builtin/log: fix leaking branch name when creating cover letters","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-30T09:14:00Z","receivedAt":"2024-07-30T09:14:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> When calling `make_cover_letter()` without a branch name, then we try to\n\nNit: s/then//\n\n> derive the branch name by calling `find_branch_name()`. But while this\n> function returns an allocated string, we never free the result and thus\n> have a memory leak. Fix this.\n\n[snip]\n"},{"id":"499589","messageId":"CAOLa=ZTj6RxXH9t71q2fi-M+Xh-vOZnKzDk_mPmhNTEqVjkAzg@mail.gmail.com","threadId":"61850","inReplyTo":"08a12be13c2fed247d6086967e7a3f03fa6519e1.1721995576.git.ps@pks.im","subject":"Re: [PATCH 03/23] builtin/describe: fix memory leak with `--contains=`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-30T09:23:33Z","receivedAt":"2024-07-30T09:23:34Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n[snip]\n\n> @@ -637,7 +639,21 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n> \t\t\tstrvec_pushv(&args, argv);\n> \t\telse\n> \t\t\tstrvec_push(&args, \"HEAD\");\n> -\t\treturn cmd_name_rev(args.nr, args.v, prefix);\n> +\n> +\t\t/*\n> +\t\t * `cmd_name_rev()` modifies the array, so we'd link its\n> +\t\t * contained strings if we didn't do a copy here.\n> +\t\t */\n> +\t\tALLOC_ARRAY(argv_copy, args.nr + 1);\n> +\t\tfor (size_t i = 0; i < args.nr; i++)\n> +\t\t\targv_copy[i] = args.v[i];\n> +\t\targv_copy[args.nr] = NULL;\n\nEventhough we pass `args.nr`, we still set the last element to NULL.\nThis replicates what strvec does and makes it more robust. Nice.\n\n> +\t\tret = cmd_name_rev(args.nr, argv_copy, prefix);\n> +\n> +\t\tstrvec_clear(&args);\n> +\t\tfree(argv_copy);\n> +\t\treturn ret;\n> \t}\n>\n> \thashmap_init(&names, commit_name_neq, NULL, 0);\n> --\n> 2.46.0.rc1.dirty\n"},{"id":"499590","messageId":"CAOLa=ZRJs4USrE2XbPQtqHU4Q7dAWqOkJtJ=OFisMc9Hy3vECw@mail.gmail.com","threadId":"61850","inReplyTo":"e8d86c01cf1316fabcec7dfacad55e1dc98baa7d.1721995576.git.ps@pks.im","subject":"Re: [PATCH 04/23] builtin/describe: fix leaking array when running diff-index","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-30T09:34:45Z","receivedAt":"2024-07-30T09:34:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> When running git-describe(1) with `--dirty`, we will set up a `struct\n> rev_info` with arguments for git-diff-index(1). The way we assmble the\n\ns/assmble/assemble\n\n[snip]\n"},{"id":"499593","messageId":"CAOLa=ZQ7t4eo34eQNtLzd_tjknsxkaSVGy6Z11hPUQdkGYBfow@mail.gmail.com","threadId":"61850","inReplyTo":"e277222bd298560b0d7f10cb9b5d0eee5967ba87.1721995576.git.ps@pks.im","subject":"Re: [PATCH 12/23] builtin/rev-parse: fix memory leak with `--parseopt`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-30T11:00:46Z","receivedAt":"2024-07-30T11:00:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> The `--parseopt` mode allows shell scripts to have the same option\n> parsing mode as we have in C builtins. It soaks up a set of option\n> descriptions via stdin and massages them into proper `struct option`s\n> that we can then use to parse a set of arguments.\n>\n> We only partially free those options when done though, creating a memory\n> leak. Interestingly, we only end up free'ing the first option's help,\n> which is of course wrong.\n>\n> Fix this by freeing all option's help fields as well as their `argh`\n> fielids to plug this memory leak.\n>\n\ns/fielids/fields\n\n[snip]\n"},{"id":"499594","messageId":"CAOLa=ZT==H9VQp_Z_VcQEmXVBUPWWui1CfmRQbLnOyyqMwxjmg@mail.gmail.com","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"Re: [PATCH 00/23] Memory leak fixes (pt.3)","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-30T11:09:57Z","receivedAt":"2024-07-30T11:09:58Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> I originally wanted to hold off with sending out this series until v2.46\n> was out. But I saw that Junio sent out some patches which are plugging\n> the same leaks as I did, so I dedcided to send it out now to avoid some\n> duplicated work.\n>\n> There isn't really any structure to this series, I just happened to pick\n> some random test suites that fail with the leak checker enabled and then\n> fixed those. Naturally, I've also got part 4 of this series of patch\n> series in the pipeline already :) As mentioned elsewhere, I hope to get\n> the number of failing test suites to zero this year. Let's see whether\n> this is realistic.\n>\n> Patrick\n>\n\nThis was quite easy to review since there wasn't much to add and it was\nclearly split into small commits. Apart from some nits, the series looks\ngreat to me.\n\nThanks,\nKarthik\n"},{"id":"499616","messageId":"xmqqr0bagark.fsf@gitster.g","threadId":"61850","inReplyTo":"08a12be13c2fed247d6086967e7a3f03fa6519e1.1721995576.git.ps@pks.im","subject":"Re: [PATCH 03/23] builtin/describe: fix memory leak with `--contains=`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-30T15:27:59Z","receivedAt":"2024-07-30T15:28:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> When calling `git describe --contains=`, we end up invoking\n> `cmd_name_rev()` with some munged argv array. This array may contain\n> allocated strings and furthermore will likely be modified by the called\n> function. This results in two memory leaks:\n>\n> - First, we leak the array that we use to assemble the arguments.\n>\n> - Second, we leak the allocated strings that we may have put into the\n> array.\n>\n> Fix those leaks by creating a separate copy of the array that we can\n> hand over to `cmd_name_rev()`. This allows us to free all strings\n> contained in the `strvec`, as the original vector will not be modified\n> anymore.\n\nOK, the separate copy has to be a shallow copy, as its purpose is\nnot to lose pointers to the contained strings.\n\n> Furthermore, free both the `strvec` and the copied array to fix the\n> first memory leak.\n> ...\n> +\t\tstrvec_clear(&args);\n> +\t\tfree(argv_copy);\n\nSo, calling cmd_name_rev() may shuffle the argv_copy[] array but at\nleast it will not free any element in it (as expected---it is\ntypically the (argc, argv[]) the process receives from getting\nexec'ed) [*]. Freeing the argv_copy shell itself is sufficient to\ndiscard what we used to call cmd_name_rev(). And we discard args\nboth its content strings and the array. OK.\n\n> +\t\treturn ret;\n> \t}\n> \n> \thashmap_init(&names, commit_name_neq, NULL, 0);\n\n\n[Footnote]\n\n * The fact that cmd_foo() is called is not a hygiene thing to do in\n the first place, and in the longer term #leftoverbits we may need\n to refactor the thing further, into a proper library-ish reusable\n helper function that can be used to compute name_rev() any number\n of times, plus cmd_name_rev() and this caller that call it.\n"},{"id":"499618","messageId":"xmqqikwmgacm.fsf@gitster.g","threadId":"61850","inReplyTo":"088f730572f0f5aaa8f9640ea35a4687be9094ba.1721995576.git.ps@pks.im","subject":"Re: [PATCH 06/23] builtin/name-rev: fix various trivial memory leaks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-30T15:36:57Z","receivedAt":"2024-07-30T15:37:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> There are several structures that we don't release after\n> `cmd_name_rev()` is done. Plug those leaks.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/name-rev.c | 6 ++++--\n> t/t6007-rev-list-cherry-pick-file.sh | 1 +\n> t/t6120-describe.sh | 1 +\n> 3 files changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n> index 70e9ec4e47..f62c0a36cb 100644\n> --- a/builtin/name-rev.c\n> +++ b/builtin/name-rev.c\n> @@ -677,7 +677,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n> \t\t\t\t always, allow_undefined, data.name_only);\n> \t}\n> \n> -\tUNLEAK(string_pool);\n> -\tUNLEAK(revs);\n> +\tstring_list_clear(&data.ref_filters, 0);\n> +\tstring_list_clear(&data.exclude_filters, 0);\n> +\tmem_pool_discard(&string_pool, 0);\n> +\tobject_array_clear(&revs);\n> \treturn 0;\n> }\n\nYou, originally these \"we know we are at the very end of the\nprocess, so _exit() will take care of releasing the resources\" was a\nvery much deliberate, and in a sense, cmd_describe() calling to this\nfunction as the last thing to do was still in the same spirit, but\nit was not a hygiene thing to do.\n\nIn the longer term, we would want to make a major part of the body\nof this \"main\" function into a reusable library-ish function, which\nwill be called the desribe and name-rev command implementations, and\nwhen that happens, these fixes would move together to that\nlibrary-ish function.\n\nLooking good.\n"},{"id":"499684","messageId":"ZqoVDkOw5IEWv52m@tanuki","threadId":"61850","inReplyTo":"xmqqr0bagark.fsf@gitster.g","subject":"Re: [PATCH 03/23] builtin/describe: fix memory leak with `--contains=`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-31T10:42:22Z","receivedAt":"2024-07-31T10:42:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jul 30, 2024 at 08:27:59AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> [Footnote]\n> \n> * The fact that cmd_foo() is called is not a hygiene thing to do in\n> the first place, and in the longer term #leftoverbits we may need\n> to refactor the thing further, into a proper library-ish reusable\n> helper function that can be used to compute name_rev() any number\n> of times, plus cmd_name_rev() and this caller that call it.\n\nAgreed. There have been several instances of this scattered across the\ncodebase. The fix is quite ugly in my opinion, but it would be a bigger\ntopic to refactor those cases properly, so I refrained from doing so as\npart of this series.\n\nPatrick\n"},{"id":"499685","messageId":"ZqoVo-Vl2tqlelLA@tanuki","threadId":"61850","inReplyTo":"CAOLa=ZT==H9VQp_Z_VcQEmXVBUPWWui1CfmRQbLnOyyqMwxjmg@mail.gmail.com","subject":"Re: [PATCH 00/23] Memory leak fixes (pt.3)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-31T10:44:51Z","receivedAt":"2024-07-31T10:44:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jul 30, 2024 at 07:09:57AM -0400, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> \n> > Hi,\n> >\n> > I originally wanted to hold off with sending out this series until v2.46\n> > was out. But I saw that Junio sent out some patches which are plugging\n> > the same leaks as I did, so I dedcided to send it out now to avoid some\n> > duplicated work.\n> >\n> > There isn't really any structure to this series, I just happened to pick\n> > some random test suites that fail with the leak checker enabled and then\n> > fixed those. Naturally, I've also got part 4 of this series of patch\n> > series in the pipeline already :) As mentioned elsewhere, I hope to get\n> > the number of failing test suites to zero this year. Let's see whether\n> > this is realistic.\n> >\n> > Patrick\n> >\n> \n> This was quite easy to review since there wasn't much to add and it was\n> clearly split into small commits. Apart from some nits, the series looks\n> great to me.\n\nThanks for your review! Given that until now the only thing that needs\nfixing is smallish typoes I'll refrain from sending the whole thing out\njust to fix those. I've fixed them locally though and will send them out\nin case anything else needs fixing.\n\nPatrick\n"},{"id":"499711","messageId":"xmqqv80l7djo.fsf@gitster.g","threadId":"61850","inReplyTo":"ZqoVDkOw5IEWv52m@tanuki","subject":"Re: [PATCH 03/23] builtin/describe: fix memory leak with `--contains=`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-31T16:04:59Z","receivedAt":"2024-07-31T16:05:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Tue, Jul 30, 2024 at 08:27:59AM -0700, Junio C Hamano wrote:\n>> Patrick Steinhardt writes:\n>> [Footnote]\n>> \n>> * The fact that cmd_foo() is called is not a hygiene thing to do in\n>> the first place, and in the longer term #leftoverbits we may need\n>> to refactor the thing further, into a proper library-ish reusable\n>> helper function that can be used to compute name_rev() any number\n>> of times, plus cmd_name_rev() and this caller that call it.\n>\n> Agreed. There have been several instances of this scattered across the\n> codebase. The fix is quite ugly in my opinion, but it would be a bigger\n> topic to refactor those cases properly, so I refrained from doing so as\n> part of this series.\n\nOh, I agree that it would be a \"after all the dust settles\" kind of\nclean-up.\n\nThanks.\n"},{"id":"499715","messageId":"Zqpks6Oi7I4TAk2f@nand.local","threadId":"61850","inReplyTo":"dd044eacc2b20c3e426f5825fdc30f4db6618052.1721995576.git.ps@pks.im","subject":"Re: [PATCH 01/23] builtin/replay: plug leaking `advance_name` variable","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-31T16:22:11Z","receivedAt":"2024-07-31T16:22:15Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 26, 2024 at 02:13:59PM +0200, Patrick Steinhardt wrote:\n> The `advance_name` variable can either contain a static string when\n> parsed via the `--advance` command line option or it may be an allocated\n> string when set via `determine_replay_mode()`. Because we cannot be sure\n> whether it is allocated or not we just didn't free it at all, resulting\n> in a memory leak.\n>\n> Split up the variables such that we can track the static and allocated\n> strings separately and then free the allocated one to fix the memory\n> leak.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/replay.c | 20 ++++++++++++++------\n> t/t3650-replay-basics.sh | 1 +\n> 2 files changed, 15 insertions(+), 6 deletions(-)\n\nI had to read this patch a couple of times to make sure that I\nunderstood the flow into and out of determine_replay_mode(). But after\nreading it a couple of times, I agree with the change that you made\nhere.\n\nThanks,\nTaylor\n\n"},{"id":"499716","messageId":"Zqpk8exl3yzakVQF@nand.local","threadId":"61850","inReplyTo":"CAOLa=ZQ1zGe421HFqH4wCSMwc53H5eUr18j18yM0pNgD8DxR5g@mail.gmail.com","subject":"Re: [PATCH 02/23] builtin/log: fix leaking branch name when creating cover letters","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-31T16:23:13Z","receivedAt":"2024-07-31T16:23:16Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jul 30, 2024 at 02:14:00AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n>\n> > When calling `make_cover_letter()` without a branch name, then we try to\n>\n> Nit: s/then//\n\nGood spotting :-).\n\nThanks,\nTaylor\n"},{"id":"499718","messageId":"ZqpmLsX8pug6qj3f@nand.local","threadId":"61850","inReplyTo":"08a12be13c2fed247d6086967e7a3f03fa6519e1.1721995576.git.ps@pks.im","subject":"Re: [PATCH 03/23] builtin/describe: fix memory leak with `--contains=`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-31T16:28:30Z","receivedAt":"2024-07-31T16:28:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 26, 2024 at 02:14:18PM +0200, Patrick Steinhardt wrote:\n> @@ -637,7 +639,21 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n> \t\t\tstrvec_pushv(&args, argv);\n> \t\telse\n> \t\t\tstrvec_push(&args, \"HEAD\");\n> -\t\treturn cmd_name_rev(args.nr, args.v, prefix);\n> +\n> +\t\t/*\n> +\t\t * `cmd_name_rev()` modifies the array, so we'd link its\n\ns/link/leak/ ?\n\nThanks,\nTaylor\n"},{"id":"499719","messageId":"Zqpn0bLD+aLgY9PN@nand.local","threadId":"61850","inReplyTo":"d42152654bf91e90b8b417316f255746a3a75155.1721995576.git.ps@pks.im","subject":"Re: [PATCH 08/23] builtin/ls-remote: fix leaking `pattern` strings","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-31T16:35:29Z","receivedAt":"2024-07-31T16:35:31Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 26, 2024 at 02:15:24PM +0200, Patrick Steinhardt wrote:\n> Users can pass patterns to git-ls-remote(1), which allows them to filter\n> the list of printed references. We assemble those patterns into an array\n> and prefix them with \"*/\", but never free either the array nor the\n> allocated strings.\n>\n> Fix those leaks.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/ls-remote.c | 11 +++++++----\n> t/t5535-fetch-push-symref.sh | 1 +\n> 2 files changed, 8 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\n> index debf2d4f88..500f76fe4c 100644\n> --- a/builtin/ls-remote.c\n> +++ b/builtin/ls-remote.c\n> @@ -47,7 +47,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n> \tint status = 0;\n> \tint show_symref_target = 0;\n> \tconst char *uploadpack = NULL;\n> -\tconst char **pattern = NULL;\n> +\tchar **pattern = NULL;\n> \tstruct transport_ls_refs_options transport_options =\n> \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n> \tint i;\n> @@ -96,9 +96,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n> \tif (argc > 1) {\n> \t\tint i;\n> \t\tCALLOC_ARRAY(pattern, argc);\n> -\t\tfor (i = 1; i < argc; i++) {\n> +\t\tfor (i = 1; i < argc; i++)\n> \t\t\tpattern[i - 1] = xstrfmt(\"*/%s\", argv[i]);\n> -\t\t}\n\nInstead of xstrfmt()-ing these strings directly, it may be more\nergonomic to use a strvec here and call strvec_pushf(). That would save\nyou explicitly CALLOC'ing the array, and would also save you the\nexplicit free() on each element of pattern below.\n\n> \t}\n>\n> \tif (flags & REF_TAGS)\n> @@ -136,7 +135,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n> \t\tstruct ref_array_item *item;\n> \t\tif (!check_ref_type(ref, flags))\n> \t\t\tcontinue;\n> -\t\tif (!tail_match(pattern, ref->name))\n> +\t\tif (!tail_match((const char **) pattern, ref->name))\n\nYou could also drop the const cast here as well. The resulting patch on\ntop of this one simplifies things a bit:\n\n--- 8< ---\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 500f76fe4c..b59ac98d81 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -47,7 +47,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tint status = 0;\n \tint show_symref_target = 0;\n \tconst char *uploadpack = NULL;\n-\tchar **pattern = NULL;\n+\tstruct strvec pattern = STRVEC_INIT;\n \tstruct transport_ls_refs_options transport_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tint i;\n@@ -93,12 +93,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n\n \tpacket_trace_identity(\"ls-remote\");\n\n-\tif (argc > 1) {\n-\t\tint i;\n-\t\tCALLOC_ARRAY(pattern, argc);\n-\t\tfor (i = 1; i < argc; i++)\n-\t\t\tpattern[i - 1] = xstrfmt(\"*/%s\", argv[i]);\n-\t}\n+\tfor (int i = 1; i < argc; i++)\n+\t\tstrvec_pushf(&pattern, \"*/%s\", argv[i]);\n\n \tif (flags & REF_TAGS)\n \t\tstrvec_push(&transport_options.ref_prefixes, \"refs/tags/\");\n@@ -135,7 +131,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tstruct ref_array_item *item;\n \t\tif (!check_ref_type(ref, flags))\n \t\t\tcontinue;\n-\t\tif (!tail_match((const char **) pattern, ref->name))\n+\t\tif (!tail_match(pattern.v, ref->name))\n \t\t\tcontinue;\n \t\titem = ref_array_push(&ref_array, ref->name, &ref->old_oid);\n \t\titem->symref = xstrdup_or_null(ref->symref);\n@@ -158,8 +154,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tstatus = 1;\n \ttransport_ls_refs_options_release(&transport_options);\n\n-\tfor (i = 1; i < argc; i++)\n-\t\tfree(pattern[i - 1]);\n-\tfree(pattern);\n+\tstrvec_clear(&pattern);\n \treturn status;\n }\n--- >8 ---\n\nThanks,\nTaylor\n"},{"id":"499720","messageId":"ZqpoQFFmGQa25veh@nand.local","threadId":"61850","inReplyTo":"6952fb2ff2cf453de39b63883a716a9f09cab7b7.1721995576.git.ps@pks.im","subject":"Re: [PATCH 09/23] builtin/remote: fix leaking strings in `branch_list`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-31T16:37:20Z","receivedAt":"2024-07-31T16:37:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 26, 2024 at 02:15:36PM +0200, Patrick Steinhardt wrote:\n> @@ -292,8 +292,8 @@ static int config_read_branches(const char *key, const char *value,\n> \t\ttype = PUSH_REMOTE;\n> \telse\n> \t\treturn 0;\n> -\tname = xmemdupz(key, key_len);\n>\n> +\tname = xmemdupz(key, key_len);\n\nI stared at this for longer than I'd like to admit wondering if there\nwas a whitespace error in the pre-image or something. But I see you just\nmoved initializing 'name' next to the initialization of 'item'.\n\nI'd argue that might be a stray diff, but I don't think it matters much.\n\n> \titem = string_list_insert(&branch_list, name);\n>\n> \tif (!item->util)\n> @@ -337,6 +337,7 @@ static int config_read_branches(const char *key, const char *value,\n> \t\tBUG(\"unexpected type=%d\", type);\n> \t}\n>\n> +\tfree(name);\n> \treturn 0;\n> }\n\nThe patch looks good to me otherwise.\n\nThanks,\nTaylor\n"},{"id":"499721","messageId":"Zqpo/8HnUbgF5ZmQ@nand.local","threadId":"61850","inReplyTo":"a4b3ca49c9724b79ca695d0db8e45be663a46434.1721995576.git.ps@pks.im","subject":"Re: [PATCH 11/23] builtin/stash: fix various trivial memory leaks","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-31T16:40:31Z","receivedAt":"2024-07-31T16:40:33Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 26, 2024 at 02:16:12PM +0200, Patrick Steinhardt wrote:\n> @@ -1892,5 +1895,16 @@ int cmd_stash(int argc, const char **argv, const char *prefix)\n> \t/* Assume 'stash push' */\n> \tstrvec_push(&args, \"push\");\n> \tstrvec_pushv(&args, argv);\n> -\treturn !!push_stash(args.nr, args.v, prefix, 1);\n> +\n> +\t/*\n> +\t * `push_stash()` ends up modifying the array, which causes memory\n> +\t * leaks if we didn't copy the array here.\n> +\t */\n> +\tDUP_ARRAY(args_copy, args.v, args.nr);\n> +\n> +\tret = !!push_stash(args.nr, args_copy, prefix, 1);\n> +\n> +\tstrvec_clear(&args);\n> +\tfree(args_copy);\n> +\treturn ret;\n> }\n\nOK, so this is the same pattern as we saw in the third patch of this\nseries. I agree with Junio's comments in that sub-thread, but also that\nthey are out-of-scope for the present series ;-).\n\nLooking good.\n\nThanks,\nTaylor\n"},{"id":"499725","messageId":"Zqpt0/6zBOpYh4aj@nand.local","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"Re: [PATCH 00/23] Memory leak fixes (pt.3)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-31T17:01:07Z","receivedAt":"2024-07-31T17:01:10Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 26, 2024 at 02:13:43PM +0200, Patrick Steinhardt wrote:\n> 57 files changed, 251 insertions(+), 73 deletions(-)\n\nI took a careful read through these patches, and found most of them easy\nto review. I was admittedly a little lost with the \"fix various leak\"\npatches, and having slightly more context in the commit descriptions\nthere would have been helpful.\n\nI think most of these patches look all good to me, but there is one\nwhere I think using a strvec would be a better fit than allocating our\nown array.\n\nOtherwise, these are looking in good shape. Thanks for working on this!\n\nThanks,\nTaylor\n"},{"id":"499748","messageId":"82f4bef1-57d9-4427-8316-5e63b573931d@gmail.com","threadId":"61850","inReplyTo":"5220c91bda7bc766368c2925499e5d244a03697b.1721995576.git.ps@pks.im","subject":"Re: [PATCH 07/23] builtin/submodule--helper: fix various trivial memory leaks","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-31T21:52:26Z","receivedAt":"2024-07-31T21:52:29Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Fri, Jul 26, 2024 at 02:15:09PM +0200, Patrick Steinhardt wrote:\n\n> There are multiple trivial memory leaks in the submodule helper. Fix\n> those.\n> \n> Signed-off-by: Patrick Steinhardt \n\nHi Patrick,\n\nWhile working on another series, I fixed some leaks. But, I haven't\nsent any patches.\n\nThe series this message belongs to fixes all the leaks that my unsent\nseries addresses. So, I won't bother sending my patches.\n\nHowever, I think we can do better for the second leak solved in this\npatch. I leave my patch at the end of this message in case you want\nto take a look.\n\nIn any case, thanks for working on this. I have the habit of having\n\"SANITIZE=leak\" in my config.mak, so this series will make me have\nfewer distractions :)\n\n> ---\n> builtin/submodule--helper.c | 13 ++++++++++---\n> t/t7400-submodule-basic.sh | 1 +\n> 2 files changed, 11 insertions(+), 3 deletions(-)\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index f1218a1995..5ae06c3e0b 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -2269,6 +2269,7 @@ static int is_tip_reachable(const char *path, const struct object_id *oid)\n> \tstruct child_process cp = CHILD_PROCESS_INIT;\n> \tstruct strbuf rev = STRBUF_INIT;\n> \tchar *hex = oid_to_hex(oid);\n> +\tint reachable;\n> \n> \tcp.git_cmd = 1;\n> \tcp.dir = path;\n> @@ -2278,9 +2279,12 @@ static int is_tip_reachable(const char *path, const struct object_id *oid)\n> \tprepare_submodule_repo_env(&cp.env);\n> \n> \tif (capture_command(&cp, &rev, GIT_MAX_HEXSZ + 1) || rev.len)\n> -\t\treturn 0;\n> +\t\treachable = 0;\n> +\telse\n> +\t\treachable = 1;\n> \n> -\treturn 1;\n> +\tstrbuf_release(&rev);\n> +\treturn reachable;\n> }\n> \n> static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n> @@ -3135,6 +3139,7 @@ static void append_fetch_remotes(struct strbuf *msg, const char *git_dir_path)\n> static int add_submodule(const struct add_data *add_data)\n> {\n> \tchar *submod_gitdir_path;\n> +\tchar *depth_to_free = NULL;\n> \tstruct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;\n> \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n> \tint ret = -1;\n> @@ -3200,7 +3205,7 @@ static int add_submodule(const struct add_data *add_data)\n> \t\t}\n> \t\tclone_data.dissociate = add_data->dissociate;\n> \t\tif (add_data->depth >= 0)\n> -\t\t\tclone_data.depth = xstrfmt(\"%d\", add_data->depth);\n> +\t\t\tclone_data.depth = depth_to_free = xstrfmt(\"%d\", add_data->depth);\n> \n> \t\tif (clone_submodule(&clone_data, &reference))\n> \t\t\tgoto cleanup;\n> @@ -3223,7 +3228,9 @@ static int add_submodule(const struct add_data *add_data)\n> \t\t\tdie(_(\"unable to checkout submodule '%s'\"), add_data->sm_path);\n> \t}\n> \tret = 0;\n> +\n> cleanup:\n> +\tfree(depth_to_free);\n> \tstring_list_clear(&reference, 1);\n> \treturn ret;\n> }\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index 981488885f..098d8833b6 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -12,6 +12,7 @@ subcommands of git submodule.\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_expect_success 'setup - enable local submodules' '\n> -- \n> 2.46.0.rc1.dirty\n> \n\n----- >8 --------- >8 --------- >8 --------- >8 -----\nSubject: [PATCH] submodule--helper: depth clone parameter\n\nThe `clone` subcommand has an OPT_STRING parameter: `--depth`, that\neventually is used when invoking `git-clone`.\n\nAs `git-clone` only support positive integer values for `--depth`\nsince 5594bcad21 (clone,fetch: catch non positive `--depth` option\nvalue, 2013-12-05), seems safe to change the `--depth` parameter to\nOPT_INTEGER in `module_clone`.\n\nDoing so we avoid a leak in `add_submodule`.\n\nDo it.\n\nSigned-off-by: Rubén Justo \n---\n builtin/submodule--helper.c | 12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f1218a1995..e7d52f38d9 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1530,7 +1530,7 @@ struct module_clone_data {\n \tconst char *path;\n \tconst char *name;\n \tconst char *url;\n-\tconst char *depth;\n+\tint depth;\n \tstruct list_objects_filter_options *filter_options;\n \tunsigned int quiet: 1;\n \tunsigned int progress: 1;\n@@ -1729,8 +1729,8 @@ static int clone_submodule(const struct module_clone_data *clone_data,\n \t\t\tstrvec_push(&cp.args, \"--quiet\");\n \t\tif (clone_data->progress)\n \t\t\tstrvec_push(&cp.args, \"--progress\");\n-\t\tif (clone_data->depth && *(clone_data->depth))\n-\t\t\tstrvec_pushl(&cp.args, \"--depth\", clone_data->depth, NULL);\n+\t\tif (clone_data->depth > 0)\n+\t\t\tstrvec_pushf(&cp.args, \"--depth=%d\", clone_data->depth);\n \t\tif (reference->nr) {\n \t\t\tstruct string_list_item *item;\n \n@@ -1851,8 +1851,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"reference repository\")),\n \t\tOPT_BOOL(0, \"dissociate\", &dissociate,\n \t\t\t N_(\"use --reference only while cloning\")),\n-\t\tOPT_STRING(0, \"depth\", &clone_data.depth,\n-\t\t\t N_(\"string\"),\n+\t\tOPT_INTEGER(0, \"depth\", &clone_data.depth,\n \t\t\t N_(\"depth for shallow clones\")),\n \t\tOPT__QUIET(&quiet, \"suppress output for cloning a submodule\"),\n \t\tOPT_BOOL(0, \"progress\", &progress,\n@@ -3199,8 +3198,7 @@ static int add_submodule(const struct add_data *add_data)\n \t\t\tstring_list_append(&reference, p)->util = p;\n \t\t}\n \t\tclone_data.dissociate = add_data->dissociate;\n-\t\tif (add_data->depth >= 0)\n-\t\t\tclone_data.depth = xstrfmt(\"%d\", add_data->depth);\n+\t\tclone_data.depth = add_data->depth;\n \n \t\tif (clone_submodule(&clone_data, &reference))\n \t\t\tgoto cleanup;\n-- \n2.45.1\n"},{"id":"499771","messageId":"ZqtFBTEDMsv1A91Q@tanuki","threadId":"61850","inReplyTo":"Zqpn0bLD+aLgY9PN@nand.local","subject":"Re: [PATCH 08/23] builtin/ls-remote: fix leaking `pattern` strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T08:19:17Z","receivedAt":"2024-08-01T08:19:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jul 31, 2024 at 12:35:29PM -0400, Taylor Blau wrote:\n> On Fri, Jul 26, 2024 at 02:15:24PM +0200, Patrick Steinhardt wrote:\n> > Users can pass patterns to git-ls-remote(1), which allows them to filter\n> > the list of printed references. We assemble those patterns into an array\n> > and prefix them with \"*/\", but never free either the array nor the\n> > allocated strings.\n> >\n> > Fix those leaks.\n> >\n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> > builtin/ls-remote.c | 11 +++++++----\n> > t/t5535-fetch-push-symref.sh | 1 +\n> > 2 files changed, 8 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\n> > index debf2d4f88..500f76fe4c 100644\n> > --- a/builtin/ls-remote.c\n> > +++ b/builtin/ls-remote.c\n> > @@ -47,7 +47,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n> > \tint status = 0;\n> > \tint show_symref_target = 0;\n> > \tconst char *uploadpack = NULL;\n> > -\tconst char **pattern = NULL;\n> > +\tchar **pattern = NULL;\n> > \tstruct transport_ls_refs_options transport_options =\n> > \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n> > \tint i;\n> > @@ -96,9 +96,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n> > \tif (argc > 1) {\n> > \t\tint i;\n> > \t\tCALLOC_ARRAY(pattern, argc);\n> > -\t\tfor (i = 1; i < argc; i++) {\n> > +\t\tfor (i = 1; i < argc; i++)\n> > \t\t\tpattern[i - 1] = xstrfmt(\"*/%s\", argv[i]);\n> > -\t\t}\n> \n> Instead of xstrfmt()-ing these strings directly, it may be more\n> ergonomic to use a strvec here and call strvec_pushf(). That would save\n> you explicitly CALLOC'ing the array, and would also save you the\n> explicit free() on each element of pattern below.\n\nAgreed, that reads way better. Thanks!\n\nPatrick\n"},{"id":"499772","messageId":"ZqtFIblPxqoXww-p@tanuki","threadId":"61850","inReplyTo":"Zqpt0/6zBOpYh4aj@nand.local","subject":"Re: [PATCH 00/23] Memory leak fixes (pt.3)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T08:19:45Z","receivedAt":"2024-08-01T08:19:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jul 31, 2024 at 01:01:07PM -0400, Taylor Blau wrote:\n> On Fri, Jul 26, 2024 at 02:13:43PM +0200, Patrick Steinhardt wrote:\n> > 57 files changed, 251 insertions(+), 73 deletions(-)\n> \n> I took a careful read through these patches, and found most of them easy\n> to review. I was admittedly a little lost with the \"fix various leak\"\n> patches, and having slightly more context in the commit descriptions\n> there would have been helpful.\n\nYeah, I was a bit torn here on whether to expand the commit messages or\nnot. I just didn't think it's all that useful to always say \"Variable x\nis allocated, but never freed\" for trivial cases where allocation and\nfreeing need to happen in the same function, much less so if we repeat\nsuch commits for every single such variable in the same subsystem. So I\njust threw those fixes into a single bag.\n\nPatrick\n"},{"id":"499773","messageId":"ZqtFMYEGuKRu1amZ@tanuki","threadId":"61850","inReplyTo":"82f4bef1-57d9-4427-8316-5e63b573931d@gmail.com","subject":"Re: [PATCH 07/23] builtin/submodule--helper: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T08:20:01Z","receivedAt":"2024-08-01T08:20:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jul 31, 2024 at 11:52:26PM +0200, Rubén Justo wrote:\n> On Fri, Jul 26, 2024 at 02:15:09PM +0200, Patrick Steinhardt wrote:\n> \n> > There are multiple trivial memory leaks in the submodule helper. Fix\n> > those.\n> > \n> > Signed-off-by: Patrick Steinhardt \n> \n> Hi Patrick,\n> \n> While working on another series, I fixed some leaks. But, I haven't\n> sent any patches.\n> \n> The series this message belongs to fixes all the leaks that my unsent\n> series addresses. So, I won't bother sending my patches.\n> \n> However, I think we can do better for the second leak solved in this\n> patch. I leave my patch at the end of this message in case you want\n> to take a look.\n\nAgreed, your patch looks way neater. I'll massage the patch a bit and\nthen add it to the series with an \"Original-patch-by\" trailer.\n\nPatrick\n"},{"id":"499798","messageId":"cover.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1721995576.git.ps@pks.im","subject":"[PATCH v2 00/24] Memory leak fixes (pt.3)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:38:06Z","receivedAt":"2024-08-01T10:38:13Z","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 patch series that plugs another round\nof memory leaks in Git. Changes compared to v2:\n\n - Various typo fixes.\n\n - Adopted a patch by Rubén that plugs a git-submodule--helper(1) leak\n in a neater way by using an integer to track the clone depth instead\n of using a (possibly allocated) string.\n\n - Refactored git-ls-remote(1) to use a `struct strvec` to track\n patterns instead of manually populating an array, which makes the\n code easier to reason about while still plugging the memory leak.\n This was suggested by Taylor.\n\nThanks for all the reviews so far!\n\nPatrick\n\nPatrick Steinhardt (23):\n builtin/replay: plug leaking `advance_name` variable\n builtin/log: fix leaking branch name when creating cover letters\n builtin/describe: fix memory leak with `--contains=`\n builtin/describe: fix leaking array when running diff-index\n builtin/describe: fix trivial memory leak when describing blob\n builtin/name-rev: fix various trivial memory leaks\n builtin/submodule--helper: fix leaking buffer in `is_tip_reachable`\n builtin/ls-remote: fix leaking `pattern` strings\n builtin/remote: fix leaking strings in `branch_list`\n builtin/remote: fix various trivial memory leaks\n builtin/stash: fix various trivial memory leaks\n builtin/rev-parse: fix memory leak with `--parseopt`\n builtin/show-branch: fix several memory leaks\n builtin/credential-store: fix leaking credential\n builtin/rerere: fix various trivial memory leaks\n builtin/shortlog: fix various trivial memory leaks\n builtin/worktree: fix leaking derived branch names\n builtin/credential-cache: fix trivial leaks\n t/test-repository: fix leaking repository\n object-name: fix leaking commit list items\n entry: fix leaking pathnames during delayed checkout\n convert: fix leaking config strings\n commit-reach: fix trivial memory leak when computing reachability\n\nRubén Justico (1):\n builtin/submodule--helper: fix leaking clone depth parameter\n\n builtin/credential-cache.c | 9 ++++-\n builtin/credential-store.c | 1 +\n builtin/describe.c | 25 ++++++++++--\n builtin/log.c | 4 +-\n builtin/ls-remote.c | 24 +++++-------\n builtin/name-rev.c | 6 ++-\n builtin/remote.c | 44 ++++++++++++++++-----\n builtin/replay.c | 20 +++++++---\n builtin/rerere.c | 8 +++-\n builtin/rev-parse.c | 5 ++-\n builtin/shortlog.c | 1 +\n builtin/show-branch.c | 52 +++++++++++++++++--------\n builtin/stash.c | 18 ++++++++-\n builtin/submodule--helper.c | 20 ++++++----\n builtin/worktree.c | 7 ++--\n commit-reach.c | 1 +\n convert.c | 14 +++++--\n entry.c | 4 +-\n object-name.c | 26 ++++++++-----\n rerere.c | 9 ++++-\n t/helper/test-repository.c | 4 +-\n t/t0021-conversion.sh | 1 +\n t/t0301-credential-cache.sh | 2 +\n t/t0302-credential-store.sh | 2 +\n t/t0303-credential-external.sh | 1 +\n t/t1502-rev-parse-parseopt.sh | 2 +\n t/t1511-rev-parse-caret.sh | 1 +\n t/t2030-unresolve-info.sh | 1 +\n t/t2080-parallel-checkout-basics.sh | 1 +\n t/t2082-parallel-checkout-attributes.sh | 1 +\n t/t2400-worktree-add.sh | 1 +\n t/t2501-cwd-empty.sh | 1 +\n t/t3201-branch-contains.sh | 1 +\n t/t3202-show-branch.sh | 1 +\n t/t3206-range-diff.sh | 1 +\n t/t3650-replay-basics.sh | 1 +\n t/t3903-stash.sh | 1 +\n t/t3904-stash-patch.sh | 2 +\n t/t3905-stash-include-untracked.sh | 1 +\n t/t4200-rerere.sh | 1 +\n t/t4201-shortlog.sh | 1 +\n t/t5318-commit-graph.sh | 2 +\n t/t5512-ls-remote.sh | 1 +\n t/t5514-fetch-multiple.sh | 1 +\n t/t5520-pull.sh | 1 +\n t/t5528-push-default.sh | 1 +\n t/t5535-fetch-push-symref.sh | 1 +\n t/t5543-atomic-push.sh | 1 +\n t/t5570-git-daemon.sh | 1 +\n t/t6007-rev-list-cherry-pick-file.sh | 1 +\n t/t6010-merge-base.sh | 1 +\n t/t6120-describe.sh | 1 +\n t/t6133-pathspec-rev-dwim.sh | 2 +\n t/t7064-wtstatus-pv2.sh | 1 +\n t/t7400-submodule-basic.sh | 1 +\n t/t9902-completion.sh | 1 +\n t/t9903-bash-prompt.sh | 1 +\n 57 files changed, 256 insertions(+), 88 deletions(-)\n\nRange-diff against v1:\n 1: dd044eacc2 = 1: b5c8a9495a builtin/replay: plug leaking `advance_name` variable\n 2: 4daae88877 ! 2: 73a16fff43 builtin/log: fix leaking branch name when creating cover letters\n @@ Metadata\n ## Commit message ##\n builtin/log: fix leaking branch name when creating cover letters\n \n - When calling `make_cover_letter()` without a branch name, then we try to\n + When calling `make_cover_letter()` without a branch name, we try to\n derive the branch name by calling `find_branch_name()`. But while this\n function returns an allocated string, we never free the result and thus\n have a memory leak. Fix this.\n -: ---------- > 3: 65027d13b7 builtin/describe: fix memory leak with `--contains=`\n 4: e8d86c01cf ! 4: 19ca97e33a builtin/describe: fix leaking array when running diff-index\n @@ Commit message\n builtin/describe: fix leaking array when running diff-index\n \n When running git-describe(1) with `--dirty`, we will set up a `struct\n - rev_info` with arguments for git-diff-index(1). The way we assmble the\n + rev_info` with arguments for git-diff-index(1). The way we assemble the\n arguments it causes two memory leaks though:\n \n - We never release the `struct strvec`.\n 5: 8aba7434bd = 5: 6deacb8667 builtin/describe: fix trivial memory leak when describing blob\n 6: 088f730572 = 6: e38e9d1b62 builtin/name-rev: fix various trivial memory leaks\n 3: 08a12be13c ! 7: d3eb0372bd builtin/describe: fix memory leak with `--contains=`\n @@\n ## Metadata ##\n -Author: Patrick Steinhardt \n +Author: Rubén Justico \n \n ## Commit message ##\n - builtin/describe: fix memory leak with `--contains=`\n + builtin/submodule--helper: fix leaking clone depth parameter\n \n - When calling `git describe --contains=`, we end up invoking\n - `cmd_name_rev()` with some munged argv array. This array may contain\n - allocated strings and furthermore will likely be modified by the called\n - function. This results in two memory leaks:\n + The submodule helper supports a `--depth` parameter for both its \"add\"\n + and \"clone\" subcommands, which in both cases end up being forwarded to\n + git-clone(1). But while the former subcommand uses an `OPT_INTEGER()` to\n + parse the depth, the latter uses `OPT_STRING()`. Consequently, it is\n + possible to pass non-integer input to \"--depth\" when calling the \"clone\"\n + subcommand, where the value will then ultimately cause git-clone(1) to\n + bail out.\n \n - - First, we leak the array that we use to assemble the arguments.\n + Besides the fact that the parameter verification should happen earlier,\n + the submodule helper infrastructure also internally tracks the depth via\n + a string. This requires us to convert the integer in the \"add\"\n + subcommand into an allocated string, and this string ultimately leaks.\n \n - - Second, we leak the allocated strings that we may have put into the\n - array.\n -\n - Fix those leaks by creating a separate copy of the array that we can\n - hand over to `cmd_name_rev()`. This allows us to free all strings\n - contained in the `strvec`, as the original vector will not be modified\n - anymore.\n -\n - Furthermore, free both the `strvec` and the copied array to fix the\n - first memory leak.\n + Refactor the code to consistently track the clone depth as an integer.\n + This plugs the memory leak, simplifies the code and allows us to use\n + `OPT_INTEGER()` instead of `OPT_STRING()`, validating the input before\n + we shell out to git--clone(1).\n \n + Original-patch-by: Rubén Justo \n Signed-off-by: Patrick Steinhardt \n \n - ## builtin/describe.c ##\n -@@ builtin/describe.c: int cmd_describe(int argc, const char **argv, const char *prefix)\n - \tif (contains) {\n - \t\tstruct string_list_item *item;\n - \t\tstruct strvec args;\n -+\t\tconst char **argv_copy;\n -+\t\tint ret;\n + ## builtin/submodule--helper.c ##\n +@@ builtin/submodule--helper.c: struct module_clone_data {\n + \tconst char *path;\n + \tconst char *name;\n + \tconst char *url;\n +-\tconst char *depth;\n ++\tint depth;\n + \tstruct list_objects_filter_options *filter_options;\n + \tunsigned int quiet: 1;\n + \tunsigned int progress: 1;\n +@@ builtin/submodule--helper.c: static int clone_submodule(const struct module_clone_data *clone_data,\n + \t\t\tstrvec_push(&cp.args, \"--quiet\");\n + \t\tif (clone_data->progress)\n + \t\t\tstrvec_push(&cp.args, \"--progress\");\n +-\t\tif (clone_data->depth && *(clone_data->depth))\n +-\t\t\tstrvec_pushl(&cp.args, \"--depth\", clone_data->depth, NULL);\n ++\t\tif (clone_data->depth > 0)\n ++\t\t\tstrvec_pushf(&cp.args, \"--depth=%d\", clone_data->depth);\n + \t\tif (reference->nr) {\n + \t\t\tstruct string_list_item *item;\n \n - \t\tstrvec_init(&args);\n - \t\tstrvec_pushl(&args, \"name-rev\",\n -@@ builtin/describe.c: int cmd_describe(int argc, const char **argv, const char *prefix)\n - \t\t\tstrvec_pushv(&args, argv);\n - \t\telse\n - \t\t\tstrvec_push(&args, \"HEAD\");\n --\t\treturn cmd_name_rev(args.nr, args.v, prefix);\n -+\n -+\t\t/*\n -+\t\t * `cmd_name_rev()` modifies the array, so we'd link its\n -+\t\t * contained strings if we didn't do a copy here.\n -+\t\t */\n -+\t\tALLOC_ARRAY(argv_copy, args.nr + 1);\n -+\t\tfor (size_t i = 0; i < args.nr; i++)\n -+\t\t\targv_copy[i] = args.v[i];\n -+\t\targv_copy[args.nr] = NULL;\n -+\n -+\t\tret = cmd_name_rev(args.nr, argv_copy, prefix);\n -+\n -+\t\tstrvec_clear(&args);\n -+\t\tfree(argv_copy);\n -+\t\treturn ret;\n - \t}\n +@@ builtin/submodule--helper.c: static int module_clone(int argc, const char **argv, const char *prefix)\n + \t\t\t N_(\"reference repository\")),\n + \t\tOPT_BOOL(0, \"dissociate\", &dissociate,\n + \t\t\t N_(\"use --reference only while cloning\")),\n +-\t\tOPT_STRING(0, \"depth\", &clone_data.depth,\n +-\t\t\t N_(\"string\"),\n ++\t\tOPT_INTEGER(0, \"depth\", &clone_data.depth,\n + \t\t\t N_(\"depth for shallow clones\")),\n + \t\tOPT__QUIET(&quiet, \"suppress output for cloning a submodule\"),\n + \t\tOPT_BOOL(0, \"progress\", &progress,\n +@@ builtin/submodule--helper.c: static int add_submodule(const struct add_data *add_data)\n + \t\t}\n + \t\tclone_data.dissociate = add_data->dissociate;\n + \t\tif (add_data->depth >= 0)\n +-\t\t\tclone_data.depth = xstrfmt(\"%d\", add_data->depth);\n ++\t\t\tclone_data.depth = add_data->depth;\n \n - \thashmap_init(&names, commit_name_neq, NULL, 0);\n + \t\tif (clone_submodule(&clone_data, &reference))\n + \t\t\tgoto cleanup;\n 7: 5220c91bda ! 8: 49c156cebb builtin/submodule--helper: fix various trivial memory leaks\n @@ Metadata\n Author: Patrick Steinhardt \n \n ## Commit message ##\n - builtin/submodule--helper: fix various trivial memory leaks\n + builtin/submodule--helper: fix leaking buffer in `is_tip_reachable`\n \n - There are multiple trivial memory leaks in the submodule helper. Fix\n - those.\n + The `rev` buffer in `is_tip_reachable()` is being populated with the\n + output of git-rev-list(1) -- if either the command fails or the buffer\n + contains any data, then the input commit is not reachable.\n +\n + The buffer isn't used for anything else, but neither do we free it,\n + causing a memory leak. Fix this.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ builtin/submodule--helper.c: static int is_tip_reachable(const char *path, const\n }\n \n static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n -@@ builtin/submodule--helper.c: static void append_fetch_remotes(struct strbuf *msg, const char *git_dir_path)\n - static int add_submodule(const struct add_data *add_data)\n - {\n - \tchar *submod_gitdir_path;\n -+\tchar *depth_to_free = NULL;\n - \tstruct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;\n - \tstruct string_list reference = STRING_LIST_INIT_NODUP;\n - \tint ret = -1;\n -@@ builtin/submodule--helper.c: static int add_submodule(const struct add_data *add_data)\n - \t\t}\n - \t\tclone_data.dissociate = add_data->dissociate;\n - \t\tif (add_data->depth >= 0)\n --\t\t\tclone_data.depth = xstrfmt(\"%d\", add_data->depth);\n -+\t\t\tclone_data.depth = depth_to_free = xstrfmt(\"%d\", add_data->depth);\n - \n - \t\tif (clone_submodule(&clone_data, &reference))\n - \t\t\tgoto cleanup;\n @@ builtin/submodule--helper.c: static int add_submodule(const struct add_data *add_data)\n \t\t\tdie(_(\"unable to checkout submodule '%s'\"), add_data->sm_path);\n \t}\n \tret = 0;\n +\n cleanup:\n -+\tfree(depth_to_free);\n \tstring_list_clear(&reference, 1);\n \treturn ret;\n - }\n \n ## t/t7400-submodule-basic.sh ##\n @@ t/t7400-submodule-basic.sh: subcommands of git submodule.\n 8: d42152654b ! 9: 4330c80905 builtin/ls-remote: fix leaking `pattern` strings\n @@ Commit message\n and prefix them with \"*/\", but never free either the array nor the\n allocated strings.\n \n - Fix those leaks.\n + Refactor the code to use a `struct strvec` instead of manually tracking\n + the strings in an array. Like this, we can easily use `strvec_clear()`\n + to release both the vector and the contained string for us, plugging the\n + leak.\n \n + Helped-by: Taylor Blau \n Signed-off-by: Patrick Steinhardt \n \n ## builtin/ls-remote.c ##\n +@@ builtin/ls-remote.c: static const char * const ls_remote_usage[] = {\n + * Is there one among the list of patterns that match the tail part\n + * of the path?\n + */\n +-static int tail_match(const char **pattern, const char *path)\n ++static int tail_match(const struct strvec *pattern, const char *path)\n + {\n +-\tconst char *p;\n + \tchar *pathbuf;\n + \n +-\tif (!pattern)\n ++\tif (!pattern->nr)\n + \t\treturn 1; /* no restriction */\n + \n + \tpathbuf = xstrfmt(\"/%s\", path);\n +-\twhile ((p = *(pattern++)) != NULL) {\n +-\t\tif (!wildmatch(p, pathbuf, 0)) {\n ++\tfor (size_t i = 0; i < pattern->nr; i++) {\n ++\t\tif (!wildmatch(pattern->v[i], pathbuf, 0)) {\n + \t\t\tfree(pathbuf);\n + \t\t\treturn 1;\n + \t\t}\n @@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tint status = 0;\n \tint show_symref_target = 0;\n \tconst char *uploadpack = NULL;\n -\tconst char **pattern = NULL;\n -+\tchar **pattern = NULL;\n ++\tstruct strvec pattern = STRVEC_INIT;\n \tstruct transport_ls_refs_options transport_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tint i;\n @@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n - \tif (argc > 1) {\n - \t\tint i;\n - \t\tCALLOC_ARRAY(pattern, argc);\n + \n + \tpacket_trace_identity(\"ls-remote\");\n + \n +-\tif (argc > 1) {\n +-\t\tint i;\n +-\t\tCALLOC_ARRAY(pattern, argc);\n -\t\tfor (i = 1; i < argc; i++) {\n -+\t\tfor (i = 1; i < argc; i++)\n - \t\t\tpattern[i - 1] = xstrfmt(\"*/%s\", argv[i]);\n +-\t\t\tpattern[i - 1] = xstrfmt(\"*/%s\", argv[i]);\n -\t\t}\n - \t}\n +-\t}\n ++\tfor (int i = 1; i < argc; i++)\n ++\t\tstrvec_pushf(&pattern, \"*/%s\", argv[i]);\n \n \tif (flags & REF_TAGS)\n + \t\tstrvec_push(&transport_options.ref_prefixes, \"refs/tags/\");\n @@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tstruct ref_array_item *item;\n \t\tif (!check_ref_type(ref, flags))\n \t\t\tcontinue;\n -\t\tif (!tail_match(pattern, ref->name))\n -+\t\tif (!tail_match((const char **) pattern, ref->name))\n ++\t\tif (!tail_match(&pattern, ref->name))\n \t\t\tcontinue;\n \t\titem = ref_array_push(&ref_array, ref->name, &ref->old_oid);\n \t\titem->symref = xstrdup_or_null(ref->symref);\n @@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *\n \t\tstatus = 1;\n \ttransport_ls_refs_options_release(&transport_options);\n +\n -+\tfor (i = 1; i < argc; i++)\n -+\t\tfree(pattern[i - 1]);\n -+\tfree(pattern);\n ++\tstrvec_clear(&pattern);\n \treturn status;\n }\n \n 9: 6952fb2ff2 = 10: 2e3f76b428 builtin/remote: fix leaking strings in `branch_list`\n10: 8bedcfdad8 = 11: 384203c34b builtin/remote: fix various trivial memory leaks\n11: a4b3ca49c9 = 12: b7c0671797 builtin/stash: fix various trivial memory leaks\n12: e277222bd2 ! 13: 034c416d46 builtin/rev-parse: fix memory leak with `--parseopt`\n @@ Commit message\n which is of course wrong.\n \n Fix this by freeing all option's help fields as well as their `argh`\n - fielids to plug this memory leak.\n + fields to plug this memory leak.\n \n Signed-off-by: Patrick Steinhardt \n \n13: fd857efa9f = 14: 20a40e2fd4 builtin/show-branch: fix several memory leaks\n14: ba4a883ae1 = 15: b553f1ffb9 builtin/credential-store: fix leaking credential\n15: 6d49645c0f = 16: 76d81c8ae1 builtin/rerere: fix various trivial memory leaks\n16: 778e87221a = 17: d52ac1a550 builtin/shortlog: fix various trivial memory leaks\n17: 8a649bf7ed = 18: dccaf695a9 builtin/worktree: fix leaking derived branch names\n18: 2c7a369490 = 19: 788f0bee7b builtin/credential-cache: fix trivial leaks\n19: 7564911d2a = 20: eb58826b14 t/test-repository: fix leaking repository\n20: e3dbf9116b = 21: fa95a67ccd object-name: fix leaking commit list items\n21: e426fd7ec5 = 22: 8cae4c44f4 entry: fix leaking pathnames during delayed checkout\n22: 8c2a19994f = 23: 5d2bf0f0f3 convert: fix leaking config strings\n23: 60aef16aef = 24: 6db1829c79 commit-reach: fix trivial memory leak when computing reachability\n\nbase-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee\n-- \n2.46.0.dirty\n\n"},{"id":"499799","messageId":"b5c8a9495ae81ba0757c92df634117e836e89699.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 01/24] builtin/replay: plug leaking `advance_name` variable","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:38:11Z","receivedAt":"2024-08-01T10:38:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `advance_name` variable can either contain a static string when\nparsed via the `--advance` command line option or it may be an allocated\nstring when set via `determine_replay_mode()`. Because we cannot be sure\nwhether it is allocated or not we just didn't free it at all, resulting\nin a memory leak.\n\nSplit up the variables such that we can track the static and allocated\nstrings separately and then free the allocated one to fix the memory\nleak.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/replay.c | 20 ++++++++++++++------\n t/t3650-replay-basics.sh | 1 +\n 2 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/replay.c b/builtin/replay.c\nindex 0448326636..27b40eda98 100644\n--- a/builtin/replay.c\n+++ b/builtin/replay.c\n@@ -151,7 +151,7 @@ static void get_ref_information(struct rev_cmdline_info *cmd_info,\n \n static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\t\t\t const char *onto_name,\n-\t\t\t\t const char **advance_name,\n+\t\t\t\t char **advance_name,\n \t\t\t\t struct commit **onto,\n \t\t\t\t struct strset **update_refs)\n {\n@@ -174,6 +174,7 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\t*onto = peel_committish(*advance_name);\n \t\tif (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name),\n \t\t\t &oid, &fullname, 0) == 1) {\n+\t\t\tfree(*advance_name);\n \t\t\t*advance_name = fullname;\n \t\t} else {\n \t\t\tdie(_(\"argument to --advance must be a reference\"));\n@@ -197,6 +198,7 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\tif (negative_refs_complete) {\n \t\t\tstruct hashmap_iter iter;\n \t\t\tstruct strmap_entry *entry;\n+\t\t\tconst char *last_key = NULL;\n \n \t\t\tif (rinfo.negative_refexprs == 0)\n \t\t\t\tdie(_(\"all positive revisions given must be references\"));\n@@ -208,8 +210,11 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,\n \t\t\t/* Only one entry, but we have to loop to get it */\n \t\t\tstrset_for_each_entry(&rinfo.negative_refs,\n \t\t\t\t\t &iter, entry) {\n-\t\t\t\t*advance_name = entry->key;\n+\t\t\t\tlast_key = entry->key;\n \t\t\t}\n+\n+\t\t\tfree(*advance_name);\n+\t\t\t*advance_name = xstrdup_or_null(last_key);\n \t\t} else { /* positive_refs_complete */\n \t\t\tif (rinfo.negative_refexprs > 1)\n \t\t\t\tdie(_(\"cannot implicitly determine correct base for --onto\"));\n@@ -271,7 +276,8 @@ static struct commit *pick_regular_commit(struct commit *pickme,\n \n int cmd_replay(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *advance_name = NULL;\n+\tconst char *advance_name_opt = NULL;\n+\tchar *advance_name = NULL;\n \tstruct commit *onto = NULL;\n \tconst char *onto_name = NULL;\n \tint contained = 0;\n@@ -292,7 +298,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)\n \t\tNULL\n \t};\n \tstruct option replay_options[] = {\n-\t\tOPT_STRING(0, \"advance\", &advance_name,\n+\t\tOPT_STRING(0, \"advance\", &advance_name_opt,\n \t\t\t N_(\"branch\"),\n \t\t\t N_(\"make replay advance given branch\")),\n \t\tOPT_STRING(0, \"onto\", &onto_name,\n@@ -306,14 +312,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, replay_options, replay_usage,\n \t\t\t PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);\n \n-\tif (!onto_name && !advance_name) {\n+\tif (!onto_name && !advance_name_opt) {\n \t\terror(_(\"option --onto or --advance is mandatory\"));\n \t\tusage_with_options(replay_usage, replay_options);\n \t}\n \n-\tif (advance_name && contained)\n+\tif (advance_name_opt && contained)\n \t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n \t\t \"--advance\", \"--contained\");\n+\tadvance_name = xstrdup_or_null(advance_name_opt);\n \n \trepo_init_revisions(the_repository, &revs, prefix);\n \n@@ -441,6 +448,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)\n \n cleanup:\n \trelease_revisions(&revs);\n+\tfree(advance_name);\n \n \t/* Return */\n \tif (ret < 0)\ndiff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh\nindex 389670262e..12bd3db4cb 100755\n--- a/t/t3650-replay-basics.sh\n+++ b/t/t3650-replay-basics.sh\n@@ -5,6 +5,7 @@ test_description='basic git replay 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 \n GIT_AUTHOR_NAME=author@name\n-- \n2.46.0.dirty\n\n"},{"id":"499800","messageId":"73a16fff43ee36f497f3ea7377a68babd3b9d392.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 02/24] builtin/log: fix leaking branch name when creating cover letters","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:38:16Z","receivedAt":"2024-08-01T10:38:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `make_cover_letter()` without a branch name, we try to\nderive the branch name by calling `find_branch_name()`. But while this\nfunction returns an allocated string, we never free the result and thus\nhave a memory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/log.c | 4 +++-\n t/t3206-range-diff.sh | 1 +\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 4d4b60caa7..a73a767606 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1434,6 +1434,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \tint need_8bit_cte = 0;\n \tstruct pretty_print_context pp = {0};\n \tstruct commit *head = list[0];\n+\tchar *to_free = NULL;\n \n \tif (!cmit_fmt_is_mail(rev->commit_format))\n \t\tdie(_(\"cover letter needs email format\"));\n@@ -1455,7 +1456,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t}\n \n \tif (!branch_name)\n-\t\tbranch_name = find_branch_name(rev);\n+\t\tbranch_name = to_free = find_branch_name(rev);\n \n \tpp.fmt = CMIT_FMT_EMAIL;\n \tpp.date_mode.type = DATE_RFC2822;\n@@ -1466,6 +1467,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t encoding, need_8bit_cte, cfg);\n \tfprintf(rev->diffopt.file, \"%s\\n\", sb.buf);\n \n+\tfree(to_free);\n \tfree(pp.after_subject);\n \tstrbuf_release(&sb);\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex a767c3520e..973e20254b 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -5,6 +5,7 @@ test_description='range-diff 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 \n # Note that because of the range-diff's heuristics, test_commit does more\n-- \n2.46.0.dirty\n\n"},{"id":"499801","messageId":"65027d13b75a684d60112b12019871342939f2d7.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 03/24] builtin/describe: fix memory leak with `--contains=`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:38:20Z","receivedAt":"2024-08-01T10:38:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `git describe --contains=`, we end up invoking\n`cmd_name_rev()` with some munged argv array. This array may contain\nallocated strings and furthermore will likely be modified by the called\nfunction. This results in two memory leaks:\n\n - First, we leak the array that we use to assemble the arguments.\n\n - Second, we leak the allocated strings that we may have put into the\n array.\n\nFix those leaks by creating a separate copy of the array that we can\nhand over to `cmd_name_rev()`. This allows us to free all strings\ncontained in the `strvec`, as the original vector will not be modified\nanymore.\n\nFurthermore, free both the `strvec` and the copied array to fix the\nfirst memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/describe.c | 18 +++++++++++++++++-\n 1 file changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex cf8edc4222..739878db85 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -619,6 +619,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \tif (contains) {\n \t\tstruct string_list_item *item;\n \t\tstruct strvec args;\n+\t\tconst char **argv_copy;\n+\t\tint ret;\n \n \t\tstrvec_init(&args);\n \t\tstrvec_pushl(&args, \"name-rev\",\n@@ -637,7 +639,21 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\tstrvec_pushv(&args, argv);\n \t\telse\n \t\t\tstrvec_push(&args, \"HEAD\");\n-\t\treturn cmd_name_rev(args.nr, args.v, prefix);\n+\n+\t\t/*\n+\t\t * `cmd_name_rev()` modifies the array, so we'd leak its\n+\t\t * contained strings if we didn't do a copy here.\n+\t\t */\n+\t\tALLOC_ARRAY(argv_copy, args.nr + 1);\n+\t\tfor (size_t i = 0; i < args.nr; i++)\n+\t\t\targv_copy[i] = args.v[i];\n+\t\targv_copy[args.nr] = NULL;\n+\n+\t\tret = cmd_name_rev(args.nr, argv_copy, prefix);\n+\n+\t\tstrvec_clear(&args);\n+\t\tfree(argv_copy);\n+\t\treturn ret;\n \t}\n \n \thashmap_init(&names, commit_name_neq, NULL, 0);\n-- \n2.46.0.dirty\n\n"},{"id":"499802","messageId":"19ca97e33a352b540a27feae5302c2311b707f9c.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 04/24] builtin/describe: fix leaking array when running diff-index","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:38:25Z","receivedAt":"2024-08-01T10:38:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When running git-describe(1) with `--dirty`, we will set up a `struct\nrev_info` with arguments for git-diff-index(1). The way we assemble the\narguments it causes two memory leaks though:\n\n - We never release the `struct strvec`.\n\n - `setup_revisions()` may end up removing some entries from the\n `strvec`, which we wouldn't free even if we released the struct.\n\nWhile we could plug those leaks, this is ultimately unnecessary as the\narguments we pass are part of a static array anyway. So instead,\nrefactor the code to drop the `struct strvec` and just pass this static\narray directly.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/describe.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 739878db85..2957ff7031 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -695,7 +695,6 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t} else if (dirty) {\n \t\t\tstruct lock_file index_lock = LOCK_INIT;\n \t\t\tstruct rev_info revs;\n-\t\t\tstruct strvec args = STRVEC_INIT;\n \t\t\tint fd;\n \n \t\t\tsetup_work_tree();\n@@ -710,8 +709,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\t\trepo_update_index_if_able(the_repository, &index_lock);\n \n \t\t\trepo_init_revisions(the_repository, &revs, prefix);\n-\t\t\tstrvec_pushv(&args, diff_index_args);\n-\t\t\tif (setup_revisions(args.nr, args.v, &revs, NULL) != 1)\n+\n+\t\t\tif (setup_revisions(ARRAY_SIZE(diff_index_args) - 1,\n+\t\t\t\t\t diff_index_args, &revs, NULL) != 1)\n \t\t\t\tBUG(\"malformed internal diff-index command line\");\n \t\t\trun_diff_index(&revs, 0);\n \n-- \n2.46.0.dirty\n\n"},{"id":"499803","messageId":"6deacb8667bfe94cdb5995302490b977b7fd2e8a.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 05/24] builtin/describe: fix trivial memory leak when describing blob","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:38:30Z","receivedAt":"2024-08-01T10:38:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never free the `struct strvec args` variable in `describe_blob()`,\nwhich thus causes a memory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/describe.c | 1 +\n t/t9903-bash-prompt.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 2957ff7031..954929c514 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -529,6 +529,7 @@ static void describe_blob(struct object_id oid, struct strbuf *dst)\n \ttraverse_commit_list(&revs, process_commit, process_object, &pcd);\n \treset_revision_walk();\n \trelease_revisions(&revs);\n+\tstrvec_clear(&args);\n }\n \n static void describe(const char *arg, int last_one)\ndiff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh\nindex d667dda654..95e9955bca 100755\n--- a/t/t9903-bash-prompt.sh\n+++ b/t/t9903-bash-prompt.sh\n@@ -8,6 +8,7 @@ test_description='test git-specific bash prompt functions'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./lib-bash.sh\n \n . \"$GIT_BUILD_DIR/contrib/completion/git-prompt.sh\"\n-- \n2.46.0.dirty\n\n"},{"id":"499804","messageId":"e38e9d1b623fcd4cd0113771124d44d470a48126.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 06/24] builtin/name-rev: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:38:35Z","receivedAt":"2024-08-01T10:38:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several structures that we don't release after\n`cmd_name_rev()` is done. Plug those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/name-rev.c | 6 ++++--\n t/t6007-rev-list-cherry-pick-file.sh | 1 +\n t/t6120-describe.sh | 1 +\n 3 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 70e9ec4e47..f62c0a36cb 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -677,7 +677,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t\t\t\t always, allow_undefined, data.name_only);\n \t}\n \n-\tUNLEAK(string_pool);\n-\tUNLEAK(revs);\n+\tstring_list_clear(&data.ref_filters, 0);\n+\tstring_list_clear(&data.exclude_filters, 0);\n+\tmem_pool_discard(&string_pool, 0);\n+\tobject_array_clear(&revs);\n \treturn 0;\n }\ndiff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh\nindex 6f3e543977..2d337d7287 100755\n--- a/t/t6007-rev-list-cherry-pick-file.sh\n+++ b/t/t6007-rev-list-cherry-pick-file.sh\n@@ -5,6 +5,7 @@ test_description='test git rev-list --cherry-pick -- file'\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 # A---B---D---F\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 79e0f19deb..05ed2510d9 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -14,6 +14,7 @@ test_description='test describe'\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 check_describe () {\n-- \n2.46.0.dirty\n\n"},{"id":"499805","messageId":"49c156cebb12e387a49a763595f0846678883b7b.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 08/24] builtin/submodule--helper: fix leaking buffer in `is_tip_reachable`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:39:57Z","receivedAt":"2024-08-01T10:40:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `rev` buffer in `is_tip_reachable()` is being populated with the\noutput of git-rev-list(1) -- if either the command fails or the buffer\ncontains any data, then the input commit is not reachable.\n\nThe buffer isn't used for anything else, but neither do we free it,\ncausing a memory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/submodule--helper.c | 9 +++++++--\n t/t7400-submodule-basic.sh | 1 +\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 863b01627c..673810d2c0 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2268,6 +2268,7 @@ static int is_tip_reachable(const char *path, const struct object_id *oid)\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf rev = STRBUF_INIT;\n \tchar *hex = oid_to_hex(oid);\n+\tint reachable;\n \n \tcp.git_cmd = 1;\n \tcp.dir = path;\n@@ -2277,9 +2278,12 @@ static int is_tip_reachable(const char *path, const struct object_id *oid)\n \tprepare_submodule_repo_env(&cp.env);\n \n \tif (capture_command(&cp, &rev, GIT_MAX_HEXSZ + 1) || rev.len)\n-\t\treturn 0;\n+\t\treachable = 0;\n+\telse\n+\t\treachable = 1;\n \n-\treturn 1;\n+\tstrbuf_release(&rev);\n+\treturn reachable;\n }\n \n static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n@@ -3222,6 +3226,7 @@ static int add_submodule(const struct add_data *add_data)\n \t\t\tdie(_(\"unable to checkout submodule '%s'\"), add_data->sm_path);\n \t}\n \tret = 0;\n+\n cleanup:\n \tstring_list_clear(&reference, 1);\n \treturn ret;\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 981488885f..098d8833b6 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -12,6 +12,7 @@ subcommands of git submodule.\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_expect_success 'setup - enable local submodules' '\n-- \n2.46.0.dirty\n\n"},{"id":"499806","messageId":"4330c80905805d51439006e15c64d672ba376291.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 09/24] builtin/ls-remote: fix leaking `pattern` strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:02Z","receivedAt":"2024-08-01T10:40:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Users can pass patterns to git-ls-remote(1), which allows them to filter\nthe list of printed references. We assemble those patterns into an array\nand prefix them with \"*/\", but never free either the array nor the\nallocated strings.\n\nRefactor the code to use a `struct strvec` instead of manually tracking\nthe strings in an array. Like this, we can easily use `strvec_clear()`\nto release both the vector and the contained string for us, plugging the\nleak.\n\nHelped-by: Taylor Blau \nSigned-off-by: Patrick Steinhardt \n---\n builtin/ls-remote.c | 24 ++++++++++--------------\n t/t5535-fetch-push-symref.sh | 1 +\n 2 files changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex debf2d4f88..5b61af5d78 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -19,17 +19,16 @@ static const char * const ls_remote_usage[] = {\n * Is there one among the list of patterns that match the tail part\n * of the path?\n */\n-static int tail_match(const char **pattern, const char *path)\n+static int tail_match(const struct strvec *pattern, const char *path)\n {\n-\tconst char *p;\n \tchar *pathbuf;\n \n-\tif (!pattern)\n+\tif (!pattern->nr)\n \t\treturn 1; /* no restriction */\n \n \tpathbuf = xstrfmt(\"/%s\", path);\n-\twhile ((p = *(pattern++)) != NULL) {\n-\t\tif (!wildmatch(p, pathbuf, 0)) {\n+\tfor (size_t i = 0; i < pattern->nr; i++) {\n+\t\tif (!wildmatch(pattern->v[i], pathbuf, 0)) {\n \t\t\tfree(pathbuf);\n \t\t\treturn 1;\n \t\t}\n@@ -47,7 +46,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tint status = 0;\n \tint show_symref_target = 0;\n \tconst char *uploadpack = NULL;\n-\tconst char **pattern = NULL;\n+\tstruct strvec pattern = STRVEC_INIT;\n \tstruct transport_ls_refs_options transport_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tint i;\n@@ -93,13 +92,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \n \tpacket_trace_identity(\"ls-remote\");\n \n-\tif (argc > 1) {\n-\t\tint i;\n-\t\tCALLOC_ARRAY(pattern, argc);\n-\t\tfor (i = 1; i < argc; i++) {\n-\t\t\tpattern[i - 1] = xstrfmt(\"*/%s\", argv[i]);\n-\t\t}\n-\t}\n+\tfor (int i = 1; i < argc; i++)\n+\t\tstrvec_pushf(&pattern, \"*/%s\", argv[i]);\n \n \tif (flags & REF_TAGS)\n \t\tstrvec_push(&transport_options.ref_prefixes, \"refs/tags/\");\n@@ -136,7 +130,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tstruct ref_array_item *item;\n \t\tif (!check_ref_type(ref, flags))\n \t\t\tcontinue;\n-\t\tif (!tail_match(pattern, ref->name))\n+\t\tif (!tail_match(&pattern, ref->name))\n \t\t\tcontinue;\n \t\titem = ref_array_push(&ref_array, ref->name, &ref->old_oid);\n \t\titem->symref = xstrdup_or_null(ref->symref);\n@@ -158,5 +152,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tif (transport_disconnect(transport))\n \t\tstatus = 1;\n \ttransport_ls_refs_options_release(&transport_options);\n+\n+\tstrvec_clear(&pattern);\n \treturn status;\n }\ndiff --git a/t/t5535-fetch-push-symref.sh b/t/t5535-fetch-push-symref.sh\nindex e8f6d233ff..7122af7fdb 100755\n--- a/t/t5535-fetch-push-symref.sh\n+++ b/t/t5535-fetch-push-symref.sh\n@@ -2,6 +2,7 @@\n \n test_description='avoiding conflicting update through symref aliasing'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.dirty\n\n"},{"id":"499807","messageId":"2e3f76b428a0304d6e638dc4b8ff575dbd365691.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 10/24] builtin/remote: fix leaking strings in `branch_list`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:07Z","receivedAt":"2024-08-01T10:40:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct string_list branch_list` is declared as `NODUP`, which makes\nit not copy strings inserted into it. This causes memory leaks though,\nas this means it also won't be responsible for _freeing_ inserted\nstrings. Thus, every branch we add to this will leak.\n\nFix this by marking the list as `DUP` instead and free the local copy we\nhave of the variable.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/remote.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 08292498bd..303da7f73f 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -258,7 +258,7 @@ struct branch_info {\n \tchar *push_remote_name;\n };\n \n-static struct string_list branch_list = STRING_LIST_INIT_NODUP;\n+static struct string_list branch_list = STRING_LIST_INIT_DUP;\n \n static const char *abbrev_ref(const char *name, const char *prefix)\n {\n@@ -292,8 +292,8 @@ static int config_read_branches(const char *key, const char *value,\n \t\ttype = PUSH_REMOTE;\n \telse\n \t\treturn 0;\n-\tname = xmemdupz(key, key_len);\n \n+\tname = xmemdupz(key, key_len);\n \titem = string_list_insert(&branch_list, name);\n \n \tif (!item->util)\n@@ -337,6 +337,7 @@ static int config_read_branches(const char *key, const char *value,\n \t\tBUG(\"unexpected type=%d\", type);\n \t}\n \n+\tfree(name);\n \treturn 0;\n }\n \n-- \n2.46.0.dirty\n\n"},{"id":"499808","messageId":"384203c34ba1d747866dd782165bf81b6280e896.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 11/24] builtin/remote: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:12Z","receivedAt":"2024-08-01T10:40:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are multiple trivial memory leaks in git-remote(1). Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/remote.c | 39 ++++++++++++++++++++++++++++++++-------\n t/t5512-ls-remote.sh | 1 +\n t/t5514-fetch-multiple.sh | 1 +\n t/t5520-pull.sh | 1 +\n t/t5528-push-default.sh | 1 +\n t/t5543-atomic-push.sh | 1 +\n t/t5570-git-daemon.sh | 1 +\n 7 files changed, 38 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 303da7f73f..9d54fddf8c 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -555,13 +555,16 @@ static int add_branch_for_removal(const char *refname,\n \trefspec.dst = (char *)refname;\n \tif (remote_find_tracking(branches->remote, &refspec))\n \t\treturn 0;\n+\tfree(refspec.src);\n \n \t/* don't delete a branch if another remote also uses it */\n \tfor (kr = branches->keep->list; kr; kr = kr->next) {\n \t\tmemset(&refspec, 0, sizeof(refspec));\n \t\trefspec.dst = (char *)refname;\n-\t\tif (!remote_find_tracking(kr->remote, &refspec))\n+\t\tif (!remote_find_tracking(kr->remote, &refspec)) {\n+\t\t\tfree(refspec.src);\n \t\t\treturn 0;\n+\t\t}\n \t}\n \n \t/* don't delete non-remote-tracking refs */\n@@ -668,7 +671,11 @@ static int config_read_push_default(const char *key, const char *value,\n static void handle_push_default(const char* old_name, const char* new_name)\n {\n \tstruct push_default_info push_default = {\n-\t\told_name, CONFIG_SCOPE_UNKNOWN, STRBUF_INIT, -1 };\n+\t\t.old_name = old_name,\n+\t\t.scope = CONFIG_SCOPE_UNKNOWN,\n+\t\t.origin = STRBUF_INIT,\n+\t\t.linenr = -1,\n+\t};\n \tgit_config(config_read_push_default, &push_default);\n \tif (push_default.scope >= CONFIG_SCOPE_COMMAND)\n \t\t; /* pass */\n@@ -688,6 +695,8 @@ static void handle_push_default(const char* old_name, const char* new_name)\n \t\t\tpush_default.origin.buf, push_default.linenr,\n \t\t\told_name);\n \t}\n+\n+\tstrbuf_release(&push_default.origin);\n }\n \n \n@@ -785,7 +794,7 @@ static int mv(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (!refspec_updated)\n-\t\treturn 0;\n+\t\tgoto out;\n \n \t/*\n \t * First remove symrefs, then rename the rest, finally create\n@@ -851,10 +860,15 @@ static int mv(int argc, const char **argv, const char *prefix)\n \t\tdisplay_progress(progress, ++refs_renamed_nr);\n \t}\n \tstop_progress(&progress);\n-\tstring_list_clear(&remote_branches, 1);\n \n \thandle_push_default(rename.old_name, rename.new_name);\n \n+out:\n+\tstring_list_clear(&remote_branches, 1);\n+\tstrbuf_release(&old_remote_context);\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&buf2);\n+\tstrbuf_release(&buf3);\n \treturn 0;\n }\n \n@@ -945,12 +959,21 @@ static int rm(int argc, const char **argv, const char *prefix)\n \n \tif (!result) {\n \t\tstrbuf_addf(&buf, \"remote.%s\", remote->name);\n-\t\tif (git_config_rename_section(buf.buf, NULL) < 1)\n-\t\t\treturn error(_(\"Could not remove config section '%s'\"), buf.buf);\n+\t\tif (git_config_rename_section(buf.buf, NULL) < 1) {\n+\t\t\tresult = error(_(\"Could not remove config section '%s'\"), buf.buf);\n+\t\t\tgoto out;\n+\t\t}\n \n \t\thandle_push_default(remote->name, NULL);\n \t}\n \n+out:\n+\tfor (struct known_remote *r = known_remotes.list; r;) {\n+\t\tstruct known_remote *next = r->next;\n+\t\tfree(r);\n+\t\tr = next;\n+\t}\n+\tstrbuf_release(&buf);\n \treturn result;\n }\n \n@@ -983,8 +1006,10 @@ static int append_ref_to_tracked_list(const char *refname,\n \n \tmemset(&refspec, 0, sizeof(refspec));\n \trefspec.dst = (char *)refname;\n-\tif (!remote_find_tracking(states->remote, &refspec))\n+\tif (!remote_find_tracking(states->remote, &refspec)) {\n \t\tstring_list_append(&states->tracked, abbrev_branch(refspec.src));\n+\t\tfree(refspec.src);\n+\t}\n \n \treturn 0;\n }\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 42e77eb5a9..d687d824d1 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -5,6 +5,7 @@ test_description='git ls-remote'\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 generate_references () {\ndiff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh\nindex 25772c85c5..579872c258 100755\n--- a/t/t5514-fetch-multiple.sh\n+++ b/t/t5514-fetch-multiple.sh\n@@ -5,6 +5,7 @@ test_description='fetch --all works correctly'\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 setup_repository () {\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 47534f1062..1098cbd0a1 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -5,6 +5,7 @@ test_description='pulling into void'\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 modify () {\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 14f7eced9a..bc2bada34c 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -4,6 +4,7 @@ test_description='check various push.default settings'\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_expect_success 'setup bare remotes' '\ndiff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh\nindex 04b47ad84a..479d103469 100755\n--- a/t/t5543-atomic-push.sh\n+++ b/t/t5543-atomic-push.sh\n@@ -5,6 +5,7 @@ test_description='pushing to a repository using the atomic push option'\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 mk_repo_pair () {\ndiff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh\nindex f9a9bf9503..c5f08b6799 100755\n--- a/t/t5570-git-daemon.sh\n+++ b/t/t5570-git-daemon.sh\n@@ -4,6 +4,7 @@ test_description='test fetching over git protocol'\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_DIRECTORY\"/lib-git-daemon.sh\n-- \n2.46.0.dirty\n\n"},{"id":"499809","messageId":"b7c0671797ecf3e19c7a8ef1941b920b1c2566e9.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 12/24] builtin/stash: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:17Z","receivedAt":"2024-08-01T10:40:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are multiple trivial memory leaks in git-stash(1). Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/stash.c | 18 ++++++++++++++++--\n t/t2501-cwd-empty.sh | 1 +\n t/t3903-stash.sh | 1 +\n t/t3904-stash-patch.sh | 2 ++\n t/t3905-stash-include-untracked.sh | 1 +\n t/t7064-wtstatus-pv2.sh | 1 +\n 6 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 46b981c4dd..7f2f989b69 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1521,6 +1521,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \tstruct strbuf patch = STRBUF_INIT;\n \tstruct strbuf stash_msg_buf = STRBUF_INIT;\n \tstruct strbuf untracked_files = STRBUF_INIT;\n+\tstruct strbuf out = STRBUF_INIT;\n \n \tif (patch_mode && keep_index == -1)\n \t\tkeep_index = 1;\n@@ -1626,7 +1627,6 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \t\t\tstruct child_process cp_add = CHILD_PROCESS_INIT;\n \t\t\tstruct child_process cp_diff = CHILD_PROCESS_INIT;\n \t\t\tstruct child_process cp_apply = CHILD_PROCESS_INIT;\n-\t\t\tstruct strbuf out = STRBUF_INIT;\n \n \t\t\tcp_add.git_cmd = 1;\n \t\t\tstrvec_push(&cp_add.args, \"add\");\n@@ -1718,6 +1718,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \n done:\n \tstrbuf_release(&patch);\n+\tstrbuf_release(&out);\n \tfree_stash_info(&info);\n \tstrbuf_release(&stash_msg_buf);\n \tstrbuf_release(&untracked_files);\n@@ -1869,6 +1870,8 @@ int cmd_stash(int argc, const char **argv, const char *prefix)\n \t\tOPT_SUBCOMMAND_F(\"save\", &fn, save_stash, PARSE_OPT_NOCOMPLETE),\n \t\tOPT_END()\n \t};\n+\tconst char **args_copy;\n+\tint ret;\n \n \tgit_config(git_stash_config, NULL);\n \n@@ -1892,5 +1895,16 @@ int cmd_stash(int argc, const char **argv, const char *prefix)\n \t/* Assume 'stash push' */\n \tstrvec_push(&args, \"push\");\n \tstrvec_pushv(&args, argv);\n-\treturn !!push_stash(args.nr, args.v, prefix, 1);\n+\n+\t/*\n+\t * `push_stash()` ends up modifying the array, which causes memory\n+\t * leaks if we didn't copy the array here.\n+\t */\n+\tDUP_ARRAY(args_copy, args.v, args.nr);\n+\n+\tret = !!push_stash(args.nr, args_copy, prefix, 1);\n+\n+\tstrvec_clear(&args);\n+\tfree(args_copy);\n+\treturn ret;\n }\ndiff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh\nindex f6d8d7d03d..8af4e8cfe3 100755\n--- a/t/t2501-cwd-empty.sh\n+++ b/t/t2501-cwd-empty.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test handling of the current working directory becoming empty'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex a7f71f8126..e4c0937f61 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -8,6 +8,7 @@ test_description='Test git stash'\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-unique-files.sh\n \ndiff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\nindex 368fc2a6cc..aa5019fd6c 100755\n--- a/t/t3904-stash-patch.sh\n+++ b/t/t3904-stash-patch.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='stash -p'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./lib-patch-mode.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 1289ae3e07..a1733f45c3 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -5,6 +5,7 @@\n \n test_description='Test git stash --include-untracked'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'stash save --include-untracked some dirty working directory' '\ndiff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh\nindex 11884d2fc3..06c1301222 100755\n--- a/t/t7064-wtstatus-pv2.sh\n+++ b/t/t7064-wtstatus-pv2.sh\n@@ -4,6 +4,7 @@ test_description='git status --porcelain=v2\n \n This test exercises porcelain V2 output for git status.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n \n-- \n2.46.0.dirty\n\n"},{"id":"499810","messageId":"034c416d4693701cecc3201b9f8d9839fd99c1ab.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 13/24] builtin/rev-parse: fix memory leak with `--parseopt`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:21Z","receivedAt":"2024-08-01T10:40:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `--parseopt` mode allows shell scripts to have the same option\nparsing mode as we have in C builtins. It soaks up a set of option\ndescriptions via stdin and massages them into proper `struct option`s\nthat we can then use to parse a set of arguments.\n\nWe only partially free those options when done though, creating a memory\nleak. Interestingly, we only end up free'ing the first option's help,\nwhich is of course wrong.\n\nFix this by freeing all option's help fields as well as their `argh`\nfields to plug this memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/rev-parse.c | 5 ++++-\n t/t1502-rev-parse-parseopt.sh | 2 ++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 2e64f5bda7..5845d3f59b 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -553,7 +553,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&sb);\n \tstrvec_clear(&longnames);\n \tstrvec_clear(&usage);\n-\tfree((char *) opts->help);\n+\tfor (size_t i = 0; i < opts_nr; i++) {\n+\t\tfree((char *) opts[i].help);\n+\t\tfree((char *) opts[i].argh);\n+\t}\n \tfree(opts);\n \treturn 0;\n }\ndiff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh\nindex b754b9fd74..5eaa6428c4 100755\n--- a/t/t1502-rev-parse-parseopt.sh\n+++ b/t/t1502-rev-parse-parseopt.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test git rev-parse --parseopt'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n check_invalid_long_option () {\n-- \n2.46.0.dirty\n\n"},{"id":"499812","messageId":"20a40e2fd49468f3c406a3124c06cd2b4a9f58eb.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 14/24] builtin/show-branch: fix several memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:26Z","receivedAt":"2024-08-01T10:40:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several memory leaks in git-show-branch(1). Fix them.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/show-branch.c | 52 +++++++++++++++++++++++++++++-------------\n t/t3202-show-branch.sh | 1 +\n t/t6010-merge-base.sh | 1 +\n 3 files changed, 38 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex d72f4cb98d..7d797a880c 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -502,14 +502,14 @@ static int rev_is_head(const char *head, const char *name)\n \treturn !strcmp(head, name);\n }\n \n-static int show_merge_base(struct commit_list *seen, int num_rev)\n+static int show_merge_base(const struct commit_list *seen, int num_rev)\n {\n \tint all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);\n \tint all_revs = all_mask & ~((1u << REV_SHIFT) - 1);\n \tint exit_status = 1;\n \n-\twhile (seen) {\n-\t\tstruct commit *commit = pop_commit(&seen);\n+\tfor (const struct commit_list *s = seen; s; s = s->next) {\n+\t\tstruct commit *commit = s->item;\n \t\tint flags = commit->object.flags & all_mask;\n \t\tif (!(flags & UNINTERESTING) &&\n \t\t ((flags & all_revs) == all_revs)) {\n@@ -635,7 +635,7 @@ static int parse_reflog_param(const struct option *opt, const char *arg,\n int cmd_show_branch(int ac, const char **av, const char *prefix)\n {\n \tstruct commit *rev[MAX_REVS], *commit;\n-\tchar *reflog_msg[MAX_REVS];\n+\tchar *reflog_msg[MAX_REVS] = {0};\n \tstruct commit_list *list = NULL, *seen = NULL;\n \tunsigned int rev_mask[MAX_REVS];\n \tint num_rev, i, extra = 0;\n@@ -692,6 +692,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t parse_reflog_param),\n \t\tOPT_END()\n \t};\n+\tconst char **args_copy = NULL;\n+\tint ret;\n \n \tinit_commit_name_slab(&name_slab);\n \n@@ -699,8 +701,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \t/* If nothing is specified, try the default first */\n \tif (ac == 1 && default_args.nr) {\n+\t\tDUP_ARRAY(args_copy, default_args.v, default_args.nr);\n \t\tac = default_args.nr;\n-\t\tav = default_args.v;\n+\t\tav = args_copy;\n \t}\n \n \tac = parse_options(ac, av, prefix, builtin_show_branch_options,\n@@ -780,7 +783,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t}\n \n \t\tfor (i = 0; i < reflog; i++) {\n-\t\t\tchar *logmsg;\n+\t\t\tchar *logmsg = NULL;\n \t\t\tchar *nth_desc;\n \t\t\tconst char *msg;\n \t\t\tchar *end;\n@@ -790,6 +793,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\tif (read_ref_at(get_main_ref_store(the_repository),\n \t\t\t\t\tref, flags, 0, base + i, &oid, &logmsg,\n \t\t\t\t\t×tamp, &tz, NULL)) {\n+\t\t\t\tfree(logmsg);\n \t\t\t\treflog = i;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -842,7 +846,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \tif (!ref_name_cnt) {\n \t\tfprintf(stderr, \"No revs to be shown.\\n\");\n-\t\texit(0);\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \tfor (num_rev = 0; ref_name[num_rev]; num_rev++) {\n@@ -879,11 +884,15 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \tcommit_list_sort_by_date(&seen);\n \n-\tif (merge_base)\n-\t\treturn show_merge_base(seen, num_rev);\n+\tif (merge_base) {\n+\t\tret = show_merge_base(seen, num_rev);\n+\t\tgoto out;\n+\t}\n \n-\tif (independent)\n-\t\treturn show_independent(rev, num_rev, rev_mask);\n+\tif (independent) {\n+\t\tret = show_independent(rev, num_rev, rev_mask);\n+\t\tgoto out;\n+\t}\n \n \t/* Show list; --more=-1 means list-only */\n \tif (1 < num_rev || extra < 0) {\n@@ -919,8 +928,10 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\tputchar('\\n');\n \t\t}\n \t}\n-\tif (extra < 0)\n-\t\texit(0);\n+\tif (extra < 0) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n \n \t/* Sort topologically */\n \tsort_in_topological_order(&seen, sort_order);\n@@ -932,8 +943,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \tall_mask = ((1u << (REV_SHIFT + num_rev)) - 1);\n \tall_revs = all_mask & ~((1u << REV_SHIFT) - 1);\n \n-\twhile (seen) {\n-\t\tstruct commit *commit = pop_commit(&seen);\n+\tfor (struct commit_list *l = seen; l; l = l->next) {\n+\t\tstruct commit *commit = l->item;\n \t\tint this_flag = commit->object.flags;\n \t\tint is_merge_point = ((this_flag & all_revs) == all_revs);\n \n@@ -973,6 +984,15 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\tif (shown_merge_point && --extra < 0)\n \t\t\tbreak;\n \t}\n+\n+\tret = 0;\n+\n+out:\n+\tfor (size_t i = 0; i < ARRAY_SIZE(reflog_msg); i++)\n+\t\tfree(reflog_msg[i]);\n+\tfree_commit_list(seen);\n+\tfree_commit_list(list);\n+\tfree(args_copy);\n \tfree(head);\n-\treturn 0;\n+\treturn ret;\n }\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex a1139f79e2..3b6dad0c46 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -2,6 +2,7 @@\n \n test_description='test show-branch'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'error descriptions on empty repository' '\ndiff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\nindex 44c726ea39..f96ea82e78 100755\n--- a/t/t6010-merge-base.sh\n+++ b/t/t6010-merge-base.sh\n@@ -6,6 +6,7 @@\n test_description='Merge base and parent list computation.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n M=1130000000\n-- \n2.46.0.dirty\n\n"},{"id":"499813","messageId":"b553f1ffb9faa5ae93d93423db7d689c2def60a1.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 15/24] builtin/credential-store: fix leaking credential","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:31Z","receivedAt":"2024-08-01T10:40:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never free credentials read by the credential store, leading to a\nmemory leak. Plug it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/credential-store.c | 1 +\n t/t0302-credential-store.sh | 2 ++\n t/t0303-credential-external.sh | 1 +\n 3 files changed, 4 insertions(+)\n\ndiff --git a/builtin/credential-store.c b/builtin/credential-store.c\nindex 494c809332..97968bfa1c 100644\n--- a/builtin/credential-store.c\n+++ b/builtin/credential-store.c\n@@ -218,5 +218,6 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)\n \t\t; /* Ignore unknown operation. */\n \n \tstring_list_clear(&fns, 0);\n+\tcredential_clear(&c);\n \treturn 0;\n }\ndiff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh\nindex 716bf1af9f..f83db659e2 100755\n--- a/t/t0302-credential-store.sh\n+++ b/t/t0302-credential-store.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='credential-store tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-credential.sh\n \ndiff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh\nindex 72ae405c3e..8aadbe86c4 100755\n--- a/t/t0303-credential-external.sh\n+++ b/t/t0303-credential-external.sh\n@@ -29,6 +29,7 @@ you can set GIT_TEST_CREDENTIAL_HELPER_SETUP to a sequence of shell\n commands.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-credential.sh\n \n-- \n2.46.0.dirty\n\n"},{"id":"499814","messageId":"76d81c8ae1e99d3160a2535c2fd93fde0734e86b.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 16/24] builtin/rerere: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:36Z","receivedAt":"2024-08-01T10:40:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are multiple trivial memory leaks in git-rerere(1). Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/rerere.c | 8 +++++++-\n rerere.c | 9 +++++++--\n t/t2030-unresolve-info.sh | 1 +\n t/t4200-rerere.sh | 1 +\n 4 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex b2efc6f640..81b65ffa39 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -73,11 +73,17 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \n \tif (!strcmp(argv[0], \"forget\")) {\n \t\tstruct pathspec pathspec;\n+\t\tint ret;\n+\n \t\tif (argc < 2)\n \t\t\twarning(_(\"'git rerere forget' without paths is deprecated\"));\n \t\tparse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD,\n \t\t\t prefix, argv + 1);\n-\t\treturn rerere_forget(the_repository, &pathspec);\n+\n+\t\tret = rerere_forget(the_repository, &pathspec);\n+\n+\t\tclear_pathspec(&pathspec);\n+\t\treturn ret;\n \t}\n \n \tif (!strcmp(argv[0], \"clear\")) {\ndiff --git a/rerere.c b/rerere.c\nindex 3a3888cce2..525ed6cc1e 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1107,7 +1107,7 @@ static int rerere_forget_one_path(struct index_state *istate,\n \n int rerere_forget(struct repository *r, struct pathspec *pathspec)\n {\n-\tint i, fd;\n+\tint i, fd, ret;\n \tstruct string_list conflict = STRING_LIST_INIT_DUP;\n \tstruct string_list merge_rr = STRING_LIST_INIT_DUP;\n \n@@ -1132,7 +1132,12 @@ int rerere_forget(struct repository *r, struct pathspec *pathspec)\n \t\t\tcontinue;\n \t\trerere_forget_one_path(r->index, it->string, &merge_rr);\n \t}\n-\treturn write_rr(&merge_rr, fd);\n+\n+\tret = write_rr(&merge_rr, fd);\n+\n+\tstring_list_clear(&conflict, 0);\n+\tstring_list_clear(&merge_rr, 1);\n+\treturn ret;\n }\n \n /*\ndiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\nindex be3fcdde07..b3f6bc97b5 100755\n--- a/t/t2030-unresolve-info.sh\n+++ b/t/t2030-unresolve-info.sh\n@@ -5,6 +5,7 @@ test_description='undoing resolution'\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 check_resolve_undo () {\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex b0a3e84984..213b36fb96 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -25,6 +25,7 @@ test_description='git rerere\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_expect_success 'setup' '\n-- \n2.46.0.dirty\n\n"},{"id":"499815","messageId":"d52ac1a550a11b238346524e73dcf04a4f7e5a30.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 17/24] builtin/shortlog: fix various trivial memory leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:41Z","receivedAt":"2024-08-01T10:40:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There is a trivial memory leak in git-shortlog(1). Fix it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/shortlog.c | 1 +\n t/t4201-shortlog.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 5bde7c68c2..b529608c92 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -514,4 +514,5 @@ void shortlog_output(struct shortlog *log)\n \tstring_list_clear(&log->list, 1);\n \tclear_mailmap(&log->mailmap);\n \tstring_list_clear(&log->format, 0);\n+\tstring_list_clear(&log->trailers, 0);\n }\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex f698d0c9ad..c20c885724 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -9,6 +9,7 @@ test_description='git shortlog\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_expect_success 'setup' '\n-- \n2.46.0.dirty\n\n"},{"id":"499816","messageId":"dccaf695a999294ae7e1eb24944a0e565b833b6a.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 18/24] builtin/worktree: fix leaking derived branch names","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:46Z","receivedAt":"2024-08-01T10:40:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are several heuristics that git-worktree(1) uses to derive the\nname of the newly created branch when not given explicitly. These\nheuristics all allocate a new string, but we only end up freeing that\nstring in a subset of cases.\n\nFix the remaining cases where we didn't yet free the derived branch\nnames. While at it, also free `opt_track`, which is being populated via\nan `OPT_PASSTHRU()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/worktree.c | 7 ++++---\n t/t2400-worktree-add.sh | 1 +\n t/t9902-completion.sh | 1 +\n 3 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 1d51e54fcd..cec3ada6b0 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -769,7 +769,7 @@ static int add(int ac, const char **av, const char *prefix)\n \tchar *branch_to_free = NULL;\n \tchar *new_branch_to_free = NULL;\n \tconst char *new_branch = NULL;\n-\tconst char *opt_track = NULL;\n+\tchar *opt_track = NULL;\n \tconst char *lock_reason = NULL;\n \tint keep_locked = 0;\n \tint used_new_branch_options;\n@@ -846,7 +846,7 @@ static int add(int ac, const char **av, const char *prefix)\n \tif (opts.orphan && !new_branch) {\n \t\tint n;\n \t\tconst char *s = worktree_basename(path, &n);\n-\t\tnew_branch = xstrndup(s, n);\n+\t\tnew_branch = new_branch_to_free = xstrndup(s, n);\n \t} else if (opts.orphan) {\n \t\t; /* no-op */\n \t} else if (opts.detach) {\n@@ -875,7 +875,7 @@ static int add(int ac, const char **av, const char *prefix)\n \t\t\tremote = unique_tracking_name(branch, &oid, NULL);\n \t\t\tif (remote) {\n \t\t\t\tnew_branch = branch;\n-\t\t\t\tbranch = remote;\n+\t\t\t\tbranch = new_branch_to_free = remote;\n \t\t\t}\n \t\t}\n \n@@ -923,6 +923,7 @@ static int add(int ac, const char **av, const char *prefix)\n \n \tret = add_worktree(path, branch, &opts);\n \tfree(path);\n+\tfree(opt_track);\n \tfree(branch_to_free);\n \tfree(new_branch_to_free);\n \treturn ret;\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex ba320dc417..cfc4aeb179 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/lib-rebase.sh\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 932d5ad759..cc6aa9f0cd 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -16,6 +16,7 @@ test_untraceable=UnfortunatelyYes\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./lib-bash.sh\n \n complete ()\n-- \n2.46.0.dirty\n\n"},{"id":"499817","messageId":"788f0bee7bb332e32c305a41a809e82289829e5a.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 19/24] builtin/credential-cache: fix trivial leaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:51Z","receivedAt":"2024-08-01T10:40:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are two trivial leaks in git-credential-cache(1):\n\n - We leak the child process in `spawn_daemon()`. As we do not call\n `finish_command()` and instead let the created process daemonize, we\n have to clear the process manually.\n\n - We do not free the computed socket path in case it wasn't given via\n `--socket=`.\n\nPlug both of these memory leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/credential-cache.c | 9 +++++++--\n t/t0301-credential-cache.sh | 2 ++\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\nindex 3db8df70a9..aaf2f8438b 100644\n--- a/builtin/credential-cache.c\n+++ b/builtin/credential-cache.c\n@@ -88,6 +88,8 @@ static void spawn_daemon(const char *socket)\n \t\tdie_errno(\"unable to read result code from cache daemon\");\n \tif (r != 3 || memcmp(buf, \"ok\\n\", 3))\n \t\tdie(\"cache daemon did not start: %.*s\", r, buf);\n+\n+\tchild_process_clear(&daemon);\n \tclose(daemon.out);\n }\n \n@@ -137,7 +139,8 @@ static void announce_capabilities(void)\n \n int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n {\n-\tchar *socket_path = NULL;\n+\tconst char *socket_path_arg = NULL;\n+\tchar *socket_path;\n \tint timeout = 900;\n \tconst char *op;\n \tconst char * const usage[] = {\n@@ -147,7 +150,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"timeout\", &timeout,\n \t\t\t \"number of seconds to cache credentials\"),\n-\t\tOPT_STRING(0, \"socket\", &socket_path, \"path\",\n+\t\tOPT_STRING(0, \"socket\", &socket_path_arg, \"path\",\n \t\t\t \"path of cache-daemon socket\"),\n \t\tOPT_END()\n \t};\n@@ -160,6 +163,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n \tif (!have_unix_sockets())\n \t\tdie(_(\"credential-cache unavailable; no unix socket support\"));\n \n+\tsocket_path = xstrdup_or_null(socket_path_arg);\n \tif (!socket_path)\n \t\tsocket_path = get_socket_path();\n \tif (!socket_path)\n@@ -176,6 +180,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)\n \telse\n \t\t; /* ignore unknown operation */\n \n+\tfree(socket_path);\n \treturn 0;\n }\n \ndiff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh\nindex c10e35905e..5d5b64205f 100755\n--- a/t/t0301-credential-cache.sh\n+++ b/t/t0301-credential-cache.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='credential-cache tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-credential.sh\n \n-- \n2.46.0.dirty\n\n"},{"id":"499818","messageId":"eb58826b146c88654ef20ab5c28948ee027b4d25.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 20/24] t/test-repository: fix leaking repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:40:56Z","receivedAt":"2024-08-01T10:41:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The test-repository test helper zeroes out `the_repository` such that it\ncan be sure that our codebase only ends up using the supplied repository\nthat we initialize in the respective helper functions. This does cause\nmemory leaks though as the data that `the_repository` has been holding\nonto is not referenced anymore.\n\nFix this by calling `repo_clear()` instead.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-repository.c | 4 ++--\n t/t5318-commit-graph.sh | 2 ++\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\nindex c6a074df3d..63c37de33d 100644\n--- a/t/helper/test-repository.c\n+++ b/t/helper/test-repository.c\n@@ -19,7 +19,7 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,\n \n \tsetup_git_env(gitdir);\n \n-\tmemset(the_repository, 0, sizeof(*the_repository));\n+\trepo_clear(the_repository);\n \n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\n@@ -49,7 +49,7 @@ static void test_get_commit_tree_in_graph(const char *gitdir,\n \n \tsetup_git_env(gitdir);\n \n-\tmemset(the_repository, 0, sizeof(*the_repository));\n+\trepo_clear(the_repository);\n \n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex a2b4442660..2916c07e3c 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='commit graph'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-chunk.sh\n \n-- \n2.46.0.dirty\n\n"},{"id":"499819","messageId":"fa95a67ccd968aa87e24f34b77b30861ff346777.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 21/24] object-name: fix leaking commit list items","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:41:01Z","receivedAt":"2024-08-01T10:41:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `get_oid_oneline()`, we pass in a `struct commit_list` that\ngets modified by the function. This creates a weird situation where the\ncommit list may sometimes be empty after returning, but sometimes it\nwill continue to carry additional commits. In those cases the remainder\nof the list leaks.\n\nUltimately, the design where we only pass partial ownership to\n`get_oid_oneline()` feels shoddy. Refactor the code such that we only\npass a constant pointer to the list, creating a local copy as needed.\nCallers are thus always responsible for freeing the commit list, which\nthen allows us to plug a bunch of memory leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-name.c | 26 ++++++++++++++++----------\n t/t1511-rev-parse-caret.sh | 1 +\n t/t6133-pathspec-rev-dwim.sh | 2 ++\n 3 files changed, 19 insertions(+), 10 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 527b853ac4..240a93e7ce 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -27,7 +27,8 @@\n #include \"date.h\"\n #include \"object-file-convert.h\"\n \n-static int get_oid_oneline(struct repository *r, const char *, struct object_id *, struct commit_list *);\n+static int get_oid_oneline(struct repository *r, const char *, struct object_id *,\n+\t\t\t const struct commit_list *);\n \n typedef int (*disambiguate_hint_fn)(struct repository *, const struct object_id *, void *);\n \n@@ -1254,6 +1255,8 @@ static int peel_onion(struct repository *r, const char *name, int len,\n \t\tprefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));\n \t\tcommit_list_insert((struct commit *)o, &list);\n \t\tret = get_oid_oneline(r, prefix, oid, list);\n+\n+\t\tfree_commit_list(list);\n \t\tfree(prefix);\n \t\treturn ret;\n \t}\n@@ -1388,9 +1391,10 @@ static int handle_one_ref(const char *path, const struct object_id *oid,\n \n static int get_oid_oneline(struct repository *r,\n \t\t\t const char *prefix, struct object_id *oid,\n-\t\t\t struct commit_list *list)\n+\t\t\t const struct commit_list *list)\n {\n-\tstruct commit_list *backup = NULL, *l;\n+\tstruct commit_list *copy = NULL;\n+\tconst struct commit_list *l;\n \tint found = 0;\n \tint negative = 0;\n \tregex_t regex;\n@@ -1411,14 +1415,14 @@ static int get_oid_oneline(struct repository *r,\n \n \tfor (l = list; l; l = l->next) {\n \t\tl->item->object.flags |= ONELINE_SEEN;\n-\t\tcommit_list_insert(l->item, &backup);\n+\t\tcommit_list_insert(l->item, ©);\n \t}\n-\twhile (list) {\n+\twhile (copy) {\n \t\tconst char *p, *buf;\n \t\tstruct commit *commit;\n \t\tint matches;\n \n-\t\tcommit = pop_most_recent_commit(&list, ONELINE_SEEN);\n+\t\tcommit = pop_most_recent_commit(©, ONELINE_SEEN);\n \t\tif (!parse_object(r, &commit->object.oid))\n \t\t\tcontinue;\n \t\tbuf = repo_get_commit_buffer(r, commit, NULL);\n@@ -1433,10 +1437,9 @@ static int get_oid_oneline(struct repository *r,\n \t\t}\n \t}\n \tregfree(®ex);\n-\tfree_commit_list(list);\n-\tfor (l = backup; l; l = l->next)\n+\tfor (l = list; l; l = l->next)\n \t\tclear_commit_marks(l->item, ONELINE_SEEN);\n-\tfree_commit_list(backup);\n+\tfree_commit_list(copy);\n \treturn found ? 0 : -1;\n }\n \n@@ -2024,7 +2027,10 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n \t\t\trefs_for_each_ref(get_main_ref_store(repo), handle_one_ref, &cb);\n \t\t\trefs_head_ref(get_main_ref_store(repo), handle_one_ref, &cb);\n \t\t\tcommit_list_sort_by_date(&list);\n-\t\t\treturn get_oid_oneline(repo, name + 2, oid, list);\n+\t\t\tret = get_oid_oneline(repo, name + 2, oid, list);\n+\n+\t\t\tfree_commit_list(list);\n+\t\t\treturn ret;\n \t\t}\n \t\tif (namelen < 3 ||\n \t\t name[2] != ':' ||\ndiff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh\nindex 6ecfed86bc..e7e78a4028 100755\n--- a/t/t1511-rev-parse-caret.sh\n+++ b/t/t1511-rev-parse-caret.sh\n@@ -5,6 +5,7 @@ test_description='tests for ref^{stuff}'\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_expect_success 'setup' '\ndiff --git a/t/t6133-pathspec-rev-dwim.sh b/t/t6133-pathspec-rev-dwim.sh\nindex a290ffca0d..6dd4bbbf9f 100755\n--- a/t/t6133-pathspec-rev-dwim.sh\n+++ b/t/t6133-pathspec-rev-dwim.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test dwim of revs versus pathspecs in revision parser'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.dirty\n\n"},{"id":"499820","messageId":"8cae4c44f4ef5bd7686d90ecc051a8650da7d057.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 22/24] entry: fix leaking pathnames during delayed checkout","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:41:06Z","receivedAt":"2024-08-01T10:41:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When filtering files during delayed checkout, we pass a string list to\n`async_query_available_blobs()`. This list is initialized with NODUP,\nand thus inserted strings will not be owned by the list. In the latter\nfunction we then try to hand over ownership by passing an `xstrup()`'d\nvalue to `string_list_insert()`. But this is not how this works: a NODUP\nlist does not take ownership of allocated strings and will never free\nthem for the caller.\n\nFix this issue by initializing the list as `DUP` instead and dropping\nthe explicit call to `xstrdup()`. This is okay to do given that this is\nthe single callsite of `async_query_available_blobs()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n convert.c | 2 +-\n entry.c | 4 +++-\n t/t2080-parallel-checkout-basics.sh | 1 +\n t/t2082-parallel-checkout-attributes.sh | 1 +\n 4 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex d8737fe0f2..61a540e212 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -960,7 +960,7 @@ int async_query_available_blobs(const char *cmd, struct string_list *available_p\n \twhile ((line = packet_read_line(process->out, NULL))) {\n \t\tconst char *path;\n \t\tif (skip_prefix(line, \"pathname=\", &path))\n-\t\t\tstring_list_insert(available_paths, xstrdup(path));\n+\t\t\tstring_list_insert(available_paths, path);\n \t\telse\n \t\t\t; /* ignore unknown keys */\n \t}\ndiff --git a/entry.c b/entry.c\nindex e7ed440ce2..3143b9996b 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -191,7 +191,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)\n \t\tprogress = start_delayed_progress(_(\"Filtering content\"), dco->paths.nr);\n \twhile (dco->filters.nr > 0) {\n \t\tfor_each_string_list_item(filter, &dco->filters) {\n-\t\t\tstruct string_list available_paths = STRING_LIST_INIT_NODUP;\n+\t\t\tstruct string_list available_paths = STRING_LIST_INIT_DUP;\n \n \t\t\tif (!async_query_available_blobs(filter->string, &available_paths)) {\n \t\t\t\t/* Filter reported an error */\n@@ -245,6 +245,8 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)\n \t\t\t\t} else\n \t\t\t\t\terrs = 1;\n \t\t\t}\n+\n+\t\t\tstring_list_clear(&available_paths, 0);\n \t\t}\n \n \t\tfilter_string_list(&dco->filters, 0, string_is_not_null, NULL);\ndiff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh\nindex 5ffe1a41e2..59e5570cb2 100755\n--- a/t/t2080-parallel-checkout-basics.sh\n+++ b/t/t2080-parallel-checkout-basics.sh\n@@ -8,6 +8,7 @@ working tree.\n '\n \n TEST_NO_CREATE_REPO=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-parallel-checkout.sh\"\n \ndiff --git a/t/t2082-parallel-checkout-attributes.sh b/t/t2082-parallel-checkout-attributes.sh\nindex f3511cd43a..aec55496eb 100755\n--- a/t/t2082-parallel-checkout-attributes.sh\n+++ b/t/t2082-parallel-checkout-attributes.sh\n@@ -10,6 +10,7 @@ properly (without access to the index or attribute stack).\n '\n \n TEST_NO_CREATE_REPO=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-parallel-checkout.sh\"\n . \"$TEST_DIRECTORY/lib-encoding.sh\"\n-- \n2.46.0.dirty\n\n"},{"id":"499821","messageId":"5d2bf0f0f3d05f402df87588511b91c8246401c3.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 23/24] convert: fix leaking config strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:41:11Z","receivedAt":"2024-08-01T10:41:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `read_convert_config()`, we end up reading some string values into\nvariables. We don't free any potentially-existing old values though,\nwhich will result in a memory leak in case the same key has been defined\nmultiple times.\n\nFix those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n convert.c | 12 +++++++++---\n t/t0021-conversion.sh | 1 +\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 61a540e212..92ce04c406 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1050,14 +1050,20 @@ static int read_convert_config(const char *var, const char *value,\n \t * The command-line will not be interpolated in any way.\n \t */\n \n-\tif (!strcmp(\"smudge\", key))\n+\tif (!strcmp(\"smudge\", key)) {\n+\t\tFREE_AND_NULL(drv->smudge);\n \t\treturn git_config_string(&drv->smudge, var, value);\n+\t}\n \n-\tif (!strcmp(\"clean\", key))\n+\tif (!strcmp(\"clean\", key)) {\n+\t\tFREE_AND_NULL(drv->clean);\n \t\treturn git_config_string(&drv->clean, var, value);\n+\t}\n \n-\tif (!strcmp(\"process\", key))\n+\tif (!strcmp(\"process\", key)) {\n+\t\tFREE_AND_NULL(drv->process);\n \t\treturn git_config_string(&drv->process, var, value);\n+\t}\n \n \tif (!strcmp(\"required\", key)) {\n \t\tdrv->required = git_config_bool(var, value);\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 0b4997022b..eeb2714d9d 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -5,6 +5,7 @@ test_description='blob conversion via gitattributes'\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-terminal.sh\n \n-- \n2.46.0.dirty\n\n"},{"id":"499822","messageId":"6db1829c79401009c81cd34322274e4196114fcf.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 24/24] commit-reach: fix trivial memory leak when computing reachability","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:41:15Z","receivedAt":"2024-08-01T10:41:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We don't free the local `stack` commit list that we use to compute\nreachability of multiple commits at once. Do so.\n\nSigned-off-by: Patrick Steinhardt \n---\n commit-reach.c | 1 +\n t/t3201-branch-contains.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex dabc2972e4..02f8218b8e 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -1227,4 +1227,5 @@ void tips_reachable_from_bases(struct repository *r,\n done:\n \tfree(commits);\n \trepo_clear_commit_marks(r, SEEN);\n+\tfree_commit_list(stack);\n }\ndiff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh\nindex 800fc33165..6e587d27d7 100755\n--- a/t/t3201-branch-contains.sh\n+++ b/t/t3201-branch-contains.sh\n@@ -2,6 +2,7 @@\n \n test_description='branch --contains , --no-contains --merged, and --no-merged'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\n-- \n2.46.0.dirty\n\n"},{"id":"499824","messageId":"d3eb0372bd3aeb824d24114ed0e3f01b2fdc6adc.1722499961.git.ps@pks.im","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"[PATCH v2 07/24] builtin/submodule--helper: fix leaking clone depth parameter","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-01T10:42:59Z","receivedAt":"2024-08-01T10:43:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The submodule helper supports a `--depth` parameter for both its \"add\"\nand \"clone\" subcommands, which in both cases end up being forwarded to\ngit-clone(1). But while the former subcommand uses an `OPT_INTEGER()` to\nparse the depth, the latter uses `OPT_STRING()`. Consequently, it is\npossible to pass non-integer input to \"--depth\" when calling the \"clone\"\nsubcommand, where the value will then ultimately cause git-clone(1) to\nbail out.\n\nBesides the fact that the parameter verification should happen earlier,\nthe submodule helper infrastructure also internally tracks the depth via\na string. This requires us to convert the integer in the \"add\"\nsubcommand into an allocated string, and this string ultimately leaks.\n\nRefactor the code to consistently track the clone depth as an integer.\nThis plugs the memory leak, simplifies the code and allows us to use\n`OPT_INTEGER()` instead of `OPT_STRING()`, validating the input before\nwe shell out to git--clone(1).\n\nOriginal-patch-by: Rubén Justo \nSigned-off-by: Patrick Steinhardt \n---\n builtin/submodule--helper.c | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f1218a1995..863b01627c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1530,7 +1530,7 @@ struct module_clone_data {\n \tconst char *path;\n \tconst char *name;\n \tconst char *url;\n-\tconst char *depth;\n+\tint depth;\n \tstruct list_objects_filter_options *filter_options;\n \tunsigned int quiet: 1;\n \tunsigned int progress: 1;\n@@ -1729,8 +1729,8 @@ static int clone_submodule(const struct module_clone_data *clone_data,\n \t\t\tstrvec_push(&cp.args, \"--quiet\");\n \t\tif (clone_data->progress)\n \t\t\tstrvec_push(&cp.args, \"--progress\");\n-\t\tif (clone_data->depth && *(clone_data->depth))\n-\t\t\tstrvec_pushl(&cp.args, \"--depth\", clone_data->depth, NULL);\n+\t\tif (clone_data->depth > 0)\n+\t\t\tstrvec_pushf(&cp.args, \"--depth=%d\", clone_data->depth);\n \t\tif (reference->nr) {\n \t\t\tstruct string_list_item *item;\n \n@@ -1851,8 +1851,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"reference repository\")),\n \t\tOPT_BOOL(0, \"dissociate\", &dissociate,\n \t\t\t N_(\"use --reference only while cloning\")),\n-\t\tOPT_STRING(0, \"depth\", &clone_data.depth,\n-\t\t\t N_(\"string\"),\n+\t\tOPT_INTEGER(0, \"depth\", &clone_data.depth,\n \t\t\t N_(\"depth for shallow clones\")),\n \t\tOPT__QUIET(&quiet, \"suppress output for cloning a submodule\"),\n \t\tOPT_BOOL(0, \"progress\", &progress,\n@@ -3200,7 +3199,7 @@ static int add_submodule(const struct add_data *add_data)\n \t\t}\n \t\tclone_data.dissociate = add_data->dissociate;\n \t\tif (add_data->depth >= 0)\n-\t\t\tclone_data.depth = xstrfmt(\"%d\", add_data->depth);\n+\t\t\tclone_data.depth = add_data->depth;\n \n \t\tif (clone_submodule(&clone_data, &reference))\n \t\t\tgoto cleanup;\n-- \n2.46.0.dirty\n\n"},{"id":"499880","messageId":"ZqvC1CfRj0dQkmMF@nand.local","threadId":"61850","inReplyTo":"ZqtFIblPxqoXww-p@tanuki","subject":"Re: [PATCH 00/23] Memory leak fixes (pt.3)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-08-01T17:16:04Z","receivedAt":"2024-08-01T17:16:07Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Aug 01, 2024 at 10:19:45AM +0200, Patrick Steinhardt wrote:\n> On Wed, Jul 31, 2024 at 01:01:07PM -0400, Taylor Blau wrote:\n> > On Fri, Jul 26, 2024 at 02:13:43PM +0200, Patrick Steinhardt wrote:\n> > > 57 files changed, 251 insertions(+), 73 deletions(-)\n> >\n> > I took a careful read through these patches, and found most of them easy\n> > to review. I was admittedly a little lost with the \"fix various leak\"\n> > patches, and having slightly more context in the commit descriptions\n> > there would have been helpful.\n>\n> Yeah, I was a bit torn here on whether to expand the commit messages or\n> not. I just didn't think it's all that useful to always say \"Variable x\n> is allocated, but never freed\" for trivial cases where allocation and\n> freeing need to happen in the same function, much less so if we repeat\n> such commits for every single such variable in the same subsystem. So I\n> just threw those fixes into a single bag.\n\nI think sometimes that level of detail is useful, e.g., for tracking\nwhere the leak came from, if it's always existed, etc.\n\nBut I agree enumerating each leak in an otherwise straightforward commit\nis not a good use of author or reviewer time. I think more useful would\nbe enumerating the kinds of leaks that you clean up.\n\nThanks,\nTaylor\n"},{"id":"499881","messageId":"ZqvDJpNxMkvd3hSZ@nand.local","threadId":"61850","inReplyTo":"cover.1722499961.git.ps@pks.im","subject":"Re: [PATCH v2 00/24] Memory leak fixes (pt.3)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-08-01T17:17:26Z","receivedAt":"2024-08-01T17:17:29Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Aug 01, 2024 at 12:38:06PM +0200, Patrick Steinhardt wrote:\n> Range-diff against v1:\n\nAll looks reasonable to me. Thanks for incorporating mine and others'\nsuggestions, and again for working on this!\n\nThanks,\nTaylor\n"}]}