{"thread":{"id":"58743","subject":"[PATCH 02/17] {reset,merge}: call discard_index() before returning","startedAt":"2022-11-03T17:07:28Z","lastAt":"2022-11-08T20:54:19Z","messageCount":45,"participants":["Ævar Arnfjörð Bjarmason","Phillip Wood","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":17},"messages":[{"id":"466428","messageId":"patch-02.17-819fde89a24-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 02/17] {reset,merge}: call discard_index() before returning","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:01Z","receivedAt":"2022-11-03T17:07:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"These two built-ins both deal with the index, but weren't discarding\nit. In subsequent commits we'll add more free()-ing to discard_index()\nthat we've missed, but let's first call the existing function.\n\nWe can doubtless add discard_index() (or its alias discard_cache()) to\na lot more places, but let's just add it here for now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/merge.c | 1 +\n builtin/reset.c | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 5900b81729d..a6698adbfd3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1794,5 +1794,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t}\n \tstrbuf_release(&buf);\n \tfree(branch_to_free);\n+\tdiscard_index(&the_index);\n \treturn ret;\n }\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fdce6f8c856..69f18a248ee 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -481,5 +481,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (!pathspec.nr)\n \t\tremove_branch_state(the_repository, 0);\n \n+\tdiscard_index(&the_index);\n+\n \treturn update_ref_status;\n }\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466429","messageId":"patch-01.17-7fe724599a7-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 01/17] tests: mark tests as passing with SANITIZE=leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:00Z","receivedAt":"2022-11-03T17:07:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This marks tests that have been leak-free since various recent\ncommits, but which were not marked us such when the memory leak was\nfixed. These were mostly discovered with the \"check\" mode added in\nfaececa53f9 (test-lib: have the \"check\" mode for SANITIZE=leak\nconsider leak logs, 2022-07-28).\n\nCommits that fixed the last memory leak in these tests. Per narrowing\ndown when they started to pass under SANITIZE=leak with \"bisect\":\n\n- t1022-read-tree-partial-clone.sh:\n 7e2619d8ff0 (list_objects_filter_options: plug leak of filter_spec\n strings, 2022-09-08)\n\n- t4053-diff-no-index.sh: 07a6f94a6d0 (diff-no-index: release prefixed\n filenames, 2022-09-07)\n\n- t6415-merge-dir-to-symlink.sh: bac92b1f39f (Merge branch\n 'js/ort-clean-up-after-failed-merge', 2022-08-08).\n\n- t5554-noop-fetch-negotiator.sh:\n 66eede4a37c (prepare_repo_settings(): plug leak of config values,\n 2022-09-08)\n\nLet's mark all of these as passing with\n\"TEST_PASSES_SANITIZE_LEAK=true\", to have it regression tested,\nincluding as part of the \"linux-leaks\" CI job.\n\nAdditionally, let's remove the \"!SANITIZE_LEAK\" prerequisite from\ntests that now pass, these were marked as failing in:\n\n- 77e56d55ba6 (diff.c: fix a double-free regression in a18d66cefb,\n 2022-03-17)\n- c4d1d526312 (tests: change some 'test $(git) = \"x\"' to test_cmp,\n 2022-03-07)\n\nThese were not spotted with the new \"check\" mode, but manually, it\ndoesn't cover these sort of prerequisites. There's few enough that we\nshouldn't bother to automate it. They'll be going away sooner than\nlater.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t1022-read-tree-partial-clone.sh | 2 +-\n t/t4015-diff-whitespace.sh | 4 ++--\n t/t4053-diff-no-index.sh | 1 +\n t/t5554-noop-fetch-negotiator.sh | 2 ++\n t/t6415-merge-dir-to-symlink.sh | 1 +\n t/t7103-reset-bare.sh | 2 +-\n 6 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1022-read-tree-partial-clone.sh b/t/t1022-read-tree-partial-clone.sh\nindex a9953b6a71c..d08563de5c9 100755\n--- a/t/t1022-read-tree-partial-clone.sh\n+++ b/t/t1022-read-tree-partial-clone.sh\n@@ -3,7 +3,7 @@\n test_description='git read-tree in partial clones'\n \n TEST_NO_CREATE_REPO=1\n-\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'read-tree in partial clone prefetches in one batch' '\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex f3e20dd5bba..b298f220e01 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -1638,7 +1638,7 @@ test_expect_success 'no effect on diff from --color-moved with --word-diff' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success !SANITIZE_LEAK 'no effect on show from --color-moved with --word-diff' '\n+test_expect_success 'no effect on show from --color-moved with --word-diff' '\n \tgit show --color-moved --word-diff >actual &&\n \tgit show --word-diff >expect &&\n \ttest_cmp expect actual\n@@ -2024,7 +2024,7 @@ test_expect_success '--color-moved rewinds for MIN_ALNUM_COUNT' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success !SANITIZE_LEAK 'move detection with submodules' '\n+test_expect_success 'move detection with submodules' '\n \ttest_create_repo bananas &&\n \techo ripe >bananas/recipe &&\n \tgit -C bananas add recipe &&\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 3feadf0e359..4e9fa0403d3 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -2,6 +2,7 @@\n \n test_description='diff --no-index'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t5554-noop-fetch-negotiator.sh b/t/t5554-noop-fetch-negotiator.sh\nindex 2ac7b5859e7..06991e8e8aa 100755\n--- a/t/t5554-noop-fetch-negotiator.sh\n+++ b/t/t5554-noop-fetch-negotiator.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test noop fetch negotiator'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'noop negotiator does not emit any \"have\"' '\ndiff --git a/t/t6415-merge-dir-to-symlink.sh b/t/t6415-merge-dir-to-symlink.sh\nindex 2655e295f5a..ae00492c768 100755\n--- a/t/t6415-merge-dir-to-symlink.sh\n+++ b/t/t6415-merge-dir-to-symlink.sh\n@@ -4,6 +4,7 @@ test_description='merging when a directory was replaced with a symlink'\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 'create a commit where dir a/b changed to symlink' '\ndiff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh\nindex a60153f9f32..18bbd9975eb 100755\n--- a/t/t7103-reset-bare.sh\n+++ b/t/t7103-reset-bare.sh\n@@ -63,7 +63,7 @@ test_expect_success '\"mixed\" reset is not allowed in bare' '\n \ttest_must_fail git reset --mixed HEAD^\n '\n \n-test_expect_success !SANITIZE_LEAK '\"soft\" reset is allowed in bare' '\n+test_expect_success '\"soft\" reset is allowed in bare' '\n \tgit reset --soft HEAD^ &&\n \tgit show --pretty=format:%s >out &&\n \techo one >expect &&\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466430","messageId":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":null,"subject":"[PATCH 00/17] leak fixes: use existing constructors & other trivia","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:05:59Z","receivedAt":"2022-11-03T17:07:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"With the very minor exceptions of:\n\n* 03-04/17 (which need trivial oilerplate)\n* 05/17 (need to add trivial control flow to a free_*() function)\n* 12/17 (narrowing scope of allocation)\n* 17/17: Add \"goto ret\" pattern, combine two \"ret\" variables\n\nThese are all \"one-line\" leak fixes where we merely need to make use\nof an existing release function. The \"one-line\" only having the slight\ndisclaimer of needing to e.g. add braces to an \"if\" in one case, etc.\n\nEach commit in this series is tested with:\n\n\tGIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true \\\n\tmake SANITIZE=leak test\n\nI.e. mark tests as leak-free as we fix the leaks.\n\nIn 17/17 I replace uses of UNLEAK() where we can just as easily free()\ninstead, i.e. most of it's built-ins doing UNLEAK(x) instead of\nstrbuf_release(&x) etc.\n\nAs 17/17 notes I still think these's a place for unleak (some of the\nremaining ones are quite tricky), but that we gain more from leaving\nit for those tricky cases. Before this series we have 28 uses of\nUNLEAK(), after it's 15.\n\nÆvar Arnfjörð Bjarmason (17):\n tests: mark tests as passing with SANITIZE=leak\n {reset,merge}: call discard_index() before returning\n commit: discard partial cache before (re-)reading it\n read-cache.c: clear and free \"sparse_checkout_patterns\"\n dir.c: free \"ident\" and \"exclude_per_dir\" in \"struct untracked_cache\"\n built-ins & libs & helpers: add/move destructors, fix leaks\n unpack-file: fix ancient leak in create_temp_file()\n revision API: call graph_clear() in release_revisions()\n ls-files: fix a --with-tree memory leak\n sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()\n connected.c: free the \"struct packed_git\"\n sequencer.c: fix a pick_commits() leak\n rebase: don't leak on \"--abort\"\n sequencer.c: fix sequencer_continue() leak\n cherry-pick: free \"struct replay_opts\" members\n revert: fix parse_options_concat() leak\n built-ins: use free() not UNLEAK() if trivial, rm dead code\n\n builtin/add.c | 2 +-\n builtin/bugreport.c | 9 +++--\n builtin/checkout.c | 2 ++\n builtin/commit.c | 13 +++++---\n builtin/config.c | 42 +++++++++++-------------\n builtin/diff.c | 2 +-\n builtin/ls-files.c | 1 +\n builtin/merge.c | 1 +\n builtin/rebase.c | 4 +++\n builtin/repack.c | 2 +-\n builtin/reset.c | 2 ++\n builtin/rev-parse.c | 1 +\n builtin/revert.c | 4 +++\n builtin/stash.c | 2 ++\n builtin/unpack-file.c | 1 +\n builtin/worktree.c | 7 ++--\n connected.c | 6 +++-\n dir.c | 10 ++++--\n dir.h | 1 +\n read-cache.c | 5 +++\n ref-filter.c | 1 +\n revision.c | 1 +\n sequencer.c | 12 +++++--\n t/helper/test-fake-ssh.c | 1 +\n t/t0068-for-each-repo.sh | 1 +\n t/t0070-fundamental.sh | 1 +\n t/t1011-read-tree-sparse-checkout.sh | 1 +\n t/t1022-read-tree-partial-clone.sh | 2 +-\n t/t1404-update-ref-errors.sh | 2 ++\n t/t1409-avoid-packing-refs.sh | 1 +\n t/t1413-reflog-detach.sh | 1 +\n t/t1501-work-tree.sh | 2 ++\n t/t2012-checkout-last.sh | 1 +\n t/t2018-checkout-branch.sh | 1 +\n t/t2025-checkout-no-overlay.sh | 1 +\n t/t3009-ls-files-others-nonsubmodule.sh | 1 +\n t/t3010-ls-files-killed-modified.sh | 2 ++\n t/t3050-subprojects-fetch.sh | 1 +\n t/t3060-ls-files-with-tree.sh | 2 ++\n t/t3409-rebase-environ.sh | 1 +\n t/t3413-rebase-hook.sh | 1 +\n t/t3428-rebase-signoff.sh | 1 +\n t/t3429-rebase-edit-todo.sh | 1 +\n t/t3433-rebase-across-mode-change.sh | 1 +\n t/t4015-diff-whitespace.sh | 4 +--\n t/t4045-diff-relative.sh | 2 ++\n t/t4052-stat-output.sh | 1 +\n t/t4053-diff-no-index.sh | 1 +\n t/t4067-diff-partial-clone.sh | 1 +\n t/t4111-apply-subdir.sh | 1 +\n t/t4135-apply-weird-filenames.sh | 1 +\n t/t4213-log-tabexpand.sh | 1 +\n t/t5544-pack-objects-hook.sh | 2 ++\n t/t5554-noop-fetch-negotiator.sh | 2 ++\n t/t5610-clone-detached.sh | 1 +\n t/t5611-clone-config.sh | 1 +\n t/t5614-clone-submodules-shallow.sh | 1 +\n t/t5617-clone-submodules-remote.sh | 1 +\n t/t5618-alternate-refs.sh | 2 ++\n t/t6060-merge-index.sh | 2 ++\n t/t6301-for-each-ref-errors.sh | 1 +\n t/t6401-merge-criss-cross.sh | 2 ++\n t/t6406-merge-attr.sh | 1 +\n t/t6407-merge-binary.sh | 1 +\n t/t6415-merge-dir-to-symlink.sh | 1 +\n t/t6435-merge-sparse.sh | 1 +\n t/t7103-reset-bare.sh | 2 +-\n t/t7504-commit-msg-hook.sh | 1 +\n t/t7517-per-repo-email.sh | 1 +\n t/t7520-ignored-hook-warning.sh | 1 +\n t/t7605-merge-resolve.sh | 1 +\n t/t7614-merge-signoff.sh | 1 +\n t/t9003-help-autocorrect.sh | 2 ++\n t/t9115-git-svn-dcommit-funky-renames.sh | 1 -\n t/t9146-git-svn-empty-dirs.sh | 1 -\n t/t9148-git-svn-propset.sh | 1 -\n t/t9160-git-svn-preserve-empty-dirs.sh | 1 -\n 77 files changed, 150 insertions(+), 51 deletions(-)\n\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466431","messageId":"patch-04.17-68a390619b9-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 04/17] read-cache.c: clear and free \"sparse_checkout_patterns\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:03Z","receivedAt":"2022-11-03T17:07:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"sparse_checkout_patterns\" member was added to the \"struct\nindex_state\" in 836e25c51b2 (sparse-checkout: hold pattern list in\nindex, 2021-03-30), but wasn't added to discard_index(). Let's do\nthat.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n read-cache.c | 5 +++++\n t/t1011-read-tree-sparse-checkout.sh | 1 +\n t/t2018-checkout-branch.sh | 1 +\n t/t6435-merge-sparse.sh | 1 +\n 4 files changed, 8 insertions(+)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 32024029274..7c6477487a7 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2558,6 +2558,11 @@ int discard_index(struct index_state *istate)\n \tfree_untracked_cache(istate->untracked);\n \tistate->untracked = NULL;\n \n+\tif (istate->sparse_checkout_patterns) {\n+\t\tclear_pattern_list(istate->sparse_checkout_patterns);\n+\t\tFREE_AND_NULL(istate->sparse_checkout_patterns);\n+\t}\n+\n \tif (istate->ce_mem_pool) {\n \t\tmem_pool_discard(istate->ce_mem_pool, should_validate_cache_entries());\n \t\tFREE_AND_NULL(istate->ce_mem_pool);\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 742f0fa909f..595b24c0adb 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -12,6 +12,7 @@ test_description='sparse checkout tests\n '\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-read-tree.sh\n \ndiff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh\nindex 771c3c3c50e..8581ad34379 100755\n--- a/t/t2018-checkout-branch.sh\n+++ b/t/t2018-checkout-branch.sh\n@@ -3,6 +3,7 @@\n test_description='checkout'\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Arguments: [!] []\ndiff --git a/t/t6435-merge-sparse.sh b/t/t6435-merge-sparse.sh\nindex fde4aa3cd1a..78628fb248a 100755\n--- a/t/t6435-merge-sparse.sh\n+++ b/t/t6435-merge-sparse.sh\n@@ -3,6 +3,7 @@\n test_description='merge with sparse files'\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # test_file $filename $content\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466432","messageId":"patch-05.17-29123e62391-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 05/17] dir.c: free \"ident\" and \"exclude_per_dir\" in \"struct untracked_cache\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:04Z","receivedAt":"2022-11-03T17:07:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When the \"ident\" member of the structure was added in\n1e8fef609e7 (untracked cache: guard and disable on system changes,\n2015-03-08) this function wasn't updated to free it. Let's do so.\n\nLet's also free the \"exclude_per_dir\" memory we've been leaking\nsince[1], while making sure not to free() the constant \".gitignore\"\nstring we add by default[2].\n\nAs we now have three struct members we're freeing let's change\nfree_untracked_cache() to return early if \"uc\" isn't defined. We won't\nhand it to free() now, but that was just for convenience, once we're\ndealing with >=2 struct members this pattern is more convenient.\n\n1. f9e6c649589 (untracked cache: load from UNTR index extension,\n 2015-03-08)\n2. 039bc64e886 (core.excludesfile clean-up, 2007-11-14)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n dir.c | 10 +++++++---\n dir.h | 1 +\n 2 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex d604d1bab98..fbdb24fc819 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -3581,8 +3581,12 @@ static void free_untracked(struct untracked_cache_dir *ucd)\n \n void free_untracked_cache(struct untracked_cache *uc)\n {\n-\tif (uc)\n-\t\tfree_untracked(uc->root);\n+\tif (!uc)\n+\t\treturn;\n+\n+\tfree(uc->exclude_per_dir_to_free);\n+\tstrbuf_release(&uc->ident);\n+\tfree_untracked(uc->root);\n \tfree(uc);\n }\n \n@@ -3739,7 +3743,7 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n \t\t next + offset + hashsz);\n \tuc->dir_flags = get_be32(next + ouc_offset(dir_flags));\n \texclude_per_dir = (const char *)next + exclude_per_dir_offset;\n-\tuc->exclude_per_dir = xstrdup(exclude_per_dir);\n+\tuc->exclude_per_dir = uc->exclude_per_dir_to_free = xstrdup(exclude_per_dir);\n \t/* NUL after exclude_per_dir is covered by sizeof(*ouc) */\n \tnext += exclude_per_dir_offset + strlen(exclude_per_dir) + 1;\n \tif (next >= end)\ndiff --git a/dir.h b/dir.h\nindex 674747d93af..8acfc044181 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -188,6 +188,7 @@ struct untracked_cache {\n \tstruct oid_stat ss_info_exclude;\n \tstruct oid_stat ss_excludes_file;\n \tconst char *exclude_per_dir;\n+\tchar *exclude_per_dir_to_free;\n \tstruct strbuf ident;\n \t/*\n \t * dir_struct#flags must match dir_flags or the untracked\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466433","messageId":"patch-06.17-009c41d2e91-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 06/17] built-ins & libs & helpers: add/move destructors, fix leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:05Z","receivedAt":"2022-11-03T17:07:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix various leaks in built-ins, libraries and a test helper here we\nwere missing a call to strbuf_release(), string_list_clear() etc, or\nwere calling them after a potential \"return\".\n\nComments on individual changes:\n\n- builtin/checkout.c: Fix a memory leak that was introduced in [1]. A\n sibling leak introduced in [2] was recently fixed in [3]. As with [3]\n we should be using the wt_status_state_free_buffers() API introduced\n in [4].\n\n- builtin/repack.c: Fix a leak that's been here since this use of\n \"strbuf_release()\" was added in a1bbc6c0176 (repack: rewrite the shell\n script in C, 2013-09-15). We don't use the variable for anything\n except this loop, so we can instead free it right afterwards.\n\n- builtin/rev-parse: Fix a leak that's been here since this code was\n added in 21d47835386 (Add a parseopt mode to git-rev-parse to bring\n parse-options to shell scripts., 2007-11-04).\n\n- builtin/stash.c: Fix a couple of leaks that have been here since\n this code was added in d4788af875c (stash: convert create to builtin,\n 2019-02-25), we strbuf_release()'d only some of the \"struct strbuf\" we\n allocated earlier in the function, let's release all of them.\n\n- ref-filter.c: Fix a leak in 482c1191869 (gpg-interface: improve\n interface for parsing tags, 2021-02-11), we don't use the \"payload\"\n variable that we ask parse_signature() to populate for us, so let's\n free it.\n\n- t/helper/test-fake-ssh.c: Fix a leak that's been here since this\n code was added in 3064d5a38c7 (mingw: fix t5601-clone.sh,\n 2016-01-27). Let's free the \"struct strbuf\" as soon as we don't need\n it anymore.\n\n1. c45f0f525de (switch: reject if some operation is in progress,\n 2019-03-29)\n2. 2708ce62d21 (branch: sort detached HEAD based on a flag,\n 2021-01-07)\n3. abcac2e19fa (ref-filter.c: fix a leak in get_head_description,\n 2022-09-25)\n4. 962dd7ebc3e (wt-status: introduce wt_status_state_free_buffers(),\n 2020-09-27).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/checkout.c | 2 ++\n builtin/rebase.c | 3 +++\n builtin/repack.c | 2 +-\n builtin/rev-parse.c | 1 +\n builtin/stash.c | 2 ++\n ref-filter.c | 1 +\n t/helper/test-fake-ssh.c | 1 +\n t/t3428-rebase-signoff.sh | 1 +\n 8 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2a132392fbe..659dd5c4309 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1470,6 +1470,8 @@ static void die_if_some_operation_in_progress(void)\n \t\t \"or \\\"git worktree add\\\".\"));\n \tif (state.bisect_in_progress)\n \t\twarning(_(\"you are switching branch while bisecting\"));\n+\n+\twt_status_state_free_buffers(&state);\n }\n \n static int checkout_branch(struct checkout_opts *opts,\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 5d855fd8f51..64aed11c85d 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1825,10 +1825,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&buf);\n \tstrbuf_release(&revisions);\n \tfree(options.head_name);\n+\tstrvec_clear(&options.git_am_opts);\n \tfree(options.gpg_sign_opt);\n \tfree(options.cmd);\n \tfree(options.strategy);\n \tstrbuf_release(&options.git_format_patch_opt);\n \tfree(squash_onto_name);\n+\tstring_list_clear(&exec, 0);\n+\tstring_list_clear(&strategy_options, 0);\n \treturn !!ret;\n }\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 10e23f9ee1f..fb3ac197426 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -956,6 +956,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\titem = string_list_append(&names, line.buf);\n \t\titem->util = populate_pack_exts(item->string);\n \t}\n+\tstrbuf_release(&line);\n \tfclose(out);\n \tret = finish_command(&cmd);\n \tif (ret)\n@@ -1124,7 +1125,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tstring_list_clear(&existing_nonkept_packs, 0);\n \tstring_list_clear(&existing_kept_packs, 0);\n \tclear_pack_geometry(geometry);\n-\tstrbuf_release(&line);\n \n \treturn 0;\n }\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 8f61050bde8..e0244d63de6 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -530,6 +530,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)\n \tstrbuf_addstr(&parsed, \" --\");\n \tsq_quote_argv(&parsed, argv);\n \tputs(parsed.buf);\n+\tstrbuf_release(&parsed);\n \treturn 0;\n }\n \ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex bb5485b4095..8a64d564a19 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1686,8 +1686,10 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \t}\n \n done:\n+\tstrbuf_release(&patch);\n \tfree_stash_info(&info);\n \tstrbuf_release(&stash_msg_buf);\n+\tstrbuf_release(&untracked_files);\n \treturn ret;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 914908fac52..b40b76c3806 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1358,6 +1358,7 @@ static void find_subpos(const char *buf,\n \n \t/* parse signature first; we might not even have a subject line */\n \tparse_signature(buf, end - buf, &payload, &signature);\n+\tstrbuf_release(&payload);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\ndiff --git a/t/helper/test-fake-ssh.c b/t/helper/test-fake-ssh.c\nindex 12beee99ad2..d5e511633ab 100644\n--- a/t/helper/test-fake-ssh.c\n+++ b/t/helper/test-fake-ssh.c\n@@ -17,6 +17,7 @@ int cmd_main(int argc, const char **argv)\n \tf = fopen(buf.buf, \"w\");\n \tif (!f)\n \t\tdie(\"Could not write to %s\", buf.buf);\n+\tstrbuf_release(&buf);\n \tfor (i = 0; i < argc; i++)\n \t\tfprintf(f, \"%s%s\", i > 0 ? \" \" : \"\", i > 0 ? argv[i] : \"ssh:\");\n \tfprintf(f, \"\\n\");\ndiff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\nindex f6993b7e14d..e1b1e947647 100755\n--- a/t/t3428-rebase-signoff.sh\n+++ b/t/t3428-rebase-signoff.sh\n@@ -5,6 +5,7 @@ test_description='git rebase --signoff\n This test runs git rebase --signoff and make sure that it works.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # A simple file to commit\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466434","messageId":"patch-07.17-38e7c863dd8-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 07/17] unpack-file: fix ancient leak in create_temp_file()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:06Z","receivedAt":"2022-11-03T17:07:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a leak that's been with us since 3407bb4940c (Add \"unpack-file\"\nhelper that unpacks a sha1 blob into a tmpfile., 2005-04-18). See\n00c8fd493af (cat-file: use streaming API to print blobs, 2012-03-07)\nfor prior art which shows the same API pattern, i.e. free()-ing the\nresult of read_object_file() after it's used.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/unpack-file.c | 1 +\n t/t6060-merge-index.sh | 2 ++\n t/t6401-merge-criss-cross.sh | 2 ++\n t/t6406-merge-attr.sh | 1 +\n t/t6407-merge-binary.sh | 1 +\n t/t7605-merge-resolve.sh | 1 +\n 6 files changed, 8 insertions(+)\n\ndiff --git a/builtin/unpack-file.c b/builtin/unpack-file.c\nindex 9e8119dd354..88de32b7d7e 100644\n--- a/builtin/unpack-file.c\n+++ b/builtin/unpack-file.c\n@@ -19,6 +19,7 @@ static char *create_temp_file(struct object_id *oid)\n \tif (write_in_full(fd, buf, size) < 0)\n \t\tdie_errno(\"unable to write temp-file\");\n \tclose(fd);\n+\tfree(buf);\n \treturn path;\n }\n \ndiff --git a/t/t6060-merge-index.sh b/t/t6060-merge-index.sh\nindex ed449abe552..1a8b64cce18 100755\n--- a/t/t6060-merge-index.sh\n+++ b/t/t6060-merge-index.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='basic git merge-index / git-merge-one-file tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup diverging branches' '\ndiff --git a/t/t6401-merge-criss-cross.sh b/t/t6401-merge-criss-cross.sh\nindex 9d5e992878f..1962310408b 100755\n--- a/t/t6401-merge-criss-cross.sh\n+++ b/t/t6401-merge-criss-cross.sh\n@@ -8,6 +8,8 @@\n \n \n test_description='Test criss-cross merge'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'prepare repository' '\ndiff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh\nindex 8650a88c40a..5e4e4dd6d9e 100755\n--- a/t/t6406-merge-attr.sh\n+++ b/t/t6406-merge-attr.sh\n@@ -8,6 +8,7 @@ test_description='per path merge controlled by merge attribute'\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/t6407-merge-binary.sh b/t/t6407-merge-binary.sh\nindex e8a28717cec..0753fc95f45 100755\n--- a/t/t6407-merge-binary.sh\n+++ b/t/t6407-merge-binary.sh\n@@ -5,6 +5,7 @@ test_description='ask merge-recursive to merge binary files'\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/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh\nindex 5d56c385464..62d935d31c2 100755\n--- a/t/t7605-merge-resolve.sh\n+++ b/t/t7605-merge-resolve.sh\n@@ -4,6 +4,7 @@ test_description='git merge\n \n Testing the resolve strategy.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466435","messageId":"patch-09.17-d592439760e-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 09/17] ls-files: fix a --with-tree memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:08Z","receivedAt":"2022-11-03T17:07:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in overlay_tree_on_index(), we need to\nclear_pathspec() at some point, which might as well be after the last\ntime we use it in the function.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-files.c | 1 +\n t/t3060-ls-files-with-tree.sh | 2 ++\n t/t9148-git-svn-propset.sh | 1 -\n 3 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 4cf8a236483..a03b559ecaa 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -613,6 +613,7 @@ void overlay_tree_on_index(struct index_state *istate,\n \tif (!fn)\n \t\tfn = read_one_entry_quick;\n \terr = read_tree(the_repository, tree, &pathspec, fn, istate);\n+\tclear_pathspec(&pathspec);\n \tif (err)\n \t\tdie(\"unable to read tree entries %s\", tree_name);\n \ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex 52f76f7b57f..c4a72ae4462 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -8,6 +8,8 @@ test_description='git ls-files test (--with-tree).\n This test runs git ls-files --with-tree and in particular in\n a scenario known to trigger a crash with some versions of git.\n '\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh\nindex 6cc76a07b39..aebb28995e5 100755\n--- a/t/t9148-git-svn-propset.sh\n+++ b/t/t9148-git-svn-propset.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn propset tests'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'setup propset via import' '\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466436","messageId":"patch-10.17-9c70bfa334e-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 10/17] sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:09Z","receivedAt":"2022-11-03T17:07:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When \"read_strategy_opts()\" is called we may have populated the\n\"opts->strategy\" before, so we'll need to free() it to avoid leaking\nmemory. Along with the preceding commit this change various\nrebase-related tests pass.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e658df7e8ff..07d7062bfb8 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2894,6 +2894,7 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n \tstrbuf_reset(buf);\n \tif (!read_oneliner(buf, rebase_path_strategy(), 0))\n \t\treturn;\n+\tfree(opts->strategy);\n \topts->strategy = strbuf_detach(buf, NULL);\n \tif (!read_oneliner(buf, rebase_path_strategy_opts(), 0))\n \t\treturn;\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466437","messageId":"patch-03.17-c1003454939-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 03/17] commit: discard partial cache before (re-)reading it","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:02Z","receivedAt":"2022-11-03T17:07:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The read_cache() in prepare_to_commit() would end up clobbering the\npointer we had for a previously populated \"the_index.cache_tree\" in\nthe very common case of \"git commit\" stressed by e.g. the tests being\nchanged here.\n\nWe'd populate \"the_index.cache_tree\" by calling\n\"update_main_cache_tree\" in prepare_index(), but would not end up with\na \"fully prepared\" index. What constitutes an existing index is\nclearly overly fuzzy, here we'll check \"active_nr\" (aka\n\"the_index.cache_nr\"), but our \"the_index.cache_tree\" might have been\nmalloc()'d already.\n\nThus the code added in 11c8a74a64a (commit: write cache-tree data when\nwriting index anyway, 2011-12-06) would end up allocating the\n\"cache_tree\", and would interact here with code added in\n7168624c353 (Do not generate full commit log message if it is not\ngoing to be used, 2007-11-28). The result was a very common memory\nleak.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/commit.c | 7 +++++--\n t/t0068-for-each-repo.sh | 1 +\n t/t0070-fundamental.sh | 1 +\n t/t1404-update-ref-errors.sh | 2 ++\n t/t1409-avoid-packing-refs.sh | 1 +\n t/t1413-reflog-detach.sh | 1 +\n t/t1501-work-tree.sh | 2 ++\n t/t2025-checkout-no-overlay.sh | 1 +\n t/t3009-ls-files-others-nonsubmodule.sh | 1 +\n t/t3010-ls-files-killed-modified.sh | 2 ++\n t/t4045-diff-relative.sh | 2 ++\n t/t4111-apply-subdir.sh | 1 +\n t/t4135-apply-weird-filenames.sh | 1 +\n t/t4213-log-tabexpand.sh | 1 +\n t/t5618-alternate-refs.sh | 2 ++\n t/t6301-for-each-ref-errors.sh | 1 +\n t/t7520-ignored-hook-warning.sh | 1 +\n t/t7614-merge-signoff.sh | 1 +\n t/t9003-help-autocorrect.sh | 2 ++\n 19 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e22bdf23f5f..c291199b704 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -987,8 +987,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tstruct object_id oid;\n \t\tconst char *parent = \"HEAD\";\n \n-\t\tif (!active_nr && read_cache() < 0)\n-\t\t\tdie(_(\"Cannot read index\"));\n+\t\tif (!active_nr) {\n+\t\t\tdiscard_cache();\n+\t\t\tif (read_cache() < 0)\n+\t\t\t\tdie(_(\"Cannot read index\"));\n+\t\t}\n \n \t\tif (amend)\n \t\t\tparent = \"HEAD^1\";\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 4675e852517..9f63f612425 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -2,6 +2,7 @@\n \n test_description='git for-each-repo builtin'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'run based on configured value' '\ndiff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh\nindex 8d59905ef09..574de341980 100755\n--- a/t/t0070-fundamental.sh\n+++ b/t/t0070-fundamental.sh\n@@ -6,6 +6,7 @@ test_description='check that the most basic functions work\n Verify wrappers and compatibility functions.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'character classes (isspace, isalpha etc.)' '\ndiff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh\nindex 13c2b43bbae..b5606d93b52 100755\n--- a/t/t1404-update-ref-errors.sh\n+++ b/t/t1404-update-ref-errors.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='Test git update-ref error handling'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Create some references, perhaps run pack-refs --all, then try to\ndiff --git a/t/t1409-avoid-packing-refs.sh b/t/t1409-avoid-packing-refs.sh\nindex be12fb63506..f23c0152a85 100755\n--- a/t/t1409-avoid-packing-refs.sh\n+++ b/t/t1409-avoid-packing-refs.sh\n@@ -2,6 +2,7 @@\n \n test_description='avoid rewriting packed-refs unnecessarily'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Add an identifying mark to the packed-refs file header line. This\ndiff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh\nindex 934688a1ee8..d2a4822d46f 100755\n--- a/t/t1413-reflog-detach.sh\n+++ b/t/t1413-reflog-detach.sh\n@@ -4,6 +4,7 @@ test_description='Test reflog interaction with detached HEAD'\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 reset_state () {\ndiff --git a/t/t1501-work-tree.sh b/t/t1501-work-tree.sh\nindex b75558040ff..ae6528aecea 100755\n--- a/t/t1501-work-tree.sh\n+++ b/t/t1501-work-tree.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test separate work tree'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t2025-checkout-no-overlay.sh b/t/t2025-checkout-no-overlay.sh\nindex 8f13341cf8e..3832c3de813 100755\n--- a/t/t2025-checkout-no-overlay.sh\n+++ b/t/t2025-checkout-no-overlay.sh\n@@ -2,6 +2,7 @@\n \n test_description='checkout --no-overlay -- '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nindex 963f3462b75..14218b34243 100755\n--- a/t/t3009-ls-files-others-nonsubmodule.sh\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -18,6 +18,7 @@ This test runs git ls-files --others with the following working tree:\n git repository with a commit and an untracked file\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup: directories' '\ndiff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh\nindex 580e158f991..054178703d7 100755\n--- a/t/t3010-ls-files-killed-modified.sh\n+++ b/t/t3010-ls-files-killed-modified.sh\n@@ -41,6 +41,8 @@ Also for modification test, the cache and working tree have:\n We should report path0, path1, path2/file2, path3/file3, path7 and path8\n modified without reporting path9 and path10. submod1 is also modified.\n '\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'git update-index --add to add various paths.' '\ndiff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh\nindex fab351b48a1..198dfc91906 100755\n--- a/t/t4045-diff-relative.sh\n+++ b/t/t4045-diff-relative.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='diff --relative tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4111-apply-subdir.sh b/t/t4111-apply-subdir.sh\nindex 1618a6dbc7c..e9a87d761d1 100755\n--- a/t/t4111-apply-subdir.sh\n+++ b/t/t4111-apply-subdir.sh\n@@ -2,6 +2,7 @@\n \n test_description='patching from inconvenient places'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh\nindex 6bc3fb97a75..d3502c6fddf 100755\n--- a/t/t4135-apply-weird-filenames.sh\n+++ b/t/t4135-apply-weird-filenames.sh\n@@ -2,6 +2,7 @@\n \n test_description='git apply with weird postimage filenames'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh\nindex 53a4af32449..590fce95e90 100755\n--- a/t/t4213-log-tabexpand.sh\n+++ b/t/t4213-log-tabexpand.sh\n@@ -2,6 +2,7 @@\n \n test_description='log/show --expand-tabs'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n HT=\"\t\"\ndiff --git a/t/t5618-alternate-refs.sh b/t/t5618-alternate-refs.sh\nindex 3353216f09e..f905db0a3fd 100755\n--- a/t/t5618-alternate-refs.sh\n+++ b/t/t5618-alternate-refs.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test handling of --alternate-refs traversal'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Avoid test_commit because we want a specific and known set of refs:\ndiff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh\nindex 40edf9dab53..bfda1f46ad2 100755\n--- a/t/t6301-for-each-ref-errors.sh\n+++ b/t/t6301-for-each-ref-errors.sh\n@@ -2,6 +2,7 @@\n \n test_description='for-each-ref errors for broken refs'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n ZEROS=$ZERO_OID\ndiff --git a/t/t7520-ignored-hook-warning.sh b/t/t7520-ignored-hook-warning.sh\nindex dc57526e6f1..184b2589893 100755\n--- a/t/t7520-ignored-hook-warning.sh\n+++ b/t/t7520-ignored-hook-warning.sh\n@@ -2,6 +2,7 @@\n \n test_description='ignored hook warning'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\nindex fee258d4f0c..cf96a35e8e7 100755\n--- a/t/t7614-merge-signoff.sh\n+++ b/t/t7614-merge-signoff.sh\n@@ -8,6 +8,7 @@ This test runs git merge --signoff and makes sure that it works.\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 test files\ndiff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh\nindex f00deaf3815..4b9cb4c942f 100755\n--- a/t/t9003-help-autocorrect.sh\n+++ b/t/t9003-help-autocorrect.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='help.autocorrect finding a match'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466438","messageId":"patch-08.17-2944aaa3ddc-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 08/17] revision API: call graph_clear() in release_revisions()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:07Z","receivedAt":"2022-11-03T17:07:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Call graph_clear() in release_revisions(), this will free memory\nallocated by e.g. this command, which will now run without memory\nleaks:\n\n\tgit -P log -1 --graph --no-graph --graph\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n revision.c | 1 +\n t/t4052-stat-output.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex 0760e78936e..650dd17599a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3020,6 +3020,7 @@ void release_revisions(struct rev_info *revs)\n \tdate_mode_release(&revs->date_mode);\n \trelease_revisions_mailmap(revs->mailmap);\n \tfree_grep_patterns(&revs->grep_filter);\n+\tgraph_clear(revs->graph);\n \t/* TODO (need to handle \"no_free\"): diff_free(&revs->diffopt) */\n \tdiff_free(&revs->pruning);\n \treflog_walk_info_release(revs->reflog_info);\ndiff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh\nindex b5c281edaa7..3ee27e277dc 100755\n--- a/t/t4052-stat-output.sh\n+++ b/t/t4052-stat-output.sh\n@@ -8,6 +8,7 @@ test_description='test --stat output of various commands'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466439","messageId":"patch-12.17-7429151b5c4-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 12/17] sequencer.c: fix a pick_commits() leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:11Z","receivedAt":"2022-11-03T17:07:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a leak introduced in 1f6965f994f (sequencer: honor\nGIT_REFLOG_ACTION, 2020-04-07), we should free() the xstrdup()'d\n\"prev_reflog_action\" string.\n\nWe only needed to xstrdup() the previous action if we took this branch\nwithin the \"while\" statement, and needed to set the GIT_REFLOG_ACTION\nfor the duration of the do_pick_commit(). Let's just query and restore\nit there instead.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 9 ++++++---\n t/t2012-checkout-last.sh | 1 +\n t/t3409-rebase-environ.sh | 1 +\n t/t3413-rebase-hook.sh | 1 +\n t/t3433-rebase-across-mode-change.sh | 1 +\n t/t7504-commit-msg-hook.sh | 1 +\n t/t9115-git-svn-dcommit-funky-renames.sh | 1 -\n t/t9146-git-svn-empty-dirs.sh | 1 -\n t/t9160-git-svn-preserve-empty-dirs.sh | 1 -\n 9 files changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 07d7062bfb8..14ca0af2ade 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4567,11 +4567,9 @@ static int pick_commits(struct repository *r,\n \t\t\tstruct replay_opts *opts)\n {\n \tint res = 0, reschedule = 0;\n-\tchar *prev_reflog_action;\n \n \t/* Note that 0 for 3rd parameter of setenv means set only if not set */\n \tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n-\tprev_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));\n \tif (opts->allow_ff)\n \t\tassert(!(opts->signoff || opts->no_commit ||\n \t\t\t opts->record_origin || should_edit(opts) ||\n@@ -4618,15 +4616,20 @@ static int pick_commits(struct repository *r,\n \t\t\t}\n \t\t}\n \t\tif (item->command <= TODO_SQUASH) {\n-\t\t\tif (is_rebase_i(opts))\n+\t\t\tchar *prev_reflog_action = NULL;\n+\n+\t\t\tif (is_rebase_i(opts)) {\n+\t\t\t\tprev_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));\n \t\t\t\tsetenv(GIT_REFLOG_ACTION, reflog_message(opts,\n \t\t\t\t\tcommand_to_string(item->command), NULL),\n \t\t\t\t\t1);\n+\t\t\t}\n \t\t\tres = do_pick_commit(r, item, opts,\n \t\t\t\t\t is_final_fixup(todo_list),\n \t\t\t\t\t &check_todo);\n \t\t\tif (is_rebase_i(opts))\n \t\t\t\tsetenv(GIT_REFLOG_ACTION, prev_reflog_action, 1);\n+\t\t\tfree(prev_reflog_action);\n \t\t\tif (is_rebase_i(opts) && res < 0) {\n \t\t\t\t/* Reschedule */\n \t\t\t\tadvise(_(rescheduled_advice),\ndiff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh\nindex 1f6c4ed0428..4b6372f4c3e 100755\n--- a/t/t2012-checkout-last.sh\n+++ b/t/t2012-checkout-last.sh\n@@ -5,6 +5,7 @@ test_description='checkout can switch to last branch and merge base'\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/t3409-rebase-environ.sh b/t/t3409-rebase-environ.sh\nindex 83ffb39d9ff..acaf5558dbe 100755\n--- a/t/t3409-rebase-environ.sh\n+++ b/t/t3409-rebase-environ.sh\n@@ -2,6 +2,7 @@\n \n test_description='git rebase interactive environment'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh\nindex 9fab0d779bb..e8456831e8b 100755\n--- a/t/t3413-rebase-hook.sh\n+++ b/t/t3413-rebase-hook.sh\n@@ -5,6 +5,7 @@ test_description='git rebase with its hook(s)'\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/t3433-rebase-across-mode-change.sh b/t/t3433-rebase-across-mode-change.sh\nindex 05df964670f..c8172b08522 100755\n--- a/t/t3433-rebase-across-mode-change.sh\n+++ b/t/t3433-rebase-across-mode-change.sh\n@@ -2,6 +2,7 @@\n \n test_description='git rebase across mode change'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh\nindex a39de8c1126..07ca46fb0d5 100755\n--- a/t/t7504-commit-msg-hook.sh\n+++ b/t/t7504-commit-msg-hook.sh\n@@ -5,6 +5,7 @@ test_description='commit-msg hook'\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 'with no hook' '\ndiff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh\nindex 419f055721d..743fbe1fe46 100755\n--- a/t/t9115-git-svn-dcommit-funky-renames.sh\n+++ b/t/t9115-git-svn-dcommit-funky-renames.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn dcommit can commit renames of files with ugly names'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'load repository with strange names' '\ndiff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh\nindex 79c26ed69c1..09606f1b3cf 100755\n--- a/t/t9146-git-svn-empty-dirs.sh\n+++ b/t/t9146-git-svn-empty-dirs.sh\n@@ -4,7 +4,6 @@\n \n test_description='git svn creates empty directories'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'initialize repo' '\ndiff --git a/t/t9160-git-svn-preserve-empty-dirs.sh b/t/t9160-git-svn-preserve-empty-dirs.sh\nindex 9cf7a1427ab..36c6b1a12ff 100755\n--- a/t/t9160-git-svn-preserve-empty-dirs.sh\n+++ b/t/t9160-git-svn-preserve-empty-dirs.sh\n@@ -9,7 +9,6 @@ This test uses git to clone a Subversion repository that contains empty\n directories, and checks that corresponding directories are created in the\n local Git repository with placeholder files.'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n GIT_REPO=git-svn-repo\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466440","messageId":"patch-11.17-c7722f68ae2-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 11/17] connected.c: free the \"struct packed_git\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:10Z","receivedAt":"2022-11-03T17:07:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"new_pack\" we allocate in check_connected() wasn't being\nfree'd. Let's do that before we return from the function. This has\nleaked ever since \"new_pack\" was added to this function in\nc6807a40dcd (clone: open a shortcut for connectivity check,\n2013-05-26).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n connected.c | 6 +++++-\n t/t3050-subprojects-fetch.sh | 1 +\n t/t4067-diff-partial-clone.sh | 1 +\n t/t5544-pack-objects-hook.sh | 2 ++\n t/t5610-clone-detached.sh | 1 +\n t/t5611-clone-config.sh | 1 +\n t/t5614-clone-submodules-shallow.sh | 1 +\n t/t5617-clone-submodules-remote.sh | 1 +\n 8 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/connected.c b/connected.c\nindex 74a20cb32e7..b7770825c7b 100644\n--- a/connected.c\n+++ b/connected.c\n@@ -85,6 +85,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n promisor_pack_found:\n \t\t\t;\n \t\t} while ((oid = fn(cb_data)) != NULL);\n+\t\tfree(new_pack);\n \t\treturn 0;\n \t}\n \n@@ -118,8 +119,10 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n \telse\n \t\trev_list.no_stderr = opt->quiet;\n \n-\tif (start_command(&rev_list))\n+\tif (start_command(&rev_list)) {\n+\t\tfree(new_pack);\n \t\treturn error(_(\"Could not run 'git rev-list'\"));\n+\t}\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n@@ -151,5 +154,6 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n \t\terr = error_errno(_(\"failed to close rev-list's stdin\"));\n \n \tsigchain_pop(SIGPIPE);\n+\tfree(new_pack);\n \treturn finish_command(&rev_list) || err;\n }\ndiff --git a/t/t3050-subprojects-fetch.sh b/t/t3050-subprojects-fetch.sh\nindex f1f09abdd9b..38846941655 100755\n--- a/t/t3050-subprojects-fetch.sh\n+++ b/t/t3050-subprojects-fetch.sh\n@@ -2,6 +2,7 @@\n \n test_description='fetching and pushing project with subproject'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh\nindex 28f42a4046e..f60f5cbd65f 100755\n--- a/t/t4067-diff-partial-clone.sh\n+++ b/t/t4067-diff-partial-clone.sh\n@@ -2,6 +2,7 @@\n \n test_description='behavior of diff when reading objects in a partial clone'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'git show batches blobs' '\ndiff --git a/t/t5544-pack-objects-hook.sh b/t/t5544-pack-objects-hook.sh\nindex 54f54f8d2eb..1a9e14bbccd 100755\n--- a/t/t5544-pack-objects-hook.sh\n+++ b/t/t5544-pack-objects-hook.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test custom script in place of pack-objects'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'create some history to fetch' '\ndiff --git a/t/t5610-clone-detached.sh b/t/t5610-clone-detached.sh\nindex a7ec21eda5a..022ed3d87c3 100755\n--- a/t/t5610-clone-detached.sh\n+++ b/t/t5610-clone-detached.sh\n@@ -4,6 +4,7 @@ test_description='test cloning a repository with detached HEAD'\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 head_is_detached() {\ndiff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh\nindex 4b3877216ee..727caff4433 100755\n--- a/t/t5611-clone-config.sh\n+++ b/t/t5611-clone-config.sh\n@@ -4,6 +4,7 @@ test_description='tests for git clone -c key=value'\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 'clone -c sets config in cloned repo' '\ndiff --git a/t/t5614-clone-submodules-shallow.sh b/t/t5614-clone-submodules-shallow.sh\nindex 0c85ef834ab..c2a2bb453ee 100755\n--- a/t/t5614-clone-submodules-shallow.sh\n+++ b/t/t5614-clone-submodules-shallow.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test shallow cloning of repos with submodules'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n pwd=$(pwd)\ndiff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh\nindex 68843382493..5a4d7936a72 100755\n--- a/t/t5617-clone-submodules-remote.sh\n+++ b/t/t5617-clone-submodules-remote.sh\n@@ -5,6 +5,7 @@ test_description='Test cloning repos with submodules using remote-tracking branc\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 pwd=$(pwd)\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466441","messageId":"patch-13.17-fda9914b558-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 13/17] rebase: don't leak on \"--abort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:12Z","receivedAt":"2022-11-03T17:07:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a leak in the recent 6159e7add49 (rebase --abort: improve reflog\nmessage, 2022-10-12). Before that commit we'd strbuf_release() the\nreflog message we were formatting, but when that code was refactored\nto use \"ropts.head_msg\" the strbuf_release() was omitted.\n\nIdeally the three users of \"ropts\" in cmd_rebase() should use\ndifferent \"ropts\" variables, in practice they're completely separate,\nas this and the other user in the \"switch\" statement will \"goto\ncleanup\", which won't touch \"ropts\".\n\nThe third caller after the \"switch\" is then unreachable if we take\nthese two branches, so all of them are getting a \"{ 0 }\" init'd\n\"ropts\".\n\nSo it's OK that we're leaving a stale pointer in \"ropts.head_msg\",\ncleaning it up was our responsibility, and it won't be used again.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/rebase.c | 1 +\n t/t7517-per-repo-email.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 64aed11c85d..8a8b5e34786 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1320,6 +1320,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tif (reset_head(the_repository, &ropts) < 0)\n \t\t\tdie(_(\"could not move back to %s\"),\n \t\t\t oid_to_hex(&options.orig_head->object.oid));\n+\t\tstrbuf_release(&head_msg);\n \t\tremove_branch_state(the_repository, 0);\n \t\tret = finish_rebase(&options);\n \t\tgoto cleanup;\ndiff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh\nindex 163ae804685..efc6496e2b2 100755\n--- a/t/t7517-per-repo-email.sh\n+++ b/t/t7517-per-repo-email.sh\n@@ -9,6 +9,7 @@ test_description='per-repo forced setting of email address'\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 a likely user.useConfigOnly use case' '\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466442","messageId":"patch-16.17-f7d39020a7c-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 16/17] revert: fix parse_options_concat() leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:15Z","receivedAt":"2022-11-03T17:08:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Free memory from parse_options_concat(), which comes from code\noriginally added (then extended) in [1].\n\nAt this point we could get several more tests leak-free by free()-ing\nthe xstrdup() just above the line being changed, but that one's\ntrickier than it seems. The sequencer_remove_state() function\nsupposedly owns it, but sometimes we don't call it. I have a fix for\nit, but it's non-trivial, so let's fix the easy one first.\n\n1. c62f6ec341b (revert: add --ff option to allow fast forward when\n cherry-picking, 2010-03-06)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/revert.c | 1 +\n t/t3429-rebase-edit-todo.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 0f81c8a795a..8bc87e4c770 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -221,6 +221,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \topts->strategy = xstrdup_or_null(opts->strategy);\n \tif (!opts->strategy && getenv(\"GIT_TEST_MERGE_ALGORITHM\"))\n \t\topts->strategy = xstrdup(getenv(\"GIT_TEST_MERGE_ALGORITHM\"));\n+\tfree(options);\n \n \tif (cmd == 'q') {\n \t\tint ret = sequencer_remove_state(opts);\ndiff --git a/t/t3429-rebase-edit-todo.sh b/t/t3429-rebase-edit-todo.sh\nindex abd66f36021..8e0d03969a2 100755\n--- a/t/t3429-rebase-edit-todo.sh\n+++ b/t/t3429-rebase-edit-todo.sh\n@@ -2,6 +2,7 @@\n \n test_description='rebase should reread the todo file if an exec modifies it'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-rebase.sh\n \n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466443","messageId":"patch-14.17-02e1dddf149-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 14/17] sequencer.c: fix sequencer_continue() leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:13Z","receivedAt":"2022-11-03T17:08:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a leak in the recent da1d63363f1 (rebase --merge: fix reflog when\ncontinuing, 2022-10-12), per [1] the author was under the impression\nthat by calling \"setenv()\" the C library took possession of the\nstring, but we need to free() it.\n\nAs [1] also notes there's upcoming changes to do some larger rewrite\nof these codepaths in sequencer.c, but let's first do this much\nsmaller and isolated leak fix.\n\n1. https://lore.kernel.org/git/86699708-d631-fb49-482c-af27204a3570@dunelm.org.uk/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 14ca0af2ade..3095d0d2b3b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5069,10 +5069,12 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)\n \t\tprevious_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));\n \t\tsetenv(GIT_REFLOG_ACTION, reflog_message(opts, \"continue\", NULL), 1);\n \t\tif (commit_staged_changes(r, opts, &todo_list)) {\n+\t\t\tfree(previous_reflog_action);\n \t\t\tres = -1;\n \t\t\tgoto release_todo_list;\n \t\t}\n \t\tsetenv(GIT_REFLOG_ACTION, previous_reflog_action, 1);\n+\t\tfree(previous_reflog_action);\n \t} else if (!file_exists(get_todo_path(opts)))\n \t\treturn continue_single_pick(r, opts);\n \telse if ((res = read_populate_todo(r, &todo_list, opts)))\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466444","messageId":"patch-15.17-c9f51abf0e2-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 15/17] cherry-pick: free \"struct replay_opts\" members","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:14Z","receivedAt":"2022-11-03T17:08:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Call the release_revisions() function added in\n1878b5edc03 (revision.[ch]: provide and start using a\nrelease_revisions(), 2022-04-13) in cmd_cherry_pick(), as well as\nfreeing the xmalloc()'d \"revs\" member itself.\n\nThis is the same change as the one made for cmd_revert() a few lines\nabove it in fd74ac95ac3 (revert: free \"struct replay_opts\" members,\n2022-07-01).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/revert.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex ee32c714a76..0f81c8a795a 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -261,6 +261,9 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)\n \topts.action = REPLAY_PICK;\n \tsequencer_init_config(&opts);\n \tres = run_sequencer(argc, argv, &opts);\n+\tif (opts.revs)\n+\t\trelease_revisions(opts.revs);\n+\tfree(opts.revs);\n \tif (res < 0)\n \t\tdie(_(\"cherry-pick failed\"));\n \treturn res;\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466445","messageId":"patch-17.17-33a5753cc3a-20221103T164632Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH 17/17] built-ins: use free() not UNLEAK() if trivial, rm dead code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-03T17:06:16Z","receivedAt":"2022-11-03T17:08:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"For a lot of uses of UNLEAK() it would be quite tricky to release the\nmemory involved, or we're missing the relevant *_(release|clear)()\nfunctions. But in these cases we have them already, and can just\ninvoke them on the variable(s) involved, instead of UNLEAK().\n\nFor \"builtin/worktree.c\" the UNLEAK() was also added in [1], but the\nstruct member it's unleaking was removed in [2]. The only non-\"int\"\nmember of that structure is \"const char *keep_locked\", which comes to\nus via \"argv\" or a string literal[3].\n\nWe have good visibility via the compiler and\ntooling (e.g. SANITIZE=address) on bad free()-ing, but none on\nUNLEAK() we don't need anymore. So let's prefer releasing the memory\nwhen it's easy.\n\nFor \"bugreport\", \"worktree\" and \"config\" we need to start using a \"ret\n= ...\" return pattern. For \"builtin/bugreport.c\" these UNLEAK() were\nadded in [4], and for \"builtin/config.c\" in [1].\n\nFor \"config\" the code seen here was the only user of the \"value\"\nvariable. For \"ACTION_{RENAME,REMOVE}_SECTION\" we need to be sure to\nreturn the right exit code in the cases where we were relying on\nfalling through to the top-level.\n\nI think there's still a use-case for UNLEAK(), but hat it's changed\nsince then. Using it so that \"we can see the real leaks\" is\ncounter-productive in these cases.\n\nIt's more useful to have UNLEAK() be a marker of the remaining odd\ncases where it's hard to free() the memory for whatever reason. With\nthis change less than 20 of them remain in-tree.\n\n1. 0e5bba53af7 (add UNLEAK annotation for reducing leak false\n positives, 2017-09-08)\n2. d861d34a6ed (worktree: remove extra members from struct add_opts,\n 2018-04-24)\n3. 0db4961c49b (worktree: teach `add` to accept --reason with\n --lock, 2021-07-15)\n4. 0e5bba53af7 and 00d8c311050 (commit: fix \"author_ident\" leak,\n 2022-05-12).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/add.c | 2 +-\n builtin/bugreport.c | 9 ++++++---\n builtin/commit.c | 6 +++---\n builtin/config.c | 42 ++++++++++++++++++++----------------------\n builtin/diff.c | 2 +-\n builtin/worktree.c | 7 ++++---\n 6 files changed, 35 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex f84372964c8..c68ebafed5e 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -695,6 +695,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"Unable to write new index file\"));\n \n \tdir_clear(&dir);\n-\tUNLEAK(pathspec);\n+\tclear_pathspec(&pathspec);\n \treturn exit_status;\n }\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex 96052541cbf..5bc254be80f 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -106,6 +106,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)\n \tconst char *user_relative_path = NULL;\n \tchar *prefixed_filename;\n \tsize_t output_path_len;\n+\tint ret;\n \n \tconst struct option bugreport_options[] = {\n \t\tOPT_CALLBACK_F(0, \"diagnose\", &diagnose, N_(\"mode\"),\n@@ -182,7 +183,9 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)\n \t\tuser_relative_path);\n \n \tfree(prefixed_filename);\n-\tUNLEAK(buffer);\n-\tUNLEAK(report_path);\n-\treturn !!launch_editor(report_path.buf, NULL, NULL);\n+\tstrbuf_release(&buffer);\n+\n+\tret = !!launch_editor(report_path.buf, NULL, NULL);\n+\tstrbuf_release(&report_path);\n+\treturn ret;\n }\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c291199b704..f88a29167f4 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1874,8 +1874,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tapply_autostash(git_path_merge_autostash(the_repository));\n \n cleanup:\n-\tUNLEAK(author_ident);\n-\tUNLEAK(err);\n-\tUNLEAK(sb);\n+\tstrbuf_release(&author_ident);\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&sb);\n \treturn ret;\n }\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 753e5fac297..060cf9f3e05 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -639,8 +639,9 @@ static char *default_user_config(void)\n int cmd_config(int argc, const char **argv, const char *prefix)\n {\n \tint nongit = !startup_info->have_repository;\n-\tchar *value;\n+\tchar *value = NULL;\n \tint flags = 0;\n+\tint ret = 0;\n \n \tgiven_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));\n \n@@ -856,44 +857,38 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tfree(config_file);\n \t}\n \telse if (actions == ACTION_SET) {\n-\t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n \t\tret = git_config_set_in_file_gently(given_config_source.file, argv[0], value);\n \t\tif (ret == CONFIG_NOTHING_SET)\n \t\t\terror(_(\"cannot overwrite multiple values with a single value\\n\"\n \t\t\t\" Use a regexp, --add or --replace-all to change %s.\"), argv[0]);\n-\t\treturn ret;\n \t}\n \telse if (actions == ACTION_SET_ALL) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n-\t\treturn git_config_set_multivar_in_file_gently(given_config_source.file,\n-\t\t\t\t\t\t\t argv[0], value, argv[2],\n-\t\t\t\t\t\t\t flags);\n+\t\tret = git_config_set_multivar_in_file_gently(given_config_source.file,\n+\t\t\t\t\t\t\t argv[0], value, argv[2],\n+\t\t\t\t\t\t\t flags);\n \t}\n \telse if (actions == ACTION_ADD) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n-\t\treturn git_config_set_multivar_in_file_gently(given_config_source.file,\n-\t\t\t\t\t\t\t argv[0], value,\n-\t\t\t\t\t\t\t CONFIG_REGEX_NONE,\n-\t\t\t\t\t\t\t flags);\n+\t\tret = git_config_set_multivar_in_file_gently(given_config_source.file,\n+\t\t\t\t\t\t\t argv[0], value,\n+\t\t\t\t\t\t\t CONFIG_REGEX_NONE,\n+\t\t\t\t\t\t\t flags);\n \t}\n \telse if (actions == ACTION_REPLACE_ALL) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n-\t\treturn git_config_set_multivar_in_file_gently(given_config_source.file,\n-\t\t\t\t\t\t\t argv[0], value, argv[2],\n-\t\t\t\t\t\t\t flags | CONFIG_FLAGS_MULTI_REPLACE);\n+\t\tret = git_config_set_multivar_in_file_gently(given_config_source.file,\n+\t\t\t\t\t\t\t argv[0], value, argv[2],\n+\t\t\t\t\t\t\t flags | CONFIG_FLAGS_MULTI_REPLACE);\n \t}\n \telse if (actions == ACTION_GET) {\n \t\tcheck_argc(argc, 1, 2);\n@@ -934,26 +929,28 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\t\t flags | CONFIG_FLAGS_MULTI_REPLACE);\n \t}\n \telse if (actions == ACTION_RENAME_SECTION) {\n-\t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tret = git_config_rename_section_in_file(given_config_source.file,\n \t\t\t\t\t\t\targv[0], argv[1]);\n \t\tif (ret < 0)\n \t\t\treturn ret;\n-\t\tif (ret == 0)\n+\t\telse if (!ret)\n \t\t\tdie(_(\"no such section: %s\"), argv[0]);\n+\t\telse\n+\t\t\tret = 0;\n \t}\n \telse if (actions == ACTION_REMOVE_SECTION) {\n-\t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 1, 1);\n \t\tret = git_config_rename_section_in_file(given_config_source.file,\n \t\t\t\t\t\t\targv[0], NULL);\n \t\tif (ret < 0)\n \t\t\treturn ret;\n-\t\tif (ret == 0)\n+\t\telse if (!ret)\n \t\t\tdie(_(\"no such section: %s\"), argv[0]);\n+\t\telse\n+\t\t\tret = 0;\n \t}\n \telse if (actions == ACTION_GET_COLOR) {\n \t\tcheck_argc(argc, 1, 2);\n@@ -966,5 +963,6 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\treturn get_colorbool(argv[0], argc == 2);\n \t}\n \n-\treturn 0;\n+\tfree(value);\n+\treturn ret;\n }\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 854d2c5a5c4..cb63f157dd1 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -609,7 +609,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \tif (1 < rev.diffopt.skip_stat_unmatch)\n \t\trefresh_index_quietly();\n \trelease_revisions(&rev);\n-\tUNLEAK(ent);\n+\tobject_array_clear(&ent);\n \tUNLEAK(blob);\n \treturn result;\n }\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 4a24d53be15..591d659faea 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -629,6 +629,7 @@ static int add(int ac, const char **av, const char *prefix)\n \t\t\t N_(\"try to match the new branch name with a remote-tracking branch\")),\n \t\tOPT_END()\n \t};\n+\tint ret;\n \n \tmemset(&opts, 0, sizeof(opts));\n \topts.checkout = 1;\n@@ -705,9 +706,9 @@ static int add(int ac, const char **av, const char *prefix)\n \t\tdie(_(\"--[no-]track can only be used if a new branch is created\"));\n \t}\n \n-\tUNLEAK(path);\n-\tUNLEAK(opts);\n-\treturn add_worktree(path, branch, &opts);\n+\tret = add_worktree(path, branch, &opts);\n+\tfree(path);\n+\treturn ret;\n }\n \n static void show_worktree_porcelain(struct worktree *wt, int line_terminator)\n-- \n2.38.0.1451.g86b35f4140a\n\n"},{"id":"466533","messageId":"f9df4dfc-6024-deb0-5277-bab96c81a875@dunelm.org.uk","threadId":"58743","inReplyTo":"patch-13.17-fda9914b558-20221103T164632Z-avarab@gmail.com","subject":"Re: [PATCH 13/17] rebase: don't leak on \"--abort\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-04T14:42:25Z","receivedAt":"2022-11-04T14:43:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\n\nOn 03/11/2022 17:06, Ævar Arnfjörð Bjarmason wrote:\n> Fix a leak in the recent 6159e7add49 (rebase --abort: improve reflog\n> message, 2022-10-12). Before that commit we'd strbuf_release() the\n> reflog message we were formatting, but when that code was refactored\n> to use \"ropts.head_msg\" the strbuf_release() was omitted.\n> \n> Ideally the three users of \"ropts\" in cmd_rebase() should use\n> different \"ropts\" variables, in practice they're completely separate,\n> as this and the other user in the \"switch\" statement will \"goto\n> cleanup\", which won't touch \"ropts\".\n> \n> The third caller after the \"switch\" is then unreachable if we take\n> these two branches, so all of them are getting a \"{ 0 }\" init'd\n> \"ropts\".\n> \n> So it's OK that we're leaving a stale pointer in \"ropts.head_msg\",\n> cleaning it up was our responsibility, and it won't be used again.\n\nThis looks fine. One could argue that the leak is not a \"real\" leak as \nwe're about to exit but the omission of strbuf_release() was \nunintentional and fix is nice and simple.\n\nThanks\n\nPhillip\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/rebase.c | 1 +\n> t/t7517-per-repo-email.sh | 1 +\n> 2 files changed, 2 insertions(+)\n> \n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 64aed11c85d..8a8b5e34786 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1320,6 +1320,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\tif (reset_head(the_repository, &ropts) < 0)\n> \t\t\tdie(_(\"could not move back to %s\"),\n> \t\t\t oid_to_hex(&options.orig_head->object.oid));\n> +\t\tstrbuf_release(&head_msg);\n> \t\tremove_branch_state(the_repository, 0);\n> \t\tret = finish_rebase(&options);\n> \t\tgoto cleanup;\n> diff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh\n> index 163ae804685..efc6496e2b2 100755\n> --- a/t/t7517-per-repo-email.sh\n> +++ b/t/t7517-per-repo-email.sh\n> @@ -9,6 +9,7 @@ test_description='per-repo forced setting of email address'\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 a likely user.useConfigOnly use case' '\n"},{"id":"466534","messageId":"b9f08244-36bc-b74f-ac1a-b665423bc2e7@dunelm.org.uk","threadId":"58743","inReplyTo":"patch-10.17-9c70bfa334e-20221103T164632Z-avarab@gmail.com","subject":"Re: [PATCH 10/17] sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-04T14:50:02Z","receivedAt":"2022-11-04T14:50:11Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 03/11/2022 17:06, Ævar Arnfjörð Bjarmason wrote:\n> When \"read_strategy_opts()\" is called we may have populated the\n> \"opts->strategy\" before, so we'll need to free() it to avoid leaking\n> memory. \n\nWhere is the previous value coming from? I guess it may be the config \nbut otherwise I'm confused.\n\n> Along with the preceding commit this change various\n> rebase-related tests pass.\n\nReally? at a glance the previous patch looks unrelated and there are no \ntests marked as passing in this one.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> sequencer.c | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index e658df7e8ff..07d7062bfb8 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2894,6 +2894,7 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n> \tstrbuf_reset(buf);\n> \tif (!read_oneliner(buf, rebase_path_strategy(), 0))\n> \t\treturn;\n> +\tfree(opts->strategy);\n> \topts->strategy = strbuf_detach(buf, NULL);\n> \tif (!read_oneliner(buf, rebase_path_strategy_opts(), 0))\n> \t\treturn;\n"},{"id":"466539","messageId":"8eec228d-d392-523d-2415-149b946f642e@dunelm.org.uk","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"Re: [PATCH 00/17] leak fixes: use existing constructors & other trivia","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-04T15:20:43Z","receivedAt":"2022-11-04T15:20:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 03/11/2022 17:05, Ævar Arnfjörð Bjarmason wrote:\n> With the very minor exceptions of:\n> \n> * 03-04/17 (which need trivial oilerplate)\n> * 05/17 (need to add trivial control flow to a free_*() function)\n> * 12/17 (narrowing scope of allocation)\n\nI've only looked at the rebase related patches. I'd really appreciate it \nif you could drop patches 12 & 14 as they conflict with [1] that fixes \nthese issues by removing the setenv() calls.\n\nThanks\n\nPhillip\n\n[1] \nhttps://lore.kernel.org/git/pull.1405.git.1667575142.gitgitgadget@gmail.com\n\n> * 17/17: Add \"goto ret\" pattern, combine two \"ret\" variables\n> \n> These are all \"one-line\" leak fixes where we merely need to make use\n> of an existing release function. The \"one-line\" only having the slight\n> disclaimer of needing to e.g. add braces to an \"if\" in one case, etc.\n> \n> Each commit in this series is tested with:\n> \n> \tGIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true \\\n> \tmake SANITIZE=leak test\n> \n> I.e. mark tests as leak-free as we fix the leaks.\n> \n> In 17/17 I replace uses of UNLEAK() where we can just as easily free()\n> instead, i.e. most of it's built-ins doing UNLEAK(x) instead of\n> strbuf_release(&x) etc.\n> \n> As 17/17 notes I still think these's a place for unleak (some of the\n> remaining ones are quite tricky), but that we gain more from leaving\n> it for those tricky cases. Before this series we have 28 uses of\n> UNLEAK(), after it's 15.\n> \n> Ævar Arnfjörð Bjarmason (17):\n> tests: mark tests as passing with SANITIZE=leak\n> {reset,merge}: call discard_index() before returning\n> commit: discard partial cache before (re-)reading it\n> read-cache.c: clear and free \"sparse_checkout_patterns\"\n> dir.c: free \"ident\" and \"exclude_per_dir\" in \"struct untracked_cache\"\n> built-ins & libs & helpers: add/move destructors, fix leaks\n> unpack-file: fix ancient leak in create_temp_file()\n> revision API: call graph_clear() in release_revisions()\n> ls-files: fix a --with-tree memory leak\n> sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()\n> connected.c: free the \"struct packed_git\"\n> sequencer.c: fix a pick_commits() leak\n> rebase: don't leak on \"--abort\"\n> sequencer.c: fix sequencer_continue() leak\n> cherry-pick: free \"struct replay_opts\" members\n> revert: fix parse_options_concat() leak\n> built-ins: use free() not UNLEAK() if trivial, rm dead code\n> \n> builtin/add.c | 2 +-\n> builtin/bugreport.c | 9 +++--\n> builtin/checkout.c | 2 ++\n> builtin/commit.c | 13 +++++---\n> builtin/config.c | 42 +++++++++++-------------\n> builtin/diff.c | 2 +-\n> builtin/ls-files.c | 1 +\n> builtin/merge.c | 1 +\n> builtin/rebase.c | 4 +++\n> builtin/repack.c | 2 +-\n> builtin/reset.c | 2 ++\n> builtin/rev-parse.c | 1 +\n> builtin/revert.c | 4 +++\n> builtin/stash.c | 2 ++\n> builtin/unpack-file.c | 1 +\n> builtin/worktree.c | 7 ++--\n> connected.c | 6 +++-\n> dir.c | 10 ++++--\n> dir.h | 1 +\n> read-cache.c | 5 +++\n> ref-filter.c | 1 +\n> revision.c | 1 +\n> sequencer.c | 12 +++++--\n> t/helper/test-fake-ssh.c | 1 +\n> t/t0068-for-each-repo.sh | 1 +\n> t/t0070-fundamental.sh | 1 +\n> t/t1011-read-tree-sparse-checkout.sh | 1 +\n> t/t1022-read-tree-partial-clone.sh | 2 +-\n> t/t1404-update-ref-errors.sh | 2 ++\n> t/t1409-avoid-packing-refs.sh | 1 +\n> t/t1413-reflog-detach.sh | 1 +\n> t/t1501-work-tree.sh | 2 ++\n> t/t2012-checkout-last.sh | 1 +\n> t/t2018-checkout-branch.sh | 1 +\n> t/t2025-checkout-no-overlay.sh | 1 +\n> t/t3009-ls-files-others-nonsubmodule.sh | 1 +\n> t/t3010-ls-files-killed-modified.sh | 2 ++\n> t/t3050-subprojects-fetch.sh | 1 +\n> t/t3060-ls-files-with-tree.sh | 2 ++\n> t/t3409-rebase-environ.sh | 1 +\n> t/t3413-rebase-hook.sh | 1 +\n> t/t3428-rebase-signoff.sh | 1 +\n> t/t3429-rebase-edit-todo.sh | 1 +\n> t/t3433-rebase-across-mode-change.sh | 1 +\n> t/t4015-diff-whitespace.sh | 4 +--\n> t/t4045-diff-relative.sh | 2 ++\n> t/t4052-stat-output.sh | 1 +\n> t/t4053-diff-no-index.sh | 1 +\n> t/t4067-diff-partial-clone.sh | 1 +\n> t/t4111-apply-subdir.sh | 1 +\n> t/t4135-apply-weird-filenames.sh | 1 +\n> t/t4213-log-tabexpand.sh | 1 +\n> t/t5544-pack-objects-hook.sh | 2 ++\n> t/t5554-noop-fetch-negotiator.sh | 2 ++\n> t/t5610-clone-detached.sh | 1 +\n> t/t5611-clone-config.sh | 1 +\n> t/t5614-clone-submodules-shallow.sh | 1 +\n> t/t5617-clone-submodules-remote.sh | 1 +\n> t/t5618-alternate-refs.sh | 2 ++\n> t/t6060-merge-index.sh | 2 ++\n> t/t6301-for-each-ref-errors.sh | 1 +\n> t/t6401-merge-criss-cross.sh | 2 ++\n> t/t6406-merge-attr.sh | 1 +\n> t/t6407-merge-binary.sh | 1 +\n> t/t6415-merge-dir-to-symlink.sh | 1 +\n> t/t6435-merge-sparse.sh | 1 +\n> t/t7103-reset-bare.sh | 2 +-\n> t/t7504-commit-msg-hook.sh | 1 +\n> t/t7517-per-repo-email.sh | 1 +\n> t/t7520-ignored-hook-warning.sh | 1 +\n> t/t7605-merge-resolve.sh | 1 +\n> t/t7614-merge-signoff.sh | 1 +\n> t/t9003-help-autocorrect.sh | 2 ++\n> t/t9115-git-svn-dcommit-funky-renames.sh | 1 -\n> t/t9146-git-svn-empty-dirs.sh | 1 -\n> t/t9148-git-svn-propset.sh | 1 -\n> t/t9160-git-svn-preserve-empty-dirs.sh | 1 -\n> 77 files changed, 150 insertions(+), 51 deletions(-)\n> \n"},{"id":"466557","messageId":"Y2WHxveDDH5YLpZq@nand.local","threadId":"58743","inReplyTo":"b9f08244-36bc-b74f-ac1a-b665423bc2e7@dunelm.org.uk","subject":"Re: [PATCH 10/17] sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-04T21:44:38Z","receivedAt":"2022-11-04T21:44:42Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Nov 04, 2022 at 02:50:02PM +0000, Phillip Wood wrote:\n> > Along with the preceding commit this change various\n> > rebase-related tests pass.\n>\n> Really? at a glance the previous patch looks unrelated and there are no\n> tests marked as passing in this one.\n\nYeah, I wondered the same thing. What are we missing here?\n\nThanks,\nTaylor\n"},{"id":"466576","messageId":"221105.86eduhy3l8.gmgdl@evledraar.gmail.com","threadId":"58743","inReplyTo":"f9df4dfc-6024-deb0-5277-bab96c81a875@dunelm.org.uk","subject":"Re: [PATCH 13/17] rebase: don't leak on \"--abort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-05T12:01:42Z","receivedAt":"2022-11-05T12:43:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 04 2022, Phillip Wood wrote:\n\n> On 03/11/2022 17:06, Ævar Arnfjörð Bjarmason wrote:\n>> Fix a leak in the recent 6159e7add49 (rebase --abort: improve reflog\n>> message, 2022-10-12). Before that commit we'd strbuf_release() the\n>> reflog message we were formatting, but when that code was refactored\n>> to use \"ropts.head_msg\" the strbuf_release() was omitted.\n>> Ideally the three users of \"ropts\" in cmd_rebase() should use\n>> different \"ropts\" variables, in practice they're completely separate,\n>> as this and the other user in the \"switch\" statement will \"goto\n>> cleanup\", which won't touch \"ropts\".\n>> The third caller after the \"switch\" is then unreachable if we take\n>> these two branches, so all of them are getting a \"{ 0 }\" init'd\n>> \"ropts\".\n>> So it's OK that we're leaving a stale pointer in \"ropts.head_msg\",\n>> cleaning it up was our responsibility, and it won't be used again.\n>> [...]\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1320,6 +1320,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t\tif (reset_head(the_repository, &ropts) < 0)\n>> \t\t\tdie(_(\"could not move back to %s\"),\n>> \t\t\t oid_to_hex(&options.orig_head->object.oid));\n>> +\t\tstrbuf_release(&head_msg);\n>> \t\tremove_branch_state(the_repository, 0);\n>> \t\tret = finish_rebase(&options);\n>> \t\tgoto cleanup;\n>\n> This looks fine. One could argue that the leak is not a \"real\" leak as\n> we're about to exit but the omission of strbuf_release() was \n> unintentional and fix is nice and simple.\n\nWhat I've been targeting in this and past leak fix topics is roughly\nwhat valgrind calls[1] \"definitely lost\" and \"indirectly lost\", and\nwhich -fsanitize=leak calls \"that thing I die on whan I run into it\" :)\n\nSo, you and I know we're \"about to exit\" as we're in cmd_rebase() and\nabout to return to common-main.c etc, but it's harder to convince a\ncompiler of that.\n\nMaybe I'm just touchy, sorry. It just feels like every time I submit a\nleak fix topic there's some rehash of the \"why do this at all?\" or \"why\nthis one?\" discussion.\n\nTo summarize (maybe too much for just this reply, but I hope to link\nback to this in the future):\n\n * Historically we've said \"that's just a built-in, let the OS take care\n of it!\"\n\n * I agree with that in principle. It's pretty pointless to free()\n things in cmd_*() in the abstract. We're \"about to exit\", even if\n valgrind etc. call it \"definitely lost\"[1].\n\n * But, it would be nice to have git's APIs not leak, so we could call\n them in loops, replace some of our own sub-process invocations\n etc[2].\n\n * If we had 100% test coverage for those libraries we could just test\n that, and get those leak-free, and ignore the built-ins.\n\n * We don't have that, and probably never will. E.g. \"git log\" and\n friends are *the* things that stress revision.[ch]. So to assert that\n the API is leak-free we really need to assert that its users\n are. Ditto for pretty much any other API we carry.\n\n * Still, we could just say \"if something's alloc'd in cmd_*() don't\n care about it\". Both LSAN and valgrind support \"suppressions\" to\n ignore certain functions, patterns etc.\n\n * But that doesn't really work either, as e.g. \"struct rev_info\" would\n seem to come from such a cmd_*() as far as ownership goes...\n\n * ...\"but you could have an even more clever suppressions mechanism,\n and only ignore those things that are malloc'd by cmd_*(). But not\n e.g. a malloc in revision.c in a struct member in a struct that's on\n the stack in a cmd_*()\", one might say.\n\n Yeah, you could do that, e.g. in this case we'll call strbuf_addf()\n which makes a beeline to malloc(). Sufficiently clever\n post-processing of leak stacktraces could probably make the\n distinction.\n\n But then you run into the problem of e.g. how the the log family and\n others use \"mailmap\". I.e. in that case (and many others) the cmd_*()\n will malloc(), but \"hand it off\" to the API, whose job we know it is\n to free() it.\n\n But we only know because of knowledge about the API, an automated\n system would care, still, it's not unworkable. We could go through\n those, list the exceptions etc. etc.\n\nThat's a lot of context, but I think brings us up-to-date on this\none-line change.\n\nSo, could we do things differently here? Definitely. And if the codebase\nwas in a state where e.g. builtin/*.c had ~1500 UNLEAK() already I'd\nprobably try to extend that, rather than fighting that uphill battle.\n\nBut that's not the case, we have around ~30 of them, fewer after this\ntopic. By far the majority of builtin/* is caring about directly\nrecahable leaks already.\n\nE.g. just above this context in another \"case\" arm we're doing the exact\n\"variable on stack, use it, then release()\", just with a \"struct\nstring_list\".\n\nSo I think this is the right direction to go in, both in this patch & in\ngeneral.\n\n1. https://valgrind.org/docs/manual/mc-manual.html#mc-manual.leaks\n2. Even the assumption that cmd_*() is a one-off is shaky, e.g. for\n cases where we shell out to \"git commit\" we're close now to being\n able to just call cmd_commit(), but let's leave that aside for now).\n\n So, we're pretty close to cmd_*() being a funny API that happens to\n take strvec arguments, which is going to be the shortest path to\n eliminating sub-process invocation in many cases. I.e. as opposed to\n refactoring the cmd_*() into a \"real\" library, that takes a struct\n with options, doesn't call parse_options() etc.\n"},{"id":"466578","messageId":"221105.86a655y3fp.gmgdl@evledraar.gmail.com","threadId":"58743","inReplyTo":"Y2WHxveDDH5YLpZq@nand.local","subject":"Re: [PATCH 10/17] sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-05T12:43:34Z","receivedAt":"2022-11-05T12:46:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 04 2022, Taylor Blau wrote:\n\n> On Fri, Nov 04, 2022 at 02:50:02PM +0000, Phillip Wood wrote:\n>> > Along with the preceding commit this change various\n>> > rebase-related tests pass.\n>>\n>> Really? at a glance the previous patch looks unrelated and there are no\n>> tests marked as passing in this one.\n>\n> Yeah, I wondered the same thing. What are we missing here?\n\nI see I was using \"tests that pass\" in an arguably ambiguous way.\n\nThis does make more tests pass, but not in such a way that the entire\nset of tests in any given file in t/t[0-9]*.sh now pases in its\nentirety, which is the point at which we can add:\n\n\tTEST_PASSES_SANITIZE_LEAK=true\n\n\n"},{"id":"466579","messageId":"221105.865yfty38w.gmgdl@evledraar.gmail.com","threadId":"58743","inReplyTo":"8eec228d-d392-523d-2415-149b946f642e@dunelm.org.uk","subject":"Re: [PATCH 00/17] leak fixes: use existing constructors & other trivia","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-05T12:46:29Z","receivedAt":"2022-11-05T12:50:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 04 2022, Phillip Wood wrote:\n\n> On 03/11/2022 17:05, Ævar Arnfjörð Bjarmason wrote:\n>> With the very minor exceptions of:\n>> * 03-04/17 (which need trivial oilerplate)\n>> * 05/17 (need to add trivial control flow to a free_*() function)\n>> * 12/17 (narrowing scope of allocation)\n>\n> I've only looked at the rebase related patches. I'd really appreciate\n> it if you could drop patches 12 & 14 as they conflict with [1] that\n> fixes these issues by removing the setenv() calls.\n\nSure, sounds good to me.\n\nI see Taylor queued it up like that, and the combination of the two\npasses with the \"true\" and \"check\" mode leak checking modes.\n\nFor context: I saw that you planned to fix those in some follow-up\ntopic, but had this mostly ready before the new leak in the recent topic\nshowed up, and that leak had big knock-on effects in what test files I\ncould mark as passing.\n\nSo I figured I'd include some more more narrow fixes, but I'm even\nhappier to see the root cause addressed.\n"},{"id":"466646","messageId":"ce3c6afa-2f82-d7f6-8919-e3e8912a7555@dunelm.org.uk","threadId":"58743","inReplyTo":"221105.865yfty38w.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 00/17] leak fixes: use existing constructors & other trivia","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-07T09:46:15Z","receivedAt":"2022-11-07T09:46:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 05/11/2022 12:46, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Nov 04 2022, Phillip Wood wrote:\n> \n>> On 03/11/2022 17:05, Ævar Arnfjörð Bjarmason wrote:\n>>> With the very minor exceptions of:\n>>> * 03-04/17 (which need trivial oilerplate)\n>>> * 05/17 (need to add trivial control flow to a free_*() function)\n>>> * 12/17 (narrowing scope of allocation)\n>>\n>> I've only looked at the rebase related patches. I'd really appreciate\n>> it if you could drop patches 12 & 14 as they conflict with [1] that\n>> fixes these issues by removing the setenv() calls.\n> \n> Sure, sounds good to me.\n\nThanks\n\n> I see Taylor queued it up like that, and the combination of the two\n> passes with the \"true\" and \"check\" mode leak checking modes.\n\nThat's great\n\nBest Wishes\n\nPhillip\n\n\n> For context: I saw that you planned to fix those in some follow-up\n> topic, but had this mostly ready before the new leak in the recent topic\n> showed up, and that leak had big knock-on effects in what test files I\n> could mark as passing.\n> \n> So I figured I'd include some more more narrow fixes, but I'm even\n> happier to see the root cause addressed.\n"},{"id":"466845","messageId":"66835bfb-2815-4321-9d06-52f85a0c72f9@dunelm.org.uk","threadId":"58743","inReplyTo":"b9f08244-36bc-b74f-ac1a-b665423bc2e7@dunelm.org.uk","subject":"Re: [PATCH 10/17] sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-08T15:00:55Z","receivedAt":"2022-11-08T15:01:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 04/11/2022 14:50, Phillip Wood wrote:\n> On 03/11/2022 17:06, Ævar Arnfjörð Bjarmason wrote:\n>> When \"read_strategy_opts()\" is called we may have populated the\n>> \"opts->strategy\" before, so we'll need to free() it to avoid leaking\n>> memory. \n> \n> Where is the previous value coming from? I guess it may be the config \n> but otherwise I'm confused.\n\nHaving looked a bit more at this I think this is the wrong fix. The \nreason we're overwriting the existing value is that we're reading some \nof the state files twice. I think the only way to get to this point is \nto go through a code path that calls rebase.c:read_basic_state() which \nalready populates these options via a later call to \nrebase.c:get_replay_opts(). I think the correct fixes looks something \nlike the diff below.\n\nI have also looked at the cherry-pick/revert case and I think that we're \nleaking opts->strategy (and probably some others) when running\n\n\tgit cherry-pick --continue\n\nafter\n\n\tgit -c pull.twohead=recursive cherry-pick -s ort \n\nI'm not sure what your strategy has been with the fixes in this series \nbut we're never going to have 100% coverage of all the option \ncombinations for rebase & cherry-pick so I think it is helpful to treat \nthese LSAN reports as a starting point for looking into why the leak is \noccurring and also look for similar leaks.\n\nBest Wishes\n\nPhillip\n\n--- >8 ---\n sequencer.c | 46 ----------------------------------------------\n 1 file changed, 46 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e658df7e8f..ece34dce9f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2889,55 +2889,12 @@ void parse_strategy_opts(struct replay_opts \n*opts, char *raw_opts)\n }\n }\n\n-static void read_strategy_opts(struct replay_opts *opts, struct strbuf \n*buf)\n-{\n- strbuf_reset(buf);\n- if (!read_oneliner(buf, rebase_path_strategy(), 0))\n- return;\n- opts->strategy = strbuf_detach(buf, NULL);\n- if (!read_oneliner(buf, rebase_path_strategy_opts(), 0))\n- return;\n-\n- parse_strategy_opts(opts, buf->buf);\n-}\n-\n static int read_populate_opts(struct replay_opts *opts)\n {\n if (is_rebase_i(opts)) {\n struct strbuf buf = STRBUF_INIT;\n int ret = 0;\n\n- if (read_oneliner(&buf, rebase_path_gpg_sign_opt(),\n- READ_ONELINER_SKIP_IF_EMPTY)) {\n- if (!starts_with(buf.buf, \"-S\"))\n- strbuf_reset(&buf);\n- else {\n- free(opts->gpg_sign);\n- opts->gpg_sign = xstrdup(buf.buf + 2);\n- }\n- strbuf_reset(&buf);\n- }\n-\n- if (read_oneliner(&buf, \nrebase_path_allow_rerere_autoupdate(),\n- READ_ONELINER_SKIP_IF_EMPTY)) {\n- if (!strcmp(buf.buf, \"--rerere-autoupdate\"))\n- opts->allow_rerere_auto = RERERE_AUTOUPDATE;\n- else if (!strcmp(buf.buf, \"--no-rerere-autoupdate\"))\n- opts->allow_rerere_auto = \nRERERE_NOAUTOUPDATE;\n- strbuf_reset(&buf);\n- }\n-\n- if (file_exists(rebase_path_verbose()))\n- opts->verbose = 1;\n-\n- if (file_exists(rebase_path_quiet()))\n- opts->quiet = 1;\n-\n- if (file_exists(rebase_path_signoff())) {\n- opts->allow_ff = 0;\n- opts->signoff = 1;\n- }\n-\n if (file_exists(rebase_path_cdate_is_adate())) {\n opts->allow_ff = 0;\n opts->committer_date_is_author_date = 1;\n@@ -2959,9 +2916,6 @@ static int read_populate_opts(struct replay_opts \n*opts)\n if (file_exists(rebase_path_keep_redundant_commits()))\n opts->keep_redundant_commits = 1;\n\n- read_strategy_opts(opts, &buf);\n- strbuf_reset(&buf);\n-\n if (read_oneliner(&opts->current_fixups,\n rebase_path_current_fixups(),\n READ_ONELINER_SKIP_IF_EMPTY)) {\n"},{"id":"466848","messageId":"221108.86zgd1qxac.gmgdl@evledraar.gmail.com","threadId":"58743","inReplyTo":"66835bfb-2815-4321-9d06-52f85a0c72f9@dunelm.org.uk","subject":"Re: [PATCH 10/17] sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T15:26:37Z","receivedAt":"2022-11-08T15:30:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 08 2022, Phillip Wood wrote:\n\n> On 04/11/2022 14:50, Phillip Wood wrote:\n>> On 03/11/2022 17:06, Ævar Arnfjörð Bjarmason wrote:\n>>> When \"read_strategy_opts()\" is called we may have populated the\n>>> \"opts->strategy\" before, so we'll need to free() it to avoid leaking\n>>> memory. \n>> Where is the previous value coming from? I guess it may be the\n>> config but otherwise I'm confused.\n>\n> Having looked a bit more at this I think this is the wrong fix. The\n> reason we're overwriting the existing value is that we're reading some \n> of the state files twice. I think the only way to get to this point is\n> to go through a code path that calls rebase.c:read_basic_state() which \n> already populates these options via a later call to\n> rebase.c:get_replay_opts(). I think the correct fixes looks something \n> like the diff below.\n>\n> I have also looked at the cherry-pick/revert case and I think that\n> we're leaking opts->strategy (and probably some others) when running\n>\n> \tgit cherry-pick --continue\n>\n> after\n>\n> \tgit -c pull.twohead=recursive cherry-pick -s ort \n\nRelated: My just-sent [1].\n\nFor this one though, I have a v2 re-roll prepared, in which I explained\nwhy the leak is happening (I'll send this soon to the list, but for\nnow): https://github.com/avar/git/commit/243ab74120b\n\n> I'm not sure what your strategy has been with the fixes in this series\n> but we're never going to have 100% coverage of all the option \n> combinations for rebase & cherry-pick so I think it is helpful to\n> treat these LSAN reports as a starting point for looking into why the\n> leak is occurring and also look for similar leaks.\n\nFor this series I'm aiming to solve some common leaks, and increase our\ntest coverage.\n\nPer [1] the destruction in this area is quite messy, and we should do it\nbetter. But that's a much larger fix. I think in the meantime adding\nthis free() is the bets way to make incremental progress here, and (an\nthe commit linked above notes) it's consistent with existing patterns.\n\n1. https://lore.kernel.org/git/221108.864jv9sc9r.gmgdl@evledraar.gmail.com/\n"},{"id":"466852","messageId":"patch-v2-01.15-58a02e6bb4e-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 01/15] tests: mark tests as passing with SANITIZE=leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:37Z","receivedAt":"2022-11-08T18:17:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This marks tests that have been leak-free since various recent\ncommits, but which were not marked us such when the memory leak was\nfixed. These were mostly discovered with the \"check\" mode added in\nfaececa53f9 (test-lib: have the \"check\" mode for SANITIZE=leak\nconsider leak logs, 2022-07-28).\n\nCommits that fixed the last memory leak in these tests. Per narrowing\ndown when they started to pass under SANITIZE=leak with \"bisect\":\n\n- t1022-read-tree-partial-clone.sh:\n 7e2619d8ff0 (list_objects_filter_options: plug leak of filter_spec\n strings, 2022-09-08)\n\n- t4053-diff-no-index.sh: 07a6f94a6d0 (diff-no-index: release prefixed\n filenames, 2022-09-07)\n\n- t6415-merge-dir-to-symlink.sh: bac92b1f39f (Merge branch\n 'js/ort-clean-up-after-failed-merge', 2022-08-08).\n\n- t5554-noop-fetch-negotiator.sh:\n 66eede4a37c (prepare_repo_settings(): plug leak of config values,\n 2022-09-08)\n\n- t2012-checkout-last.sh, t7504-commit-msg-hook.sh,\n t91{15,46,60}-git-svn-*.sh: The in-flight \"pw/rebase-no-reflog-action\"\n series, upon which this is based:\n https://lore.kernel.org/git/pull.1405.git.1667575142.gitgitgadget@gmail.com/\n\nLet's mark all of these as passing with\n\"TEST_PASSES_SANITIZE_LEAK=true\", to have it regression tested,\nincluding as part of the \"linux-leaks\" CI job.\n\nAdditionally, let's remove the \"!SANITIZE_LEAK\" prerequisite from\ntests that now pass, these were marked as failing in:\n\n- 77e56d55ba6 (diff.c: fix a double-free regression in a18d66cefb,\n 2022-03-17)\n- c4d1d526312 (tests: change some 'test $(git) = \"x\"' to test_cmp,\n 2022-03-07)\n\nThese were not spotted with the new \"check\" mode, but manually, it\ndoesn't cover these sort of prerequisites. There's few enough that we\nshouldn't bother to automate it. They'll be going away sooner than\nlater.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t1022-read-tree-partial-clone.sh | 2 +-\n t/t2012-checkout-last.sh | 1 +\n t/t4015-diff-whitespace.sh | 4 ++--\n t/t4053-diff-no-index.sh | 1 +\n t/t5554-noop-fetch-negotiator.sh | 2 ++\n t/t6415-merge-dir-to-symlink.sh | 1 +\n t/t7103-reset-bare.sh | 2 +-\n t/t7504-commit-msg-hook.sh | 1 +\n t/t9115-git-svn-dcommit-funky-renames.sh | 1 -\n t/t9146-git-svn-empty-dirs.sh | 1 -\n t/t9160-git-svn-preserve-empty-dirs.sh | 1 -\n 11 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t1022-read-tree-partial-clone.sh b/t/t1022-read-tree-partial-clone.sh\nindex a9953b6a71c..d08563de5c9 100755\n--- a/t/t1022-read-tree-partial-clone.sh\n+++ b/t/t1022-read-tree-partial-clone.sh\n@@ -3,7 +3,7 @@\n test_description='git read-tree in partial clones'\n \n TEST_NO_CREATE_REPO=1\n-\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'read-tree in partial clone prefetches in one batch' '\ndiff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh\nindex 1f6c4ed0428..4b6372f4c3e 100755\n--- a/t/t2012-checkout-last.sh\n+++ b/t/t2012-checkout-last.sh\n@@ -5,6 +5,7 @@ test_description='checkout can switch to last branch and merge base'\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/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex f3e20dd5bba..b298f220e01 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -1638,7 +1638,7 @@ test_expect_success 'no effect on diff from --color-moved with --word-diff' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success !SANITIZE_LEAK 'no effect on show from --color-moved with --word-diff' '\n+test_expect_success 'no effect on show from --color-moved with --word-diff' '\n \tgit show --color-moved --word-diff >actual &&\n \tgit show --word-diff >expect &&\n \ttest_cmp expect actual\n@@ -2024,7 +2024,7 @@ test_expect_success '--color-moved rewinds for MIN_ALNUM_COUNT' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success !SANITIZE_LEAK 'move detection with submodules' '\n+test_expect_success 'move detection with submodules' '\n \ttest_create_repo bananas &&\n \techo ripe >bananas/recipe &&\n \tgit -C bananas add recipe &&\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 3feadf0e359..4e9fa0403d3 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -2,6 +2,7 @@\n \n test_description='diff --no-index'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t5554-noop-fetch-negotiator.sh b/t/t5554-noop-fetch-negotiator.sh\nindex 2ac7b5859e7..06991e8e8aa 100755\n--- a/t/t5554-noop-fetch-negotiator.sh\n+++ b/t/t5554-noop-fetch-negotiator.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test noop fetch negotiator'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'noop negotiator does not emit any \"have\"' '\ndiff --git a/t/t6415-merge-dir-to-symlink.sh b/t/t6415-merge-dir-to-symlink.sh\nindex 2655e295f5a..ae00492c768 100755\n--- a/t/t6415-merge-dir-to-symlink.sh\n+++ b/t/t6415-merge-dir-to-symlink.sh\n@@ -4,6 +4,7 @@ test_description='merging when a directory was replaced with a symlink'\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 'create a commit where dir a/b changed to symlink' '\ndiff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh\nindex a60153f9f32..18bbd9975eb 100755\n--- a/t/t7103-reset-bare.sh\n+++ b/t/t7103-reset-bare.sh\n@@ -63,7 +63,7 @@ test_expect_success '\"mixed\" reset is not allowed in bare' '\n \ttest_must_fail git reset --mixed HEAD^\n '\n \n-test_expect_success !SANITIZE_LEAK '\"soft\" reset is allowed in bare' '\n+test_expect_success '\"soft\" reset is allowed in bare' '\n \tgit reset --soft HEAD^ &&\n \tgit show --pretty=format:%s >out &&\n \techo one >expect &&\ndiff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh\nindex a39de8c1126..07ca46fb0d5 100755\n--- a/t/t7504-commit-msg-hook.sh\n+++ b/t/t7504-commit-msg-hook.sh\n@@ -5,6 +5,7 @@ test_description='commit-msg hook'\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 'with no hook' '\ndiff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh\nindex 419f055721d..743fbe1fe46 100755\n--- a/t/t9115-git-svn-dcommit-funky-renames.sh\n+++ b/t/t9115-git-svn-dcommit-funky-renames.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn dcommit can commit renames of files with ugly names'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'load repository with strange names' '\ndiff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh\nindex 79c26ed69c1..09606f1b3cf 100755\n--- a/t/t9146-git-svn-empty-dirs.sh\n+++ b/t/t9146-git-svn-empty-dirs.sh\n@@ -4,7 +4,6 @@\n \n test_description='git svn creates empty directories'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'initialize repo' '\ndiff --git a/t/t9160-git-svn-preserve-empty-dirs.sh b/t/t9160-git-svn-preserve-empty-dirs.sh\nindex 9cf7a1427ab..36c6b1a12ff 100755\n--- a/t/t9160-git-svn-preserve-empty-dirs.sh\n+++ b/t/t9160-git-svn-preserve-empty-dirs.sh\n@@ -9,7 +9,6 @@ This test uses git to clone a Subversion repository that contains empty\n directories, and checks that corresponding directories are created in the\n local Git repository with placeholder files.'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n GIT_REPO=git-svn-repo\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466853","messageId":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com","subject":"[PATCH v2 00/15] leak fixes: use existing constructors & other trivia","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:36Z","receivedAt":"2022-11-08T18:18:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a bag of simple leak fixes, mostly one-line additions where we\nuse existing destructors. See [1] for v1.\n\nChanges since v1:\n\n * Rebased on pw/rebase-no-reflog-action, per [2].\n * It'll merge just fine without pw/rebase-no-reflog-action, but some\n of the tests marked as leak-free will still leak then. So, in\n practice they can be staged independently, as long as\n pw/rebase-no-reflog-action is merged down first.\n * The 10/15 here has an updated explanation about why we were leaking\n that value. see also[3].\n\nAs noted in my [4] some of the rebase/sequencer leaks addressed here\nshould have a more thorough fix to address the root cause of some of\nthem, which is also something pw/rebase-no-reflog-action is running\ninto (and why it's introducing a new leak, while fixing another one).\n\nBut per [4] I think the right thing to do is to merge both that topic\n& this down first, and then we can follow-up with those fixes (which I\nhave staged locally already). Getting the extra leak coverage this\nseries & pw/rebase-no-reflog-action bring us will help in the\nmeantime.\n\n1. https://lore.kernel.org/git/cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com/\n2. https://lore.kernel.org/git/8eec228d-d392-523d-2415-149b946f642e@dunelm.org.uk/\n3. https://lore.kernel.org/git/221108.86zgd1qxac.gmgdl@evledraar.gmail.com/\n4. https://lore.kernel.org/git/221108.864jv9sc9r.gmgdl@evledraar.gmail.com/\n\nÆvar Arnfjörð Bjarmason (15):\n tests: mark tests as passing with SANITIZE=leak\n {reset,merge}: call discard_index() before returning\n commit: discard partial cache before (re-)reading it\n read-cache.c: clear and free \"sparse_checkout_patterns\"\n dir.c: free \"ident\" and \"exclude_per_dir\" in \"struct untracked_cache\"\n built-ins & libs & helpers: add/move destructors, fix leaks\n unpack-file: fix ancient leak in create_temp_file()\n revision API: call graph_clear() in release_revisions()\n ls-files: fix a --with-tree memory leak\n sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()\n connected.c: free the \"struct packed_git\"\n rebase: don't leak on \"--abort\"\n cherry-pick: free \"struct replay_opts\" members\n revert: fix parse_options_concat() leak\n built-ins: use free() not UNLEAK() if trivial, rm dead code\n\n builtin/add.c | 2 +-\n builtin/bugreport.c | 9 +++--\n builtin/checkout.c | 2 ++\n builtin/commit.c | 13 +++++---\n builtin/config.c | 42 +++++++++++-------------\n builtin/diff.c | 2 +-\n builtin/ls-files.c | 1 +\n builtin/merge.c | 1 +\n builtin/rebase.c | 4 +++\n builtin/repack.c | 2 +-\n builtin/reset.c | 2 ++\n builtin/rev-parse.c | 1 +\n builtin/revert.c | 4 +++\n builtin/stash.c | 2 ++\n builtin/unpack-file.c | 1 +\n builtin/worktree.c | 7 ++--\n connected.c | 6 +++-\n dir.c | 10 ++++--\n dir.h | 1 +\n read-cache.c | 5 +++\n ref-filter.c | 1 +\n revision.c | 1 +\n sequencer.c | 1 +\n t/helper/test-fake-ssh.c | 1 +\n t/t0068-for-each-repo.sh | 1 +\n t/t0070-fundamental.sh | 1 +\n t/t1011-read-tree-sparse-checkout.sh | 1 +\n t/t1022-read-tree-partial-clone.sh | 2 +-\n t/t1404-update-ref-errors.sh | 2 ++\n t/t1409-avoid-packing-refs.sh | 1 +\n t/t1413-reflog-detach.sh | 1 +\n t/t1501-work-tree.sh | 2 ++\n t/t2012-checkout-last.sh | 1 +\n t/t2018-checkout-branch.sh | 1 +\n t/t2025-checkout-no-overlay.sh | 1 +\n t/t3009-ls-files-others-nonsubmodule.sh | 1 +\n t/t3010-ls-files-killed-modified.sh | 2 ++\n t/t3050-subprojects-fetch.sh | 1 +\n t/t3060-ls-files-with-tree.sh | 2 ++\n t/t3409-rebase-environ.sh | 1 +\n t/t3413-rebase-hook.sh | 1 +\n t/t3428-rebase-signoff.sh | 1 +\n t/t3429-rebase-edit-todo.sh | 1 +\n t/t3433-rebase-across-mode-change.sh | 1 +\n t/t4015-diff-whitespace.sh | 4 +--\n t/t4045-diff-relative.sh | 2 ++\n t/t4052-stat-output.sh | 1 +\n t/t4053-diff-no-index.sh | 1 +\n t/t4067-diff-partial-clone.sh | 1 +\n t/t4111-apply-subdir.sh | 1 +\n t/t4135-apply-weird-filenames.sh | 1 +\n t/t4213-log-tabexpand.sh | 1 +\n t/t5544-pack-objects-hook.sh | 2 ++\n t/t5554-noop-fetch-negotiator.sh | 2 ++\n t/t5610-clone-detached.sh | 1 +\n t/t5611-clone-config.sh | 1 +\n t/t5614-clone-submodules-shallow.sh | 1 +\n t/t5617-clone-submodules-remote.sh | 1 +\n t/t5618-alternate-refs.sh | 2 ++\n t/t6060-merge-index.sh | 2 ++\n t/t6301-for-each-ref-errors.sh | 1 +\n t/t6401-merge-criss-cross.sh | 2 ++\n t/t6406-merge-attr.sh | 1 +\n t/t6407-merge-binary.sh | 1 +\n t/t6415-merge-dir-to-symlink.sh | 1 +\n t/t6435-merge-sparse.sh | 1 +\n t/t7103-reset-bare.sh | 2 +-\n t/t7504-commit-msg-hook.sh | 1 +\n t/t7517-per-repo-email.sh | 1 +\n t/t7520-ignored-hook-warning.sh | 1 +\n t/t7605-merge-resolve.sh | 1 +\n t/t7614-merge-signoff.sh | 1 +\n t/t9003-help-autocorrect.sh | 2 ++\n t/t9115-git-svn-dcommit-funky-renames.sh | 1 -\n t/t9146-git-svn-empty-dirs.sh | 1 -\n t/t9148-git-svn-propset.sh | 1 -\n t/t9160-git-svn-preserve-empty-dirs.sh | 1 -\n 77 files changed, 142 insertions(+), 48 deletions(-)\n\nRange-diff against v1:\n 1: 7fe724599a7 ! 1: 58a02e6bb4e tests: mark tests as passing with SANITIZE=leak\n @@ Commit message\n 66eede4a37c (prepare_repo_settings(): plug leak of config values,\n 2022-09-08)\n \n + - t2012-checkout-last.sh, t7504-commit-msg-hook.sh,\n + t91{15,46,60}-git-svn-*.sh: The in-flight \"pw/rebase-no-reflog-action\"\n + series, upon which this is based:\n + https://lore.kernel.org/git/pull.1405.git.1667575142.gitgitgadget@gmail.com/\n +\n Let's mark all of these as passing with\n \"TEST_PASSES_SANITIZE_LEAK=true\", to have it regression tested,\n including as part of the \"linux-leaks\" CI job.\n @@ t/t1022-read-tree-partial-clone.sh\n \n test_expect_success 'read-tree in partial clone prefetches in one batch' '\n \n + ## t/t2012-checkout-last.sh ##\n +@@ t/t2012-checkout-last.sh: test_description='checkout can switch to last branch and merge base'\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 +\n ## t/t4015-diff-whitespace.sh ##\n @@ t/t4015-diff-whitespace.sh: test_expect_success 'no effect on diff from --color-moved with --word-diff' '\n \ttest_cmp expect actual\n @@ t/t7103-reset-bare.sh: test_expect_success '\"mixed\" reset is not allowed in bare\n \tgit reset --soft HEAD^ &&\n \tgit show --pretty=format:%s >out &&\n \techo one >expect &&\n +\n + ## t/t7504-commit-msg-hook.sh ##\n +@@ t/t7504-commit-msg-hook.sh: test_description='commit-msg hook'\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 'with no hook' '\n +\n + ## t/t9115-git-svn-dcommit-funky-renames.sh ##\n +@@\n + \n + test_description='git svn dcommit can commit renames of files with ugly names'\n + \n +-TEST_FAILS_SANITIZE_LEAK=true\n + . ./lib-git-svn.sh\n + \n + test_expect_success 'load repository with strange names' '\n +\n + ## t/t9146-git-svn-empty-dirs.sh ##\n +@@\n + \n + test_description='git svn creates empty directories'\n + \n +-TEST_FAILS_SANITIZE_LEAK=true\n + . ./lib-git-svn.sh\n + \n + test_expect_success 'initialize repo' '\n +\n + ## t/t9160-git-svn-preserve-empty-dirs.sh ##\n +@@ t/t9160-git-svn-preserve-empty-dirs.sh: This test uses git to clone a Subversion repository that contains empty\n + directories, and checks that corresponding directories are created in the\n + local Git repository with placeholder files.'\n + \n +-TEST_FAILS_SANITIZE_LEAK=true\n + . ./lib-git-svn.sh\n + \n + GIT_REPO=git-svn-repo\n 2: 819fde89a24 = 2: e9962ad790e {reset,merge}: call discard_index() before returning\n 3: c1003454939 = 3: 8e76a29cce7 commit: discard partial cache before (re-)reading it\n 4: 68a390619b9 = 4: 24cefde07f0 read-cache.c: clear and free \"sparse_checkout_patterns\"\n 5: 29123e62391 = 5: f4fc08a9bc8 dir.c: free \"ident\" and \"exclude_per_dir\" in \"struct untracked_cache\"\n 6: 009c41d2e91 ! 6: 698a335b1a3 built-ins & libs & helpers: add/move destructors, fix leaks\n @@ builtin/checkout.c: static void die_if_some_operation_in_progress(void)\n \n ## builtin/rebase.c ##\n @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)\n - \tstrbuf_release(&buf);\n \tstrbuf_release(&revisions);\n + \tfree(options.reflog_action);\n \tfree(options.head_name);\n +\tstrvec_clear(&options.git_am_opts);\n \tfree(options.gpg_sign_opt);\n @@ t/helper/test-fake-ssh.c: int cmd_main(int argc, const char **argv)\n \t\tfprintf(f, \"%s%s\", i > 0 ? \" \" : \"\", i > 0 ? argv[i] : \"ssh:\");\n \tfprintf(f, \"\\n\");\n \n + ## t/t3409-rebase-environ.sh ##\n +@@\n + \n + test_description='git rebase interactive environment'\n + \n ++TEST_PASSES_SANITIZE_LEAK=true\n + . ./test-lib.sh\n + \n + test_expect_success 'setup' '\n +\n ## t/t3428-rebase-signoff.sh ##\n @@ t/t3428-rebase-signoff.sh: test_description='git rebase --signoff\n This test runs git rebase --signoff and make sure that it works.\n @@ t/t3428-rebase-signoff.sh: test_description='git rebase --signoff\n . ./test-lib.sh\n \n # A simple file to commit\n +\n + ## t/t3433-rebase-across-mode-change.sh ##\n +@@\n + \n + test_description='git rebase across mode change'\n + \n ++TEST_PASSES_SANITIZE_LEAK=true\n + . ./test-lib.sh\n + \n + test_expect_success 'setup' '\n 7: 38e7c863dd8 = 7: a2cc6f10d0d unpack-file: fix ancient leak in create_temp_file()\n 8: 2944aaa3ddc ! 8: 45aa6e24c66 revision API: call graph_clear() in release_revisions()\n @@ revision.c: void release_revisions(struct rev_info *revs)\n \tdiff_free(&revs->pruning);\n \treflog_walk_info_release(revs->reflog_info);\n \n + ## t/t3413-rebase-hook.sh ##\n +@@ t/t3413-rebase-hook.sh: test_description='git rebase with its hook(s)'\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 +\n ## t/t4052-stat-output.sh ##\n @@ t/t4052-stat-output.sh: test_description='test --stat output of various commands'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n 9: d592439760e = 9: 46809d9be80 ls-files: fix a --with-tree memory leak\n10: 9c70bfa334e ! 10: 243ab74120b sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()\n @@ Commit message\n \n When \"read_strategy_opts()\" is called we may have populated the\n \"opts->strategy\" before, so we'll need to free() it to avoid leaking\n - memory. Along with the preceding commit this change various\n - rebase-related tests pass.\n + memory.\n +\n + We populate it before because we cal get_replay_opts() from within\n + \"rebase.c\" with an already populated \"opts\", which we then copy. Then\n + if we're doing a \"rebase -i\" the sequencer API itself will promptly\n + clobber our alloc'd version of it with its own.\n +\n + If this code is changed to do, instead of the added free() here a:\n +\n + if (opts->strategy)\n + opts->strategy = xstrdup(\"another leak\");\n +\n + We get a couple of stacktraces from -fsanitize=leak showing how we\n + ended up clobbering the already allocated value, i.e.:\n +\n + Direct leak of 6 byte(s) in 1 object(s) allocated from:\n + #0 0x7f2e8cd45545 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:75\n + #1 0x7f2e8cb0fcaa in __GI___strdup string/strdup.c:42\n + #2 0x6c4778 in xstrdup wrapper.c:39\n + #3 0x66bcb8 in read_strategy_opts sequencer.c:2902\n + #4 0x66bf7b in read_populate_opts sequencer.c:2969\n + #5 0x6723f9 in sequencer_continue sequencer.c:5063\n + #6 0x4a4f74 in run_sequencer_rebase builtin/rebase.c:348\n + #7 0x4a64c8 in run_specific_rebase builtin/rebase.c:753\n + #8 0x4a9b8b in cmd_rebase builtin/rebase.c:1824\n + #9 0x407a32 in run_builtin git.c:466\n + #10 0x407e0a in handle_builtin git.c:721\n + #11 0x40803d in run_argv git.c:788\n + #12 0x40850f in cmd_main git.c:923\n + #13 0x4eee79 in main common-main.c:57\n + #14 0x7f2e8ca9f209 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58\n + #15 0x7f2e8ca9f2bb in __libc_start_main_impl ../csu/libc-start.c:389\n + #16 0x405fd0 in _start (git+0x405fd0)\n +\n + Direct leak of 4 byte(s) in 1 object(s) allocated from:\n + #0 0x7f2e8cd45545 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:75\n + #1 0x7f2e8cb0fcaa in __GI___strdup string/strdup.c:42\n + #2 0x6c4778 in xstrdup wrapper.c:39\n + #3 0x4a3c31 in xstrdup_or_null git-compat-util.h:1169\n + #4 0x4a447a in get_replay_opts builtin/rebase.c:163\n + #5 0x4a4f5b in run_sequencer_rebase builtin/rebase.c:346\n + #6 0x4a64c8 in run_specific_rebase builtin/rebase.c:753\n + #7 0x4a9b8b in cmd_rebase builtin/rebase.c:1824\n + #8 0x407a32 in run_builtin git.c:466\n + #9 0x407e0a in handle_builtin git.c:721\n + #10 0x40803d in run_argv git.c:788\n + #11 0x40850f in cmd_main git.c:923\n + #12 0x4eee79 in main common-main.c:57\n + #13 0x7f2e8ca9f209 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58\n + #14 0x7f2e8ca9f2bb in __libc_start_main_impl ../csu/libc-start.c:389\n + #15 0x405fd0 in _start (git+0x405fd0)\n +\n + This can be seen in e.g. the 4th test of\n + \"t3404-rebase-interactive.sh\".\n +\n + In the larger picture the ownership of the \"struct replay_opts\" is\n + quite a mess, e.g. in this case rebase.c's static \"get_replay_opts()\"\n + function partially creates it, but nothing in rebase.c will free()\n + it. The structure is \"mostly owned\" by the sequencer API, but it also\n + expects to get these partially populated versions of it.\n +\n + It would be better to have rebase keep track of what it allocated, and\n + free() that, and to pass that as a \"const\" to the sequencer API, which\n + would copy what it needs to its own version, and to free() that.\n +\n + But doing so is a much larger change, and however messy the ownership\n + boundary is here is consistent with what we're doing already, so let's\n + just free() this to fix the leak.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n11: c7722f68ae2 = 11: 15f0837697d connected.c: free the \"struct packed_git\"\n12: 7429151b5c4 < -: ----------- sequencer.c: fix a pick_commits() leak\n13: fda9914b558 = 12: f6d76250174 rebase: don't leak on \"--abort\"\n14: 02e1dddf149 < -: ----------- sequencer.c: fix sequencer_continue() leak\n15: c9f51abf0e2 = 13: 10a477c7730 cherry-pick: free \"struct replay_opts\" members\n16: f7d39020a7c = 14: 614c8fc1aef revert: fix parse_options_concat() leak\n17: 33a5753cc3a = 15: f3e6a030394 built-ins: use free() not UNLEAK() if trivial, rm dead code\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466854","messageId":"patch-v2-02.15-e9962ad790e-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 02/15] {reset,merge}: call discard_index() before returning","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:38Z","receivedAt":"2022-11-08T18:18:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"These two built-ins both deal with the index, but weren't discarding\nit. In subsequent commits we'll add more free()-ing to discard_index()\nthat we've missed, but let's first call the existing function.\n\nWe can doubtless add discard_index() (or its alias discard_cache()) to\na lot more places, but let's just add it here for now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/merge.c | 1 +\n builtin/reset.c | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 5900b81729d..a6698adbfd3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1794,5 +1794,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t}\n \tstrbuf_release(&buf);\n \tfree(branch_to_free);\n+\tdiscard_index(&the_index);\n \treturn ret;\n }\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fdce6f8c856..69f18a248ee 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -481,5 +481,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (!pathspec.nr)\n \t\tremove_branch_state(the_repository, 0);\n \n+\tdiscard_index(&the_index);\n+\n \treturn update_ref_status;\n }\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466855","messageId":"patch-v2-04.15-24cefde07f0-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 04/15] read-cache.c: clear and free \"sparse_checkout_patterns\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:40Z","receivedAt":"2022-11-08T18:18:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"sparse_checkout_patterns\" member was added to the \"struct\nindex_state\" in 836e25c51b2 (sparse-checkout: hold pattern list in\nindex, 2021-03-30), but wasn't added to discard_index(). Let's do\nthat.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n read-cache.c | 5 +++++\n t/t1011-read-tree-sparse-checkout.sh | 1 +\n t/t2018-checkout-branch.sh | 1 +\n t/t6435-merge-sparse.sh | 1 +\n 4 files changed, 8 insertions(+)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 32024029274..7c6477487a7 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2558,6 +2558,11 @@ int discard_index(struct index_state *istate)\n \tfree_untracked_cache(istate->untracked);\n \tistate->untracked = NULL;\n \n+\tif (istate->sparse_checkout_patterns) {\n+\t\tclear_pattern_list(istate->sparse_checkout_patterns);\n+\t\tFREE_AND_NULL(istate->sparse_checkout_patterns);\n+\t}\n+\n \tif (istate->ce_mem_pool) {\n \t\tmem_pool_discard(istate->ce_mem_pool, should_validate_cache_entries());\n \t\tFREE_AND_NULL(istate->ce_mem_pool);\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 742f0fa909f..595b24c0adb 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -12,6 +12,7 @@ test_description='sparse checkout tests\n '\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-read-tree.sh\n \ndiff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh\nindex 771c3c3c50e..8581ad34379 100755\n--- a/t/t2018-checkout-branch.sh\n+++ b/t/t2018-checkout-branch.sh\n@@ -3,6 +3,7 @@\n test_description='checkout'\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Arguments: [!] []\ndiff --git a/t/t6435-merge-sparse.sh b/t/t6435-merge-sparse.sh\nindex fde4aa3cd1a..78628fb248a 100755\n--- a/t/t6435-merge-sparse.sh\n+++ b/t/t6435-merge-sparse.sh\n@@ -3,6 +3,7 @@\n test_description='merge with sparse files'\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # test_file $filename $content\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466856","messageId":"patch-v2-03.15-8e76a29cce7-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 03/15] commit: discard partial cache before (re-)reading it","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:39Z","receivedAt":"2022-11-08T18:18:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The read_cache() in prepare_to_commit() would end up clobbering the\npointer we had for a previously populated \"the_index.cache_tree\" in\nthe very common case of \"git commit\" stressed by e.g. the tests being\nchanged here.\n\nWe'd populate \"the_index.cache_tree\" by calling\n\"update_main_cache_tree\" in prepare_index(), but would not end up with\na \"fully prepared\" index. What constitutes an existing index is\nclearly overly fuzzy, here we'll check \"active_nr\" (aka\n\"the_index.cache_nr\"), but our \"the_index.cache_tree\" might have been\nmalloc()'d already.\n\nThus the code added in 11c8a74a64a (commit: write cache-tree data when\nwriting index anyway, 2011-12-06) would end up allocating the\n\"cache_tree\", and would interact here with code added in\n7168624c353 (Do not generate full commit log message if it is not\ngoing to be used, 2007-11-28). The result was a very common memory\nleak.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/commit.c | 7 +++++--\n t/t0068-for-each-repo.sh | 1 +\n t/t0070-fundamental.sh | 1 +\n t/t1404-update-ref-errors.sh | 2 ++\n t/t1409-avoid-packing-refs.sh | 1 +\n t/t1413-reflog-detach.sh | 1 +\n t/t1501-work-tree.sh | 2 ++\n t/t2025-checkout-no-overlay.sh | 1 +\n t/t3009-ls-files-others-nonsubmodule.sh | 1 +\n t/t3010-ls-files-killed-modified.sh | 2 ++\n t/t4045-diff-relative.sh | 2 ++\n t/t4111-apply-subdir.sh | 1 +\n t/t4135-apply-weird-filenames.sh | 1 +\n t/t4213-log-tabexpand.sh | 1 +\n t/t5618-alternate-refs.sh | 2 ++\n t/t6301-for-each-ref-errors.sh | 1 +\n t/t7520-ignored-hook-warning.sh | 1 +\n t/t7614-merge-signoff.sh | 1 +\n t/t9003-help-autocorrect.sh | 2 ++\n 19 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e22bdf23f5f..c291199b704 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -987,8 +987,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tstruct object_id oid;\n \t\tconst char *parent = \"HEAD\";\n \n-\t\tif (!active_nr && read_cache() < 0)\n-\t\t\tdie(_(\"Cannot read index\"));\n+\t\tif (!active_nr) {\n+\t\t\tdiscard_cache();\n+\t\t\tif (read_cache() < 0)\n+\t\t\t\tdie(_(\"Cannot read index\"));\n+\t\t}\n \n \t\tif (amend)\n \t\t\tparent = \"HEAD^1\";\ndiff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh\nindex 4675e852517..9f63f612425 100755\n--- a/t/t0068-for-each-repo.sh\n+++ b/t/t0068-for-each-repo.sh\n@@ -2,6 +2,7 @@\n \n test_description='git for-each-repo builtin'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'run based on configured value' '\ndiff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh\nindex 8d59905ef09..574de341980 100755\n--- a/t/t0070-fundamental.sh\n+++ b/t/t0070-fundamental.sh\n@@ -6,6 +6,7 @@ test_description='check that the most basic functions work\n Verify wrappers and compatibility functions.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'character classes (isspace, isalpha etc.)' '\ndiff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh\nindex 13c2b43bbae..b5606d93b52 100755\n--- a/t/t1404-update-ref-errors.sh\n+++ b/t/t1404-update-ref-errors.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='Test git update-ref error handling'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Create some references, perhaps run pack-refs --all, then try to\ndiff --git a/t/t1409-avoid-packing-refs.sh b/t/t1409-avoid-packing-refs.sh\nindex be12fb63506..f23c0152a85 100755\n--- a/t/t1409-avoid-packing-refs.sh\n+++ b/t/t1409-avoid-packing-refs.sh\n@@ -2,6 +2,7 @@\n \n test_description='avoid rewriting packed-refs unnecessarily'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Add an identifying mark to the packed-refs file header line. This\ndiff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh\nindex 934688a1ee8..d2a4822d46f 100755\n--- a/t/t1413-reflog-detach.sh\n+++ b/t/t1413-reflog-detach.sh\n@@ -4,6 +4,7 @@ test_description='Test reflog interaction with detached HEAD'\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 reset_state () {\ndiff --git a/t/t1501-work-tree.sh b/t/t1501-work-tree.sh\nindex b75558040ff..ae6528aecea 100755\n--- a/t/t1501-work-tree.sh\n+++ b/t/t1501-work-tree.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test separate work tree'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t2025-checkout-no-overlay.sh b/t/t2025-checkout-no-overlay.sh\nindex 8f13341cf8e..3832c3de813 100755\n--- a/t/t2025-checkout-no-overlay.sh\n+++ b/t/t2025-checkout-no-overlay.sh\n@@ -2,6 +2,7 @@\n \n test_description='checkout --no-overlay -- '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nindex 963f3462b75..14218b34243 100755\n--- a/t/t3009-ls-files-others-nonsubmodule.sh\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -18,6 +18,7 @@ This test runs git ls-files --others with the following working tree:\n git repository with a commit and an untracked file\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup: directories' '\ndiff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh\nindex 580e158f991..054178703d7 100755\n--- a/t/t3010-ls-files-killed-modified.sh\n+++ b/t/t3010-ls-files-killed-modified.sh\n@@ -41,6 +41,8 @@ Also for modification test, the cache and working tree have:\n We should report path0, path1, path2/file2, path3/file3, path7 and path8\n modified without reporting path9 and path10. submod1 is also modified.\n '\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'git update-index --add to add various paths.' '\ndiff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh\nindex fab351b48a1..198dfc91906 100755\n--- a/t/t4045-diff-relative.sh\n+++ b/t/t4045-diff-relative.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='diff --relative tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4111-apply-subdir.sh b/t/t4111-apply-subdir.sh\nindex 1618a6dbc7c..e9a87d761d1 100755\n--- a/t/t4111-apply-subdir.sh\n+++ b/t/t4111-apply-subdir.sh\n@@ -2,6 +2,7 @@\n \n test_description='patching from inconvenient places'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4135-apply-weird-filenames.sh b/t/t4135-apply-weird-filenames.sh\nindex 6bc3fb97a75..d3502c6fddf 100755\n--- a/t/t4135-apply-weird-filenames.sh\n+++ b/t/t4135-apply-weird-filenames.sh\n@@ -2,6 +2,7 @@\n \n test_description='git apply with weird postimage filenames'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh\nindex 53a4af32449..590fce95e90 100755\n--- a/t/t4213-log-tabexpand.sh\n+++ b/t/t4213-log-tabexpand.sh\n@@ -2,6 +2,7 @@\n \n test_description='log/show --expand-tabs'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n HT=\"\t\"\ndiff --git a/t/t5618-alternate-refs.sh b/t/t5618-alternate-refs.sh\nindex 3353216f09e..f905db0a3fd 100755\n--- a/t/t5618-alternate-refs.sh\n+++ b/t/t5618-alternate-refs.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test handling of --alternate-refs traversal'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Avoid test_commit because we want a specific and known set of refs:\ndiff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh\nindex 40edf9dab53..bfda1f46ad2 100755\n--- a/t/t6301-for-each-ref-errors.sh\n+++ b/t/t6301-for-each-ref-errors.sh\n@@ -2,6 +2,7 @@\n \n test_description='for-each-ref errors for broken refs'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n ZEROS=$ZERO_OID\ndiff --git a/t/t7520-ignored-hook-warning.sh b/t/t7520-ignored-hook-warning.sh\nindex dc57526e6f1..184b2589893 100755\n--- a/t/t7520-ignored-hook-warning.sh\n+++ b/t/t7520-ignored-hook-warning.sh\n@@ -2,6 +2,7 @@\n \n test_description='ignored hook warning'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\nindex fee258d4f0c..cf96a35e8e7 100755\n--- a/t/t7614-merge-signoff.sh\n+++ b/t/t7614-merge-signoff.sh\n@@ -8,6 +8,7 @@ This test runs git merge --signoff and makes sure that it works.\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 test files\ndiff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh\nindex f00deaf3815..4b9cb4c942f 100755\n--- a/t/t9003-help-autocorrect.sh\n+++ b/t/t9003-help-autocorrect.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='help.autocorrect finding a match'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466857","messageId":"patch-v2-05.15-f4fc08a9bc8-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 05/15] dir.c: free \"ident\" and \"exclude_per_dir\" in \"struct untracked_cache\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:41Z","receivedAt":"2022-11-08T18:18:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When the \"ident\" member of the structure was added in\n1e8fef609e7 (untracked cache: guard and disable on system changes,\n2015-03-08) this function wasn't updated to free it. Let's do so.\n\nLet's also free the \"exclude_per_dir\" memory we've been leaking\nsince[1], while making sure not to free() the constant \".gitignore\"\nstring we add by default[2].\n\nAs we now have three struct members we're freeing let's change\nfree_untracked_cache() to return early if \"uc\" isn't defined. We won't\nhand it to free() now, but that was just for convenience, once we're\ndealing with >=2 struct members this pattern is more convenient.\n\n1. f9e6c649589 (untracked cache: load from UNTR index extension,\n 2015-03-08)\n2. 039bc64e886 (core.excludesfile clean-up, 2007-11-14)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n dir.c | 10 +++++++---\n dir.h | 1 +\n 2 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex d604d1bab98..fbdb24fc819 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -3581,8 +3581,12 @@ static void free_untracked(struct untracked_cache_dir *ucd)\n \n void free_untracked_cache(struct untracked_cache *uc)\n {\n-\tif (uc)\n-\t\tfree_untracked(uc->root);\n+\tif (!uc)\n+\t\treturn;\n+\n+\tfree(uc->exclude_per_dir_to_free);\n+\tstrbuf_release(&uc->ident);\n+\tfree_untracked(uc->root);\n \tfree(uc);\n }\n \n@@ -3739,7 +3743,7 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n \t\t next + offset + hashsz);\n \tuc->dir_flags = get_be32(next + ouc_offset(dir_flags));\n \texclude_per_dir = (const char *)next + exclude_per_dir_offset;\n-\tuc->exclude_per_dir = xstrdup(exclude_per_dir);\n+\tuc->exclude_per_dir = uc->exclude_per_dir_to_free = xstrdup(exclude_per_dir);\n \t/* NUL after exclude_per_dir is covered by sizeof(*ouc) */\n \tnext += exclude_per_dir_offset + strlen(exclude_per_dir) + 1;\n \tif (next >= end)\ndiff --git a/dir.h b/dir.h\nindex 674747d93af..8acfc044181 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -188,6 +188,7 @@ struct untracked_cache {\n \tstruct oid_stat ss_info_exclude;\n \tstruct oid_stat ss_excludes_file;\n \tconst char *exclude_per_dir;\n+\tchar *exclude_per_dir_to_free;\n \tstruct strbuf ident;\n \t/*\n \t * dir_struct#flags must match dir_flags or the untracked\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466858","messageId":"patch-v2-06.15-698a335b1a3-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 06/15] built-ins & libs & helpers: add/move destructors, fix leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:42Z","receivedAt":"2022-11-08T18:18:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix various leaks in built-ins, libraries and a test helper here we\nwere missing a call to strbuf_release(), string_list_clear() etc, or\nwere calling them after a potential \"return\".\n\nComments on individual changes:\n\n- builtin/checkout.c: Fix a memory leak that was introduced in [1]. A\n sibling leak introduced in [2] was recently fixed in [3]. As with [3]\n we should be using the wt_status_state_free_buffers() API introduced\n in [4].\n\n- builtin/repack.c: Fix a leak that's been here since this use of\n \"strbuf_release()\" was added in a1bbc6c0176 (repack: rewrite the shell\n script in C, 2013-09-15). We don't use the variable for anything\n except this loop, so we can instead free it right afterwards.\n\n- builtin/rev-parse: Fix a leak that's been here since this code was\n added in 21d47835386 (Add a parseopt mode to git-rev-parse to bring\n parse-options to shell scripts., 2007-11-04).\n\n- builtin/stash.c: Fix a couple of leaks that have been here since\n this code was added in d4788af875c (stash: convert create to builtin,\n 2019-02-25), we strbuf_release()'d only some of the \"struct strbuf\" we\n allocated earlier in the function, let's release all of them.\n\n- ref-filter.c: Fix a leak in 482c1191869 (gpg-interface: improve\n interface for parsing tags, 2021-02-11), we don't use the \"payload\"\n variable that we ask parse_signature() to populate for us, so let's\n free it.\n\n- t/helper/test-fake-ssh.c: Fix a leak that's been here since this\n code was added in 3064d5a38c7 (mingw: fix t5601-clone.sh,\n 2016-01-27). Let's free the \"struct strbuf\" as soon as we don't need\n it anymore.\n\n1. c45f0f525de (switch: reject if some operation is in progress,\n 2019-03-29)\n2. 2708ce62d21 (branch: sort detached HEAD based on a flag,\n 2021-01-07)\n3. abcac2e19fa (ref-filter.c: fix a leak in get_head_description,\n 2022-09-25)\n4. 962dd7ebc3e (wt-status: introduce wt_status_state_free_buffers(),\n 2020-09-27).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/checkout.c | 2 ++\n builtin/rebase.c | 3 +++\n builtin/repack.c | 2 +-\n builtin/rev-parse.c | 1 +\n builtin/stash.c | 2 ++\n ref-filter.c | 1 +\n t/helper/test-fake-ssh.c | 1 +\n t/t3409-rebase-environ.sh | 1 +\n t/t3428-rebase-signoff.sh | 1 +\n t/t3433-rebase-across-mode-change.sh | 1 +\n 10 files changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2a132392fbe..659dd5c4309 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1470,6 +1470,8 @@ static void die_if_some_operation_in_progress(void)\n \t\t \"or \\\"git worktree add\\\".\"));\n \tif (state.bisect_in_progress)\n \t\twarning(_(\"you are switching branch while bisecting\"));\n+\n+\twt_status_state_free_buffers(&state);\n }\n \n static int checkout_branch(struct checkout_opts *opts,\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 4d6839a5785..6a831320313 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1828,10 +1828,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&revisions);\n \tfree(options.reflog_action);\n \tfree(options.head_name);\n+\tstrvec_clear(&options.git_am_opts);\n \tfree(options.gpg_sign_opt);\n \tfree(options.cmd);\n \tfree(options.strategy);\n \tstrbuf_release(&options.git_format_patch_opt);\n \tfree(squash_onto_name);\n+\tstring_list_clear(&exec, 0);\n+\tstring_list_clear(&strategy_options, 0);\n \treturn !!ret;\n }\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 10e23f9ee1f..fb3ac197426 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -956,6 +956,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\titem = string_list_append(&names, line.buf);\n \t\titem->util = populate_pack_exts(item->string);\n \t}\n+\tstrbuf_release(&line);\n \tfclose(out);\n \tret = finish_command(&cmd);\n \tif (ret)\n@@ -1124,7 +1125,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tstring_list_clear(&existing_nonkept_packs, 0);\n \tstring_list_clear(&existing_kept_packs, 0);\n \tclear_pack_geometry(geometry);\n-\tstrbuf_release(&line);\n \n \treturn 0;\n }\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 8f61050bde8..e0244d63de6 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -530,6 +530,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)\n \tstrbuf_addstr(&parsed, \" --\");\n \tsq_quote_argv(&parsed, argv);\n \tputs(parsed.buf);\n+\tstrbuf_release(&parsed);\n \treturn 0;\n }\n \ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex bb5485b4095..8a64d564a19 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1686,8 +1686,10 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \t}\n \n done:\n+\tstrbuf_release(&patch);\n \tfree_stash_info(&info);\n \tstrbuf_release(&stash_msg_buf);\n+\tstrbuf_release(&untracked_files);\n \treturn ret;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 914908fac52..b40b76c3806 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1358,6 +1358,7 @@ static void find_subpos(const char *buf,\n \n \t/* parse signature first; we might not even have a subject line */\n \tparse_signature(buf, end - buf, &payload, &signature);\n+\tstrbuf_release(&payload);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\ndiff --git a/t/helper/test-fake-ssh.c b/t/helper/test-fake-ssh.c\nindex 12beee99ad2..d5e511633ab 100644\n--- a/t/helper/test-fake-ssh.c\n+++ b/t/helper/test-fake-ssh.c\n@@ -17,6 +17,7 @@ int cmd_main(int argc, const char **argv)\n \tf = fopen(buf.buf, \"w\");\n \tif (!f)\n \t\tdie(\"Could not write to %s\", buf.buf);\n+\tstrbuf_release(&buf);\n \tfor (i = 0; i < argc; i++)\n \t\tfprintf(f, \"%s%s\", i > 0 ? \" \" : \"\", i > 0 ? argv[i] : \"ssh:\");\n \tfprintf(f, \"\\n\");\ndiff --git a/t/t3409-rebase-environ.sh b/t/t3409-rebase-environ.sh\nindex 83ffb39d9ff..acaf5558dbe 100755\n--- a/t/t3409-rebase-environ.sh\n+++ b/t/t3409-rebase-environ.sh\n@@ -2,6 +2,7 @@\n \n test_description='git rebase interactive environment'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\nindex f6993b7e14d..e1b1e947647 100755\n--- a/t/t3428-rebase-signoff.sh\n+++ b/t/t3428-rebase-signoff.sh\n@@ -5,6 +5,7 @@ test_description='git rebase --signoff\n This test runs git rebase --signoff and make sure that it works.\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # A simple file to commit\ndiff --git a/t/t3433-rebase-across-mode-change.sh b/t/t3433-rebase-across-mode-change.sh\nindex 05df964670f..c8172b08522 100755\n--- a/t/t3433-rebase-across-mode-change.sh\n+++ b/t/t3433-rebase-across-mode-change.sh\n@@ -2,6 +2,7 @@\n \n test_description='git rebase across mode change'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466859","messageId":"patch-v2-08.15-45aa6e24c66-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 08/15] revision API: call graph_clear() in release_revisions()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:44Z","receivedAt":"2022-11-08T18:18:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Call graph_clear() in release_revisions(), this will free memory\nallocated by e.g. this command, which will now run without memory\nleaks:\n\n\tgit -P log -1 --graph --no-graph --graph\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n revision.c | 1 +\n t/t3413-rebase-hook.sh | 1 +\n t/t4052-stat-output.sh | 1 +\n 3 files changed, 3 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex 0760e78936e..650dd17599a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3020,6 +3020,7 @@ void release_revisions(struct rev_info *revs)\n \tdate_mode_release(&revs->date_mode);\n \trelease_revisions_mailmap(revs->mailmap);\n \tfree_grep_patterns(&revs->grep_filter);\n+\tgraph_clear(revs->graph);\n \t/* TODO (need to handle \"no_free\"): diff_free(&revs->diffopt) */\n \tdiff_free(&revs->pruning);\n \treflog_walk_info_release(revs->reflog_info);\ndiff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh\nindex 9fab0d779bb..e8456831e8b 100755\n--- a/t/t3413-rebase-hook.sh\n+++ b/t/t3413-rebase-hook.sh\n@@ -5,6 +5,7 @@ test_description='git rebase with its hook(s)'\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/t4052-stat-output.sh b/t/t4052-stat-output.sh\nindex b5c281edaa7..3ee27e277dc 100755\n--- a/t/t4052-stat-output.sh\n+++ b/t/t4052-stat-output.sh\n@@ -8,6 +8,7 @@ test_description='test --stat output of various commands'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466860","messageId":"patch-v2-07.15-a2cc6f10d0d-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 07/15] unpack-file: fix ancient leak in create_temp_file()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:43Z","receivedAt":"2022-11-08T18:18:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a leak that's been with us since 3407bb4940c (Add \"unpack-file\"\nhelper that unpacks a sha1 blob into a tmpfile., 2005-04-18). See\n00c8fd493af (cat-file: use streaming API to print blobs, 2012-03-07)\nfor prior art which shows the same API pattern, i.e. free()-ing the\nresult of read_object_file() after it's used.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/unpack-file.c | 1 +\n t/t6060-merge-index.sh | 2 ++\n t/t6401-merge-criss-cross.sh | 2 ++\n t/t6406-merge-attr.sh | 1 +\n t/t6407-merge-binary.sh | 1 +\n t/t7605-merge-resolve.sh | 1 +\n 6 files changed, 8 insertions(+)\n\ndiff --git a/builtin/unpack-file.c b/builtin/unpack-file.c\nindex 9e8119dd354..88de32b7d7e 100644\n--- a/builtin/unpack-file.c\n+++ b/builtin/unpack-file.c\n@@ -19,6 +19,7 @@ static char *create_temp_file(struct object_id *oid)\n \tif (write_in_full(fd, buf, size) < 0)\n \t\tdie_errno(\"unable to write temp-file\");\n \tclose(fd);\n+\tfree(buf);\n \treturn path;\n }\n \ndiff --git a/t/t6060-merge-index.sh b/t/t6060-merge-index.sh\nindex ed449abe552..1a8b64cce18 100755\n--- a/t/t6060-merge-index.sh\n+++ b/t/t6060-merge-index.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='basic git merge-index / git-merge-one-file tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup diverging branches' '\ndiff --git a/t/t6401-merge-criss-cross.sh b/t/t6401-merge-criss-cross.sh\nindex 9d5e992878f..1962310408b 100755\n--- a/t/t6401-merge-criss-cross.sh\n+++ b/t/t6401-merge-criss-cross.sh\n@@ -8,6 +8,8 @@\n \n \n test_description='Test criss-cross merge'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'prepare repository' '\ndiff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh\nindex 8650a88c40a..5e4e4dd6d9e 100755\n--- a/t/t6406-merge-attr.sh\n+++ b/t/t6406-merge-attr.sh\n@@ -8,6 +8,7 @@ test_description='per path merge controlled by merge attribute'\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/t6407-merge-binary.sh b/t/t6407-merge-binary.sh\nindex e8a28717cec..0753fc95f45 100755\n--- a/t/t6407-merge-binary.sh\n+++ b/t/t6407-merge-binary.sh\n@@ -5,6 +5,7 @@ test_description='ask merge-recursive to merge binary files'\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/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh\nindex 5d56c385464..62d935d31c2 100755\n--- a/t/t7605-merge-resolve.sh\n+++ b/t/t7605-merge-resolve.sh\n@@ -4,6 +4,7 @@ test_description='git merge\n \n Testing the resolve strategy.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466861","messageId":"patch-v2-09.15-46809d9be80-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 09/15] ls-files: fix a --with-tree memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:45Z","receivedAt":"2022-11-08T18:18:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in overlay_tree_on_index(), we need to\nclear_pathspec() at some point, which might as well be after the last\ntime we use it in the function.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/ls-files.c | 1 +\n t/t3060-ls-files-with-tree.sh | 2 ++\n t/t9148-git-svn-propset.sh | 1 -\n 3 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 4cf8a236483..a03b559ecaa 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -613,6 +613,7 @@ void overlay_tree_on_index(struct index_state *istate,\n \tif (!fn)\n \t\tfn = read_one_entry_quick;\n \terr = read_tree(the_repository, tree, &pathspec, fn, istate);\n+\tclear_pathspec(&pathspec);\n \tif (err)\n \t\tdie(\"unable to read tree entries %s\", tree_name);\n \ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex 52f76f7b57f..c4a72ae4462 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -8,6 +8,8 @@ test_description='git ls-files test (--with-tree).\n This test runs git ls-files --with-tree and in particular in\n a scenario known to trigger a crash with some versions of git.\n '\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t9148-git-svn-propset.sh b/t/t9148-git-svn-propset.sh\nindex 6cc76a07b39..aebb28995e5 100755\n--- a/t/t9148-git-svn-propset.sh\n+++ b/t/t9148-git-svn-propset.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn propset tests'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'setup propset via import' '\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466862","messageId":"patch-v2-10.15-243ab74120b-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 10/15] sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:46Z","receivedAt":"2022-11-08T18:18:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When \"read_strategy_opts()\" is called we may have populated the\n\"opts->strategy\" before, so we'll need to free() it to avoid leaking\nmemory.\n\nWe populate it before because we cal get_replay_opts() from within\n\"rebase.c\" with an already populated \"opts\", which we then copy. Then\nif we're doing a \"rebase -i\" the sequencer API itself will promptly\nclobber our alloc'd version of it with its own.\n\nIf this code is changed to do, instead of the added free() here a:\n\n\tif (opts->strategy)\n\t\topts->strategy = xstrdup(\"another leak\");\n\nWe get a couple of stacktraces from -fsanitize=leak showing how we\nended up clobbering the already allocated value, i.e.:\n\n\tDirect leak of 6 byte(s) in 1 object(s) allocated from:\n\t #0 0x7f2e8cd45545 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:75\n\t #1 0x7f2e8cb0fcaa in __GI___strdup string/strdup.c:42\n\t #2 0x6c4778 in xstrdup wrapper.c:39\n\t #3 0x66bcb8 in read_strategy_opts sequencer.c:2902\n\t #4 0x66bf7b in read_populate_opts sequencer.c:2969\n\t #5 0x6723f9 in sequencer_continue sequencer.c:5063\n\t #6 0x4a4f74 in run_sequencer_rebase builtin/rebase.c:348\n\t #7 0x4a64c8 in run_specific_rebase builtin/rebase.c:753\n\t #8 0x4a9b8b in cmd_rebase builtin/rebase.c:1824\n\t #9 0x407a32 in run_builtin git.c:466\n\t #10 0x407e0a in handle_builtin git.c:721\n\t #11 0x40803d in run_argv git.c:788\n\t #12 0x40850f in cmd_main git.c:923\n\t #13 0x4eee79 in main common-main.c:57\n\t #14 0x7f2e8ca9f209 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58\n\t #15 0x7f2e8ca9f2bb in __libc_start_main_impl ../csu/libc-start.c:389\n\t #16 0x405fd0 in _start (git+0x405fd0)\n\n\tDirect leak of 4 byte(s) in 1 object(s) allocated from:\n\t #0 0x7f2e8cd45545 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:75\n\t #1 0x7f2e8cb0fcaa in __GI___strdup string/strdup.c:42\n\t #2 0x6c4778 in xstrdup wrapper.c:39\n\t #3 0x4a3c31 in xstrdup_or_null git-compat-util.h:1169\n\t #4 0x4a447a in get_replay_opts builtin/rebase.c:163\n\t #5 0x4a4f5b in run_sequencer_rebase builtin/rebase.c:346\n\t #6 0x4a64c8 in run_specific_rebase builtin/rebase.c:753\n\t #7 0x4a9b8b in cmd_rebase builtin/rebase.c:1824\n\t #8 0x407a32 in run_builtin git.c:466\n\t #9 0x407e0a in handle_builtin git.c:721\n\t #10 0x40803d in run_argv git.c:788\n\t #11 0x40850f in cmd_main git.c:923\n\t #12 0x4eee79 in main common-main.c:57\n\t #13 0x7f2e8ca9f209 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58\n\t #14 0x7f2e8ca9f2bb in __libc_start_main_impl ../csu/libc-start.c:389\n\t #15 0x405fd0 in _start (git+0x405fd0)\n\nThis can be seen in e.g. the 4th test of\n\"t3404-rebase-interactive.sh\".\n\nIn the larger picture the ownership of the \"struct replay_opts\" is\nquite a mess, e.g. in this case rebase.c's static \"get_replay_opts()\"\nfunction partially creates it, but nothing in rebase.c will free()\nit. The structure is \"mostly owned\" by the sequencer API, but it also\nexpects to get these partially populated versions of it.\n\nIt would be better to have rebase keep track of what it allocated, and\nfree() that, and to pass that as a \"const\" to the sequencer API, which\nwould copy what it needs to its own version, and to free() that.\n\nBut doing so is a much larger change, and however messy the ownership\nboundary is here is consistent with what we're doing already, so let's\njust free() this to fix the leak.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n sequencer.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e23f6f0b718..433fec21562 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2897,6 +2897,7 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf)\n \tstrbuf_reset(buf);\n \tif (!read_oneliner(buf, rebase_path_strategy(), 0))\n \t\treturn;\n+\tfree(opts->strategy);\n \topts->strategy = strbuf_detach(buf, NULL);\n \tif (!read_oneliner(buf, rebase_path_strategy_opts(), 0))\n \t\treturn;\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466863","messageId":"patch-v2-11.15-15f0837697d-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 11/15] connected.c: free the \"struct packed_git\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:47Z","receivedAt":"2022-11-08T18:18:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"new_pack\" we allocate in check_connected() wasn't being\nfree'd. Let's do that before we return from the function. This has\nleaked ever since \"new_pack\" was added to this function in\nc6807a40dcd (clone: open a shortcut for connectivity check,\n2013-05-26).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n connected.c | 6 +++++-\n t/t3050-subprojects-fetch.sh | 1 +\n t/t4067-diff-partial-clone.sh | 1 +\n t/t5544-pack-objects-hook.sh | 2 ++\n t/t5610-clone-detached.sh | 1 +\n t/t5611-clone-config.sh | 1 +\n t/t5614-clone-submodules-shallow.sh | 1 +\n t/t5617-clone-submodules-remote.sh | 1 +\n 8 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/connected.c b/connected.c\nindex 74a20cb32e7..b7770825c7b 100644\n--- a/connected.c\n+++ b/connected.c\n@@ -85,6 +85,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n promisor_pack_found:\n \t\t\t;\n \t\t} while ((oid = fn(cb_data)) != NULL);\n+\t\tfree(new_pack);\n \t\treturn 0;\n \t}\n \n@@ -118,8 +119,10 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n \telse\n \t\trev_list.no_stderr = opt->quiet;\n \n-\tif (start_command(&rev_list))\n+\tif (start_command(&rev_list)) {\n+\t\tfree(new_pack);\n \t\treturn error(_(\"Could not run 'git rev-list'\"));\n+\t}\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n@@ -151,5 +154,6 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n \t\terr = error_errno(_(\"failed to close rev-list's stdin\"));\n \n \tsigchain_pop(SIGPIPE);\n+\tfree(new_pack);\n \treturn finish_command(&rev_list) || err;\n }\ndiff --git a/t/t3050-subprojects-fetch.sh b/t/t3050-subprojects-fetch.sh\nindex f1f09abdd9b..38846941655 100755\n--- a/t/t3050-subprojects-fetch.sh\n+++ b/t/t3050-subprojects-fetch.sh\n@@ -2,6 +2,7 @@\n \n test_description='fetching and pushing project with subproject'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh\nindex 28f42a4046e..f60f5cbd65f 100755\n--- a/t/t4067-diff-partial-clone.sh\n+++ b/t/t4067-diff-partial-clone.sh\n@@ -2,6 +2,7 @@\n \n test_description='behavior of diff when reading objects in a partial clone'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'git show batches blobs' '\ndiff --git a/t/t5544-pack-objects-hook.sh b/t/t5544-pack-objects-hook.sh\nindex 54f54f8d2eb..1a9e14bbccd 100755\n--- a/t/t5544-pack-objects-hook.sh\n+++ b/t/t5544-pack-objects-hook.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test custom script in place of pack-objects'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'create some history to fetch' '\ndiff --git a/t/t5610-clone-detached.sh b/t/t5610-clone-detached.sh\nindex a7ec21eda5a..022ed3d87c3 100755\n--- a/t/t5610-clone-detached.sh\n+++ b/t/t5610-clone-detached.sh\n@@ -4,6 +4,7 @@ test_description='test cloning a repository with detached HEAD'\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 head_is_detached() {\ndiff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh\nindex 4b3877216ee..727caff4433 100755\n--- a/t/t5611-clone-config.sh\n+++ b/t/t5611-clone-config.sh\n@@ -4,6 +4,7 @@ test_description='tests for git clone -c key=value'\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 'clone -c sets config in cloned repo' '\ndiff --git a/t/t5614-clone-submodules-shallow.sh b/t/t5614-clone-submodules-shallow.sh\nindex 0c85ef834ab..c2a2bb453ee 100755\n--- a/t/t5614-clone-submodules-shallow.sh\n+++ b/t/t5614-clone-submodules-shallow.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test shallow cloning of repos with submodules'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n pwd=$(pwd)\ndiff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh\nindex 68843382493..5a4d7936a72 100755\n--- a/t/t5617-clone-submodules-remote.sh\n+++ b/t/t5617-clone-submodules-remote.sh\n@@ -5,6 +5,7 @@ test_description='Test cloning repos with submodules using remote-tracking branc\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 pwd=$(pwd)\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466864","messageId":"patch-v2-13.15-10a477c7730-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 13/15] cherry-pick: free \"struct replay_opts\" members","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:49Z","receivedAt":"2022-11-08T18:18:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Call the release_revisions() function added in\n1878b5edc03 (revision.[ch]: provide and start using a\nrelease_revisions(), 2022-04-13) in cmd_cherry_pick(), as well as\nfreeing the xmalloc()'d \"revs\" member itself.\n\nThis is the same change as the one made for cmd_revert() a few lines\nabove it in fd74ac95ac3 (revert: free \"struct replay_opts\" members,\n2022-07-01).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/revert.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex ee32c714a76..0f81c8a795a 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -261,6 +261,9 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)\n \topts.action = REPLAY_PICK;\n \tsequencer_init_config(&opts);\n \tres = run_sequencer(argc, argv, &opts);\n+\tif (opts.revs)\n+\t\trelease_revisions(opts.revs);\n+\tfree(opts.revs);\n \tif (res < 0)\n \t\tdie(_(\"cherry-pick failed\"));\n \treturn res;\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466865","messageId":"patch-v2-12.15-f6d76250174-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 12/15] rebase: don't leak on \"--abort\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:48Z","receivedAt":"2022-11-08T18:18:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a leak in the recent 6159e7add49 (rebase --abort: improve reflog\nmessage, 2022-10-12). Before that commit we'd strbuf_release() the\nreflog message we were formatting, but when that code was refactored\nto use \"ropts.head_msg\" the strbuf_release() was omitted.\n\nIdeally the three users of \"ropts\" in cmd_rebase() should use\ndifferent \"ropts\" variables, in practice they're completely separate,\nas this and the other user in the \"switch\" statement will \"goto\ncleanup\", which won't touch \"ropts\".\n\nThe third caller after the \"switch\" is then unreachable if we take\nthese two branches, so all of them are getting a \"{ 0 }\" init'd\n\"ropts\".\n\nSo it's OK that we're leaving a stale pointer in \"ropts.head_msg\",\ncleaning it up was our responsibility, and it won't be used again.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/rebase.c | 1 +\n t/t7517-per-repo-email.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 6a831320313..3f360eb2f37 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1322,6 +1322,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tif (reset_head(the_repository, &ropts) < 0)\n \t\t\tdie(_(\"could not move back to %s\"),\n \t\t\t oid_to_hex(&options.orig_head->object.oid));\n+\t\tstrbuf_release(&head_msg);\n \t\tremove_branch_state(the_repository, 0);\n \t\tret = finish_rebase(&options);\n \t\tgoto cleanup;\ndiff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh\nindex 163ae804685..efc6496e2b2 100755\n--- a/t/t7517-per-repo-email.sh\n+++ b/t/t7517-per-repo-email.sh\n@@ -9,6 +9,7 @@ test_description='per-repo forced setting of email address'\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 a likely user.useConfigOnly use case' '\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466866","messageId":"patch-v2-15.15-f3e6a030394-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 15/15] built-ins: use free() not UNLEAK() if trivial, rm dead code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:51Z","receivedAt":"2022-11-08T18:19:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"For a lot of uses of UNLEAK() it would be quite tricky to release the\nmemory involved, or we're missing the relevant *_(release|clear)()\nfunctions. But in these cases we have them already, and can just\ninvoke them on the variable(s) involved, instead of UNLEAK().\n\nFor \"builtin/worktree.c\" the UNLEAK() was also added in [1], but the\nstruct member it's unleaking was removed in [2]. The only non-\"int\"\nmember of that structure is \"const char *keep_locked\", which comes to\nus via \"argv\" or a string literal[3].\n\nWe have good visibility via the compiler and\ntooling (e.g. SANITIZE=address) on bad free()-ing, but none on\nUNLEAK() we don't need anymore. So let's prefer releasing the memory\nwhen it's easy.\n\nFor \"bugreport\", \"worktree\" and \"config\" we need to start using a \"ret\n= ...\" return pattern. For \"builtin/bugreport.c\" these UNLEAK() were\nadded in [4], and for \"builtin/config.c\" in [1].\n\nFor \"config\" the code seen here was the only user of the \"value\"\nvariable. For \"ACTION_{RENAME,REMOVE}_SECTION\" we need to be sure to\nreturn the right exit code in the cases where we were relying on\nfalling through to the top-level.\n\nI think there's still a use-case for UNLEAK(), but hat it's changed\nsince then. Using it so that \"we can see the real leaks\" is\ncounter-productive in these cases.\n\nIt's more useful to have UNLEAK() be a marker of the remaining odd\ncases where it's hard to free() the memory for whatever reason. With\nthis change less than 20 of them remain in-tree.\n\n1. 0e5bba53af7 (add UNLEAK annotation for reducing leak false\n positives, 2017-09-08)\n2. d861d34a6ed (worktree: remove extra members from struct add_opts,\n 2018-04-24)\n3. 0db4961c49b (worktree: teach `add` to accept --reason with\n --lock, 2021-07-15)\n4. 0e5bba53af7 and 00d8c311050 (commit: fix \"author_ident\" leak,\n 2022-05-12).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/add.c | 2 +-\n builtin/bugreport.c | 9 ++++++---\n builtin/commit.c | 6 +++---\n builtin/config.c | 42 ++++++++++++++++++++----------------------\n builtin/diff.c | 2 +-\n builtin/worktree.c | 7 ++++---\n 6 files changed, 35 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex f84372964c8..c68ebafed5e 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -695,6 +695,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"Unable to write new index file\"));\n \n \tdir_clear(&dir);\n-\tUNLEAK(pathspec);\n+\tclear_pathspec(&pathspec);\n \treturn exit_status;\n }\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex 96052541cbf..5bc254be80f 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -106,6 +106,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)\n \tconst char *user_relative_path = NULL;\n \tchar *prefixed_filename;\n \tsize_t output_path_len;\n+\tint ret;\n \n \tconst struct option bugreport_options[] = {\n \t\tOPT_CALLBACK_F(0, \"diagnose\", &diagnose, N_(\"mode\"),\n@@ -182,7 +183,9 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)\n \t\tuser_relative_path);\n \n \tfree(prefixed_filename);\n-\tUNLEAK(buffer);\n-\tUNLEAK(report_path);\n-\treturn !!launch_editor(report_path.buf, NULL, NULL);\n+\tstrbuf_release(&buffer);\n+\n+\tret = !!launch_editor(report_path.buf, NULL, NULL);\n+\tstrbuf_release(&report_path);\n+\treturn ret;\n }\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c291199b704..f88a29167f4 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1874,8 +1874,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tapply_autostash(git_path_merge_autostash(the_repository));\n \n cleanup:\n-\tUNLEAK(author_ident);\n-\tUNLEAK(err);\n-\tUNLEAK(sb);\n+\tstrbuf_release(&author_ident);\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&sb);\n \treturn ret;\n }\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 753e5fac297..060cf9f3e05 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -639,8 +639,9 @@ static char *default_user_config(void)\n int cmd_config(int argc, const char **argv, const char *prefix)\n {\n \tint nongit = !startup_info->have_repository;\n-\tchar *value;\n+\tchar *value = NULL;\n \tint flags = 0;\n+\tint ret = 0;\n \n \tgiven_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));\n \n@@ -856,44 +857,38 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tfree(config_file);\n \t}\n \telse if (actions == ACTION_SET) {\n-\t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n \t\tret = git_config_set_in_file_gently(given_config_source.file, argv[0], value);\n \t\tif (ret == CONFIG_NOTHING_SET)\n \t\t\terror(_(\"cannot overwrite multiple values with a single value\\n\"\n \t\t\t\" Use a regexp, --add or --replace-all to change %s.\"), argv[0]);\n-\t\treturn ret;\n \t}\n \telse if (actions == ACTION_SET_ALL) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n-\t\treturn git_config_set_multivar_in_file_gently(given_config_source.file,\n-\t\t\t\t\t\t\t argv[0], value, argv[2],\n-\t\t\t\t\t\t\t flags);\n+\t\tret = git_config_set_multivar_in_file_gently(given_config_source.file,\n+\t\t\t\t\t\t\t argv[0], value, argv[2],\n+\t\t\t\t\t\t\t flags);\n \t}\n \telse if (actions == ACTION_ADD) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n-\t\treturn git_config_set_multivar_in_file_gently(given_config_source.file,\n-\t\t\t\t\t\t\t argv[0], value,\n-\t\t\t\t\t\t\t CONFIG_REGEX_NONE,\n-\t\t\t\t\t\t\t flags);\n+\t\tret = git_config_set_multivar_in_file_gently(given_config_source.file,\n+\t\t\t\t\t\t\t argv[0], value,\n+\t\t\t\t\t\t\t CONFIG_REGEX_NONE,\n+\t\t\t\t\t\t\t flags);\n \t}\n \telse if (actions == ACTION_REPLACE_ALL) {\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 3);\n \t\tvalue = normalize_value(argv[0], argv[1]);\n-\t\tUNLEAK(value);\n-\t\treturn git_config_set_multivar_in_file_gently(given_config_source.file,\n-\t\t\t\t\t\t\t argv[0], value, argv[2],\n-\t\t\t\t\t\t\t flags | CONFIG_FLAGS_MULTI_REPLACE);\n+\t\tret = git_config_set_multivar_in_file_gently(given_config_source.file,\n+\t\t\t\t\t\t\t argv[0], value, argv[2],\n+\t\t\t\t\t\t\t flags | CONFIG_FLAGS_MULTI_REPLACE);\n \t}\n \telse if (actions == ACTION_GET) {\n \t\tcheck_argc(argc, 1, 2);\n@@ -934,26 +929,28 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\t\t flags | CONFIG_FLAGS_MULTI_REPLACE);\n \t}\n \telse if (actions == ACTION_RENAME_SECTION) {\n-\t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 2, 2);\n \t\tret = git_config_rename_section_in_file(given_config_source.file,\n \t\t\t\t\t\t\targv[0], argv[1]);\n \t\tif (ret < 0)\n \t\t\treturn ret;\n-\t\tif (ret == 0)\n+\t\telse if (!ret)\n \t\t\tdie(_(\"no such section: %s\"), argv[0]);\n+\t\telse\n+\t\t\tret = 0;\n \t}\n \telse if (actions == ACTION_REMOVE_SECTION) {\n-\t\tint ret;\n \t\tcheck_write();\n \t\tcheck_argc(argc, 1, 1);\n \t\tret = git_config_rename_section_in_file(given_config_source.file,\n \t\t\t\t\t\t\targv[0], NULL);\n \t\tif (ret < 0)\n \t\t\treturn ret;\n-\t\tif (ret == 0)\n+\t\telse if (!ret)\n \t\t\tdie(_(\"no such section: %s\"), argv[0]);\n+\t\telse\n+\t\t\tret = 0;\n \t}\n \telse if (actions == ACTION_GET_COLOR) {\n \t\tcheck_argc(argc, 1, 2);\n@@ -966,5 +963,6 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\treturn get_colorbool(argv[0], argc == 2);\n \t}\n \n-\treturn 0;\n+\tfree(value);\n+\treturn ret;\n }\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 854d2c5a5c4..cb63f157dd1 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -609,7 +609,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \tif (1 < rev.diffopt.skip_stat_unmatch)\n \t\trefresh_index_quietly();\n \trelease_revisions(&rev);\n-\tUNLEAK(ent);\n+\tobject_array_clear(&ent);\n \tUNLEAK(blob);\n \treturn result;\n }\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 4a24d53be15..591d659faea 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -629,6 +629,7 @@ static int add(int ac, const char **av, const char *prefix)\n \t\t\t N_(\"try to match the new branch name with a remote-tracking branch\")),\n \t\tOPT_END()\n \t};\n+\tint ret;\n \n \tmemset(&opts, 0, sizeof(opts));\n \topts.checkout = 1;\n@@ -705,9 +706,9 @@ static int add(int ac, const char **av, const char *prefix)\n \t\tdie(_(\"--[no-]track can only be used if a new branch is created\"));\n \t}\n \n-\tUNLEAK(path);\n-\tUNLEAK(opts);\n-\treturn add_worktree(path, branch, &opts);\n+\tret = add_worktree(path, branch, &opts);\n+\tfree(path);\n+\treturn ret;\n }\n \n static void show_worktree_porcelain(struct worktree *wt, int line_terminator)\n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466867","messageId":"patch-v2-14.15-614c8fc1aef-20221108T172650Z-avarab@gmail.com","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"[PATCH v2 14/15] revert: fix parse_options_concat() leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-08T18:17:50Z","receivedAt":"2022-11-08T18:19:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Free memory from parse_options_concat(), which comes from code\noriginally added (then extended) in [1].\n\nAt this point we could get several more tests leak-free by free()-ing\nthe xstrdup() just above the line being changed, but that one's\ntrickier than it seems. The sequencer_remove_state() function\nsupposedly owns it, but sometimes we don't call it. I have a fix for\nit, but it's non-trivial, so let's fix the easy one first.\n\n1. c62f6ec341b (revert: add --ff option to allow fast forward when\n cherry-picking, 2010-03-06)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/revert.c | 1 +\n t/t3429-rebase-edit-todo.sh | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 0f81c8a795a..8bc87e4c770 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -221,6 +221,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \topts->strategy = xstrdup_or_null(opts->strategy);\n \tif (!opts->strategy && getenv(\"GIT_TEST_MERGE_ALGORITHM\"))\n \t\topts->strategy = xstrdup(getenv(\"GIT_TEST_MERGE_ALGORITHM\"));\n+\tfree(options);\n \n \tif (cmd == 'q') {\n \t\tint ret = sequencer_remove_state(opts);\ndiff --git a/t/t3429-rebase-edit-todo.sh b/t/t3429-rebase-edit-todo.sh\nindex abd66f36021..8e0d03969a2 100755\n--- a/t/t3429-rebase-edit-todo.sh\n+++ b/t/t3429-rebase-edit-todo.sh\n@@ -2,6 +2,7 @@\n \n test_description='rebase should reread the todo file if an exec modifies it'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-rebase.sh\n \n-- \n2.38.0.1467.g709fbdff1a9\n\n"},{"id":"466897","messageId":"Y2rB9Ol7307mrw4r@nand.local","threadId":"58743","inReplyTo":"cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com","subject":"Re: [PATCH v2 00/15] leak fixes: use existing constructors & other trivia","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-08T20:54:12Z","receivedAt":"2022-11-08T20:54:19Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Nov 08, 2022 at 07:17:36PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> This is a bag of simple leak fixes, mostly one-line additions where we\n> use existing destructors. See [1] for v1.\n>\n> Changes since v1:\n>\n> * Rebased on pw/rebase-no-reflog-action, per [2].\n> * It'll merge just fine without pw/rebase-no-reflog-action, but some\n> of the tests marked as leak-free will still leak then. So, in\n> practice they can be staged independently, as long as\n> pw/rebase-no-reflog-action is merged down first.\n> * The 10/15 here has an updated explanation about why we were leaking\n> that value. see also[3].\n\nLooking good. I did a --no-ff merge of 'pw/rebase-no-reflog-action' into\nthis topic, and pushed out the result.\n\nLet's start merging it down.\n\nThanks,\nTaylor\n"}]}