{"thread":{"id":"58092","subject":"[PATCH 01/11] check-ref-format: fix trivial memory leak","startedAt":"2022-06-30T18:00:32Z","lastAt":"2022-07-01T20:25:46Z","messageCount":31,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"458219","messageId":"patch-01.11-f35cf7c6ee9-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 01/11] check-ref-format: fix trivial memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:11Z","receivedAt":"2022-06-30T18:00:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in \"git check-ref-format\" that's been present in the\ncode in one form or another since 38eedc634bc (git check-ref-format\n--print, 2009-10-12), the code got substantially refactored in\ncfbe22f03f9 (check-ref-format: handle subcommands in separate\nfunctions, 2010-08-05).\n\nAs a result we can mark a test as passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/check-ref-format.c | 11 ++++++++---\n t/t1402-check-ref-format.sh | 1 +\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex bc67d3f0a83..fd0e5f86832 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -57,6 +57,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \tint normalize = 0;\n \tint flags = 0;\n \tconst char *refname;\n+\tchar *to_free = NULL;\n+\tint ret = 1;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage(builtin_check_ref_format_usage);\n@@ -81,11 +83,14 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \n \trefname = argv[i];\n \tif (normalize)\n-\t\trefname = collapse_slashes(refname);\n+\t\trefname = to_free = collapse_slashes(refname);\n \tif (check_refname_format(refname, flags))\n-\t\treturn 1;\n+\t\tgoto cleanup;\n \tif (normalize)\n \t\tprintf(\"%s\\n\", refname);\n \n-\treturn 0;\n+\tret = 0;\n+cleanup:\n+\tfree(to_free);\n+\treturn ret;\n }\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex cabc516ae9a..5ed9d7318e0 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test git check-ref-format'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n valid_ref() {\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458220","messageId":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":null,"subject":"[PATCH 00/11] built-ins: fix common memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:10Z","receivedAt":"2022-06-30T18:00:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a series of trivial leak-fixes which allows us to mark a\nsignificant number of new tests as entirely passing under\nSANITIZE=leak.\n\nNow that 2da81d1efb0 (Merge branch 'ab/plug-leak-in-revisions',\n2022-06-07) has landed we're going to have more & more cases where\nfixing just one leak will allow us to mark N tests as passing in their\nentirety. This is the first stepping stone on the way to that end.\n\nÆvar Arnfjörð Bjarmason (11):\n check-ref-format: fix trivial memory leak\n clone: fix memory leak in copy_ref() call\n submodule.c: free() memory from xgetcwd()\n revert: free \"struct replay_opts\" members\n cat-file: fix a memory leak in --batch-command mode\n merge-file: refactor for subsequent memory leak fix\n merge-file: fix memory leaks on error path\n checkout: add a missing clear_unpack_trees_porcelain()\n gc: fix a memory leak\n cat-file: fix a common \"struct object_context\" memory leak\n pull: fix a \"struct oid_array\" memory leak\n\n builtin/cat-file.c | 33 +++++++++++++++++++---------\n builtin/check-ref-format.c | 11 +++++++---\n builtin/checkout.c | 1 +\n builtin/clone.c | 1 +\n builtin/gc.c | 8 ++++++-\n builtin/merge-file.c | 32 +++++++++++++++------------\n builtin/pull.c | 16 +++++++++-----\n builtin/revert.c | 3 +++\n submodule.c | 3 ++-\n t/t0028-working-tree-encoding.sh | 1 +\n t/t1051-large-conversion.sh | 2 ++\n t/t1402-check-ref-format.sh | 1 +\n t/t3304-notes-mixed.sh | 1 +\n t/t4044-diff-index-unique-abbrev.sh | 2 ++\n t/t4140-apply-ita.sh | 1 +\n t/t5314-pack-cycle-detection.sh | 4 ++--\n t/t5524-pull-msg.sh | 1 +\n t/t6403-merge-file.sh | 2 ++\n t/t6417-merge-ours-theirs.sh | 1 +\n t/t6422-merge-rename-corner-cases.sh | 1 +\n t/t8007-cat-file-textconv.sh | 2 ++\n t/t8010-cat-file-filters.sh | 2 ++\n t/t9101-git-svn-props.sh | 1 -\n t/t9104-git-svn-follow-parent.sh | 1 -\n t/t9132-git-svn-broken-symlink.sh | 1 -\n t/t9301-fast-import-notes.sh | 1 +\n 26 files changed, 93 insertions(+), 40 deletions(-)\n\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458221","messageId":"patch-02.11-8e424238071-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 02/11] clone: fix memory leak in copy_ref() call","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:12Z","receivedAt":"2022-06-30T18:00:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak added in 0ec4b1650cc (clone: fix ref selection in\n--single-branch --branch=xxx, 2012-06-22), we need to free_refs() the\nresult of copy_ref().\n\nWe can't mark any tests passing passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\" as a result of this change, but\ne.g. \"t/t1500-rev-parse.sh\" now gets closer to passing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/clone.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 89a91b00177..c43c85dad07 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -494,6 +494,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,\n \t\t\t/* if --branch=tag, pull the requested tag explicitly */\n \t\t\tget_fetch_map(remote_head, tag_refspec, &tail, 0);\n \t\t}\n+\t\tfree_refs(remote_head);\n \t} else {\n \t\tint i;\n \t\tfor (i = 0; i < refspec->nr; i++)\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458222","messageId":"patch-03.11-a687d1281f8-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 03/11] submodule.c: free() memory from xgetcwd()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:13Z","receivedAt":"2022-06-30T18:00:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in code added in bf0231c6614 (rev-parse: add\n--show-superproject-working-tree, 2017-03-08), we should never have\nmade the result of xgetcwd() a \"const char *\", as we return a\nstrbuf_detach()'d value. Let's fix that and free() it when we're done\nwith it.\n\nWe can't mark any tests passing passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\" as a result of this change, but\ne.g. \"t/t1500-rev-parse.sh\" now gets closer to passing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n submodule.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 4e299f578f9..06073b2e7be 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2388,7 +2388,7 @@ int get_superproject_working_tree(struct strbuf *buf)\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf one_up = STRBUF_INIT;\n-\tconst char *cwd = xgetcwd();\n+\tchar *cwd = xgetcwd();\n \tint ret = 0;\n \tconst char *subpath;\n \tint code;\n@@ -2451,6 +2451,7 @@ int get_superproject_working_tree(struct strbuf *buf)\n \t\tret = 1;\n \t\tfree(super_wt);\n \t}\n+\tfree(cwd);\n \tstrbuf_release(&sb);\n \n \tcode = finish_command(&cp);\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458223","messageId":"patch-04.11-c9c2380be34-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 04/11] revert: free \"struct replay_opts\" members","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:14Z","receivedAt":"2022-06-30T18:00:39Z","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_revert(), as well as freeing\nthe xmalloc()'d \"revs\" member itself.\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 f84c253f4c6..2554f9099cc 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -246,6 +246,9 @@ int cmd_revert(int argc, const char **argv, const char *prefix)\n \tres = run_sequencer(argc, argv, &opts);\n \tif (res < 0)\n \t\tdie(_(\"revert failed\"));\n+\tif (opts.revs)\n+\t\trelease_revisions(opts.revs);\n+\tfree(opts.revs);\n \treturn res;\n }\n \n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458224","messageId":"patch-05.11-9ba267377ee-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 05/11] cat-file: fix a memory leak in --batch-command mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:15Z","receivedAt":"2022-06-30T18:00:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak introduced in 440c705ea63 (cat-file: add\n--batch-command mode, 2022-02-18). The free_cmds() function was only\ncalled on \"queued_nr\" if we had a \"flush\" command. As the \"without\nflush for blob info\" test added in the same commit shows we can't rely\non that, so let's call free_cmds() again at the end.\n\nSince \"nr\" follows the usual pattern of being set to 0 if we've\nfree()'d the memory already it's OK to call it twice, even in cases\nwhere we are doing a \"flush\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/cat-file.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 50cf38999d1..ac0459f96be 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -655,6 +655,7 @@ static void batch_objects_command(struct batch_options *opt,\n \t\tfree_cmds(queued_cmd, &nr);\n \t}\n \n+\tfree_cmds(queued_cmd, &nr);\n \tfree(queued_cmd);\n \tstrbuf_release(&input);\n }\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458225","messageId":"patch-06.11-17e66130b94-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 06/11] merge-file: refactor for subsequent memory leak fix","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:16Z","receivedAt":"2022-06-30T18:00:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor the code in builtin/merge-file.c to:\n\n * Use the initializer to zero out \"mmfs\", and use modern C syntax for\n the rest.\n\n * Refactor the the inner loop to use a variable and \"if/else if\"\n pattern followed by \"return\". This will make a change to change it to\n a \"goto cleanup\" pattern smaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/merge-file.c | 26 ++++++++++++++------------\n 1 file changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex e695867ee54..793817f3cb9 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -25,10 +25,10 @@ static int label_cb(const struct option *opt, const char *arg, int unset)\n \n int cmd_merge_file(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *names[3] = { NULL, NULL, NULL };\n-\tmmfile_t mmfs[3];\n-\tmmbuffer_t result = {NULL, 0};\n-\txmparam_t xmp = {{0}};\n+\tconst char *names[3] = { 0 };\n+\tmmfile_t mmfs[3] = { 0 };\n+\tmmbuffer_t result = { 0 };\n+\txmparam_t xmp = { 0 };\n \tint ret = 0, i = 0, to_stdout = 0;\n \tint quiet = 0;\n \tstruct option options[] = {\n@@ -71,21 +71,23 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 0; i < 3; i++) {\n \t\tchar *fname;\n-\t\tint ret;\n+\t\tmmfile_t *mmf = mmfs + i;\n \n \t\tif (!names[i])\n \t\t\tnames[i] = argv[i];\n \n \t\tfname = prefix_filename(prefix, argv[i]);\n-\t\tret = read_mmfile(mmfs + i, fname);\n+\n+\t\tif (read_mmfile(mmf, fname))\n+\t\t\tret = -1;\n+\t\telse if (mmf->size > MAX_XDIFF_SIZE ||\n+\t\t\t buffer_is_binary(mmf->ptr, mmf->size))\n+\t\t\tret = error(\"Cannot merge binary files: %s\",\n+\t\t\t\t argv[i]);\n+\n \t\tfree(fname);\n \t\tif (ret)\n-\t\t\treturn -1;\n-\n-\t\tif (mmfs[i].size > MAX_XDIFF_SIZE ||\n-\t\t buffer_is_binary(mmfs[i].ptr, mmfs[i].size))\n-\t\t\treturn error(\"Cannot merge binary files: %s\",\n-\t\t\t\t\targv[i]);\n+\t\t\treturn ret;\n \t}\n \n \txmp.ancestor = names[1];\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458226","messageId":"patch-07.11-8803a0df799-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 07/11] merge-file: fix memory leaks on error path","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:17Z","receivedAt":"2022-06-30T18:00:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in \"merge-file\", we need to loop over the \"mmfs\"\narray and free() what we've got so far when we error out. As a result\nwe can mark a test as passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/merge-file.c | 10 ++++++----\n t/t6403-merge-file.sh | 2 ++\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex 793817f3cb9..c923bbf2abb 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -87,7 +87,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \n \t\tfree(fname);\n \t\tif (ret)\n-\t\t\treturn ret;\n+\t\t\tgoto cleanup;\n+\n \t}\n \n \txmp.ancestor = names[1];\n@@ -95,9 +96,6 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \txmp.file2 = names[2];\n \tret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result);\n \n-\tfor (i = 0; i < 3; i++)\n-\t\tfree(mmfs[i].ptr);\n-\n \tif (ret >= 0) {\n \t\tconst char *filename = argv[0];\n \t\tchar *fpath = prefix_filename(prefix, argv[0]);\n@@ -118,5 +116,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tif (ret > 127)\n \t\tret = 127;\n \n+cleanup:\n+\tfor (i = 0; i < 3; i++)\n+\t\tfree(mmfs[i].ptr);\n+\n \treturn ret;\n }\ndiff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh\nindex 2f421d967ab..1a7082323dd 100755\n--- a/t/t6403-merge-file.sh\n+++ b/t/t6403-merge-file.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='RCS merge replacement: merge-file'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458227","messageId":"patch-08.11-94e28aa7ab3-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 08/11] checkout: add a missing clear_unpack_trees_porcelain()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:18Z","receivedAt":"2022-06-30T18:00:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In 1c41d2805e4 (unpack_trees_options: free messages when done,\n2018-05-21) we started calling clear_unpack_trees_porcelain() on this\ncodepath, but missed this error path, let's also clear what we've\nallocated in that case.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/checkout.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2eefda81d8c..3d6762106e8 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -750,6 +750,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\trefresh_cache(REFRESH_QUIET);\n \n \t\tif (unmerged_cache()) {\n+\t\t\tclear_unpack_trees_porcelain(&topts);\n \t\t\terror(_(\"you need to resolve your current index first\"));\n \t\t\treturn 1;\n \t\t}\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458228","messageId":"patch-09.11-b51a275b6b1-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 09/11] gc: fix a memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:19Z","receivedAt":"2022-06-30T18:00:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in code added in 41abfe15d95 (maintenance: add\npack-refs task, 2021-02-09), we need to call strvec_clear() on the\n\"struct strvec\" that we initialized.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/gc.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 021e9256ae2..eeff2b760e0 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -168,9 +168,15 @@ struct maintenance_run_opts;\n static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)\n {\n \tstruct strvec pack_refs_cmd = STRVEC_INIT;\n+\tint ret;\n+\n \tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n \n-\treturn run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);\n+\tret = run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);\n+\n+\tstrvec_clear(&pack_refs_cmd);\n+\n+\treturn ret;\n }\n \n static int too_many_loose_objects(void)\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458229","messageId":"patch-10.11-9474e2bcf93-20220630T175714Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH 10/11] cat-file: fix a common \"struct object_context\" memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T18:00:20Z","receivedAt":"2022-06-30T18:00:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak where \"cat-file\" will leak the \"path\" member. See\ne5fba602e59 (textconv: support for cat_file, 2010-06-15) for the code\nthat introduced the offending get_oid_with_context() call (called\nget_sha1_with_context() at the time).\n\nAs a result we can mark several tests as passing with SANITIZE=leak\nusing \"TEST_PASSES_SANITIZE_LEAK=true\".\n\nAs noted in dc944b65f1d (get_sha1_with_context: dynamically allocate\noc->path, 2017-05-19) callers must free the \"path\" member. That same\ncommit added the relevant free() to this function, but we weren't\ncatching cases where we'd return early.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/cat-file.c | 32 +++++++++++++++++++---------\n t/t0028-working-tree-encoding.sh | 1 +\n t/t1051-large-conversion.sh | 2 ++\n t/t3304-notes-mixed.sh | 1 +\n t/t4044-diff-index-unique-abbrev.sh | 2 ++\n t/t4140-apply-ita.sh | 1 +\n t/t5314-pack-cycle-detection.sh | 4 ++--\n t/t6422-merge-rename-corner-cases.sh | 1 +\n t/t8007-cat-file-textconv.sh | 2 ++\n t/t8010-cat-file-filters.sh | 2 ++\n t/t9104-git-svn-follow-parent.sh | 1 -\n t/t9132-git-svn-broken-symlink.sh | 1 -\n t/t9301-fast-import-notes.sh | 1 +\n 13 files changed, 37 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex ac0459f96be..f42782e955f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -71,6 +71,7 @@ static int stream_blob(const struct object_id *oid)\n static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\t\tint unknown_type)\n {\n+\tint ret;\n \tstruct object_id oid;\n \tenum object_type type;\n \tchar *buf;\n@@ -106,7 +107,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tif (sb.len) {\n \t\t\tprintf(\"%s\\n\", sb.buf);\n \t\t\tstrbuf_release(&sb);\n-\t\t\treturn 0;\n+\t\t\tret = 0;\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tbreak;\n \n@@ -115,7 +117,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n-\t\treturn 0;\n+\t\tret = 0;\n+\t\tgoto cleanup;\n \n \tcase 'e':\n \t\treturn !has_object_file(&oid);\n@@ -123,8 +126,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \tcase 'w':\n \n \t\tif (filter_object(path, obj_context.mode,\n-\t\t\t\t &oid, &buf, &size))\n-\t\t\treturn -1;\n+\t\t\t\t &oid, &buf, &size)) {\n+\t\t\tret = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbreak;\n \n \tcase 'c':\n@@ -143,11 +148,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\t\tconst char *ls_args[3] = { NULL };\n \t\t\tls_args[0] = \"ls-tree\";\n \t\t\tls_args[1] = obj_name;\n-\t\t\treturn cmd_ls_tree(2, ls_args, NULL);\n+\t\t\tret = cmd_ls_tree(2, ls_args, NULL);\n+\t\t\tgoto cleanup;\n \t\t}\n \n-\t\tif (type == OBJ_BLOB)\n-\t\t\treturn stream_blob(&oid);\n+\t\tif (type == OBJ_BLOB) {\n+\t\t\tret = stream_blob(&oid);\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbuf = read_object_file(&oid, &type, &size);\n \t\tif (!buf)\n \t\t\tdie(\"Cannot read object %s\", obj_name);\n@@ -172,8 +180,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\t\t} else\n \t\t\t\toidcpy(&blob_oid, &oid);\n \n-\t\t\tif (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB)\n-\t\t\t\treturn stream_blob(&blob_oid);\n+\t\t\tif (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) {\n+\t\t\t\tret = stream_blob(&blob_oid);\n+\t\t\t\tgoto cleanup;\n+\t\t\t}\n \t\t\t/*\n \t\t\t * we attempted to dereference a tag to a blob\n \t\t\t * and failed; there may be new dereference\n@@ -193,9 +203,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tdie(\"git cat-file %s: bad file\", obj_name);\n \n \twrite_or_die(1, buf, size);\n+\tret = 0;\n+cleanup:\n \tfree(buf);\n \tfree(obj_context.path);\n-\treturn 0;\n+\treturn ret;\n }\n \n struct expand_data {\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 82905a2156f..416eeabdb94 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-encoding.sh\"\n \ndiff --git a/t/t1051-large-conversion.sh b/t/t1051-large-conversion.sh\nindex 042b0e44292..f6709c9f569 100755\n--- a/t/t1051-large-conversion.sh\n+++ b/t/t1051-large-conversion.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test conversion filters on large files'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n set_attr() {\ndiff --git a/t/t3304-notes-mixed.sh b/t/t3304-notes-mixed.sh\nindex 03dfcd3954c..2c3a2452668 100755\n--- a/t/t3304-notes-mixed.sh\n+++ b/t/t3304-notes-mixed.sh\n@@ -5,6 +5,7 @@ test_description='Test notes trees that also contain non-notes'\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 number_of_commits=100\ndiff --git a/t/t4044-diff-index-unique-abbrev.sh b/t/t4044-diff-index-unique-abbrev.sh\nindex 4701796d10e..29e49d22902 100755\n--- a/t/t4044-diff-index-unique-abbrev.sh\n+++ b/t/t4044-diff-index-unique-abbrev.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test unique sha1 abbreviation on \"index from..to\" line'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh\nindex c614eaf04cc..b375aca0d74 100755\n--- a/t/t4140-apply-ita.sh\n+++ b/t/t4140-apply-ita.sh\n@@ -2,6 +2,7 @@\n \n test_description='git apply of i-t-a file'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t5314-pack-cycle-detection.sh b/t/t5314-pack-cycle-detection.sh\nindex 0aec8619e22..73a241743aa 100755\n--- a/t/t5314-pack-cycle-detection.sh\n+++ b/t/t5314-pack-cycle-detection.sh\n@@ -49,9 +49,9 @@ Then no matter which order we start looking at the packs in, we know that we\n will always find a delta for \"file\", because its lookup will always come\n immediately after the lookup for \"dummy\".\n '\n-. ./test-lib.sh\n-\n \n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n \n # Create a pack containing the tree $1 and blob $1:file, with\n # the latter stored as a delta against $2:file.\ndiff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh\nindex bf4ce3c63d4..9b65768aed6 100755\n--- a/t/t6422-merge-rename-corner-cases.sh\n+++ b/t/t6422-merge-rename-corner-cases.sh\n@@ -6,6 +6,7 @@ test_description=\"recursive merge corner cases w/ renames but not criss-crosses\"\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-merge.sh\n \ndiff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh\nindex b067983ba1c..c8266f17f14 100755\n--- a/t/t8007-cat-file-textconv.sh\n+++ b/t/t8007-cat-file-textconv.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='git cat-file textconv support'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n cat >helper <<'EOF'\ndiff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh\nindex 31de4b64dc0..ca04242ca01 100755\n--- a/t/t8010-cat-file-filters.sh\n+++ b/t/t8010-cat-file-filters.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='git cat-file filters support'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup ' '\ndiff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh\nindex 5cf2ef4b8b0..85d735861fc 100755\n--- a/t/t9104-git-svn-follow-parent.sh\n+++ b/t/t9104-git-svn-follow-parent.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn fetching'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'initialize repo' '\ndiff --git a/t/t9132-git-svn-broken-symlink.sh b/t/t9132-git-svn-broken-symlink.sh\nindex 4d8d0584b79..aeceffaf7b0 100755\n--- a/t/t9132-git-svn-broken-symlink.sh\n+++ b/t/t9132-git-svn-broken-symlink.sh\n@@ -2,7 +2,6 @@\n \n test_description='test that git handles an svn repository with empty symlinks'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n test_expect_success 'load svn dumpfile' '\n \tsvnadmin load \"$rawsvnrepo\" <\n---\n builtin/pull.c | 16 ++++++++++------\n t/t5524-pull-msg.sh | 1 +\n t/t6417-merge-ours-theirs.sh | 1 +\n t/t9101-git-svn-props.sh | 1 -\n 4 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 01155ba67b2..403a24d7ca6 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -990,6 +990,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tint rebase_unspecified = 0;\n \tint can_ff;\n \tint divergent;\n+\tint ret;\n \n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n@@ -1100,7 +1101,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (is_null_oid(&orig_head)) {\n \t\tif (merge_heads.nr > 1)\n \t\t\tdie(_(\"Cannot merge multiple branches into empty head.\"));\n-\t\treturn pull_into_void(merge_heads.oid, &curr_head);\n+\t\tret = pull_into_void(merge_heads.oid, &curr_head);\n+\t\tgoto cleanup;\n \t}\n \tif (merge_heads.nr > 1) {\n \t\tif (opt_rebase)\n@@ -1125,8 +1127,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (opt_rebase) {\n-\t\tint ret = 0;\n-\n \t\tstruct object_id newbase;\n \t\tstruct object_id upstream;\n \t\tget_rebase_newbase_and_upstream(&newbase, &upstream, &curr_head,\n@@ -1149,12 +1149,16 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))\n \t\t\tret = rebase_submodules();\n \n-\t\treturn ret;\n+\t\tgoto cleanup;\n \t} else {\n-\t\tint ret = run_merge();\n+\t\tret = run_merge();\n \t\tif (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||\n \t\t\t recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))\n \t\t\tret = update_submodules();\n-\t\treturn ret;\n+\t\tgoto cleanup;\n \t}\n+\n+cleanup:\n+\toid_array_clear(&merge_heads);\n+\treturn ret;\n }\ndiff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\nindex b2be3605f5a..56716e29ddf 100755\n--- a/t/t5524-pull-msg.sh\n+++ b/t/t5524-pull-msg.sh\n@@ -2,6 +2,7 @@\n \n test_description='git pull message generation'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n dollar='$Dollar'\ndiff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh\nindex 62d1406119e..482b73a998f 100755\n--- a/t/t6417-merge-ours-theirs.sh\n+++ b/t/t6417-merge-ours-theirs.sh\n@@ -4,6 +4,7 @@ test_description='Merge-recursive ours and theirs variants'\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/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh\nindex d043e80fc34..52046e60d51 100755\n--- a/t/t9101-git-svn-props.sh\n+++ b/t/t9101-git-svn-props.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn property tests'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n mkdir import\n-- \n2.37.0.874.g7d3439f13c4\n\n"},{"id":"458275","messageId":"xmqqczepddeg.fsf@gitster.g","threadId":"58092","inReplyTo":"patch-01.11-f35cf7c6ee9-20220630T175714Z-avarab@gmail.com","subject":"Re: [PATCH 01/11] check-ref-format: fix trivial memory leak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-30T21:55:51Z","receivedAt":"2022-06-30T21:55:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\n> index bc67d3f0a83..fd0e5f86832 100644\n> --- a/builtin/check-ref-format.c\n> +++ b/builtin/check-ref-format.c\n> @@ -57,6 +57,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n> \tint normalize = 0;\n> \tint flags = 0;\n> \tconst char *refname;\n> +\tchar *to_free = NULL;\n> +\tint ret = 1;\n> \n> \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n> \t\tusage(builtin_check_ref_format_usage);\n> @@ -81,11 +83,14 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n> \n> \trefname = argv[i];\n> \tif (normalize)\n> -\t\trefname = collapse_slashes(refname);\n> +\t\trefname = to_free = collapse_slashes(refname);\n> \tif (check_refname_format(refname, flags))\n> -\t\treturn 1;\n> +\t\tgoto cleanup;\n> \tif (normalize)\n> \t\tprintf(\"%s\\n\", refname);\n> \n> -\treturn 0;\n> +\tret = 0;\n> +cleanup:\n> +\tfree(to_free);\n> +\treturn ret;\n> }\n\nNice.\n\n> diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\n> index cabc516ae9a..5ed9d7318e0 100755\n> --- a/t/t1402-check-ref-format.sh\n> +++ b/t/t1402-check-ref-format.sh\n> @@ -2,6 +2,7 @@\n> \n> test_description='Test git check-ref-format'\n> \n> +TEST_PASSES_SANITIZE_LEAK=true\n> . ./test-lib.sh\n> \n> valid_ref() {\n"},{"id":"458277","messageId":"xmqq7d4xdd1o.fsf@gitster.g","threadId":"58092","inReplyTo":"patch-02.11-8e424238071-20220630T175714Z-avarab@gmail.com","subject":"Re: [PATCH 02/11] clone: fix memory leak in copy_ref() call","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-30T22:03:31Z","receivedAt":"2022-06-30T22:03:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Fix a memory leak added in 0ec4b1650cc (clone: fix ref selection in\n> --single-branch --branch=xxx, 2012-06-22), we need to free_refs() the\n> result of copy_ref().\n\nremote_head could be the result of guess_remote_head(), instead of\ncopy_ref(), and in either case, once we are done with it, we need to\nand can safely free it.\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 89a91b00177..c43c85dad07 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -494,6 +494,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,\n> \t\t\t/* if --branch=tag, pull the requested tag explicitly */\n> \t\t\tget_fetch_map(remote_head, tag_refspec, &tail, 0);\n> \t\t}\n> +\t\tfree_refs(remote_head);\n\nLooks good.\n\n> \t} else {\n> \t\tint i;\n> \t\tfor (i = 0; i < refspec->nr; i++)\n"},{"id":"458286","messageId":"xmqq35fldc9i.fsf@gitster.g","threadId":"58092","inReplyTo":"patch-08.11-94e28aa7ab3-20220630T175714Z-avarab@gmail.com","subject":"Re: [PATCH 08/11] checkout: add a missing clear_unpack_trees_porcelain()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-30T22:20:25Z","receivedAt":"2022-06-30T22:20:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> In 1c41d2805e4 (unpack_trees_options: free messages when done,\n> 2018-05-21) we started calling clear_unpack_trees_porcelain() on this\n> codepath, but missed this error path, let's also clear what we've\n> allocated in that case.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/checkout.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 2eefda81d8c..3d6762106e8 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -750,6 +750,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n> \t\trefresh_cache(REFRESH_QUIET);\n> \n> \t\tif (unmerged_cache()) {\n> +\t\t\tclear_unpack_trees_porcelain(&topts);\n> \t\t\terror(_(\"you need to resolve your current index first\"));\n> \t\t\treturn 1;\n> \t\t}\n\nDoes refresh_cache(REFRESH_QUIET) depend on the porcelain error\nmessages already set? If not another way to fix it may be to delay\nthe call to setup_unpack_trees_porcelain() until it becomes needed.\n\nBut the patch as posted is certainly an improvement.\n\nThanks.\n"},{"id":"458294","messageId":"220701.86wncxbu7q.gmgdl@evledraar.gmail.com","threadId":"58092","inReplyTo":"xmqq35fldc9i.fsf@gitster.g","subject":"Re: [PATCH 08/11] checkout: add a missing clear_unpack_trees_porcelain()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-30T23:34:00Z","receivedAt":"2022-06-30T23:35:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 30 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> In 1c41d2805e4 (unpack_trees_options: free messages when done,\n>> 2018-05-21) we started calling clear_unpack_trees_porcelain() on this\n>> codepath, but missed this error path, let's also clear what we've\n>> allocated in that case.\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> builtin/checkout.c | 1 +\n>> 1 file changed, 1 insertion(+)\n>>\n>> diff --git a/builtin/checkout.c b/builtin/checkout.c\n>> index 2eefda81d8c..3d6762106e8 100644\n>> --- a/builtin/checkout.c\n>> +++ b/builtin/checkout.c\n>> @@ -750,6 +750,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n>> \t\trefresh_cache(REFRESH_QUIET);\n>> \n>> \t\tif (unmerged_cache()) {\n>> +\t\t\tclear_unpack_trees_porcelain(&topts);\n>> \t\t\terror(_(\"you need to resolve your current index first\"));\n>> \t\t\treturn 1;\n>> \t\t}\n>\n> Does refresh_cache(REFRESH_QUIET) depend on the porcelain error\n> messages already set? If not another way to fix it may be to delay\n> the call to setup_unpack_trees_porcelain() until it becomes needed.\n>\n> But the patch as posted is certainly an improvement.\n\nYes, that would work too, I can do it that way if you'd like.\n\nI was trying to keep these fixes as narrow as possible, and resist any\ntemptations to re-arrange code so as to avoid allocations, which we can\noften do, but then instead of a 1-line diff it's 10, 50, 100... :)\n\nIn this case it'll be somewhere around 10, and just a code-move, so\nmaybe that's fine...\n"},{"id":"458298","messageId":"xmqqzghtaf6u.fsf@gitster.g","threadId":"58092","inReplyTo":"220701.86wncxbu7q.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 08/11] checkout: add a missing clear_unpack_trees_porcelain()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-30T23:45:29Z","receivedAt":"2022-06-30T23:45:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> Does refresh_cache(REFRESH_QUIET) depend on the porcelain error\n>> messages already set? If not another way to fix it may be to delay\n>> the call to setup_unpack_trees_porcelain() until it becomes needed.\n>>\n>> But the patch as posted is certainly an improvement.\n>\n> Yes, that would work too, I can do it that way if you'd like.\n>\n> I was trying to keep these fixes as narrow as possible, and resist any\n> temptations to re-arrange code so as to avoid allocations, which we can\n> often do, but then instead of a 1-line diff it's 10, 50, 100... :)\n>\n> In this case it'll be somewhere around 10, and just a code-move, so\n> maybe that's fine...\n\nThe ideal answer I was hoping to hear was\n\n The current implementation of the refresh_cache() function might\n not, but there is no guarantee that it will stay so. We may add\n more calls that may (or may not) need to access the error\n message table before this early return. Setting the porcelain\n messages before making any calls to any functions in the API and\n unsetting after we are done is a much better code structure.\n\n;-)\n\n"},{"id":"458366","messageId":"patch-v2-01.11-f35cf7c6ee9-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 01/11] check-ref-format: fix trivial memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:50Z","receivedAt":"2022-07-01T10:43:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in \"git check-ref-format\" that's been present in the\ncode in one form or another since 38eedc634bc (git check-ref-format\n--print, 2009-10-12), the code got substantially refactored in\ncfbe22f03f9 (check-ref-format: handle subcommands in separate\nfunctions, 2010-08-05).\n\nAs a result we can mark a test as passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/check-ref-format.c | 11 ++++++++---\n t/t1402-check-ref-format.sh | 1 +\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex bc67d3f0a83..fd0e5f86832 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -57,6 +57,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \tint normalize = 0;\n \tint flags = 0;\n \tconst char *refname;\n+\tchar *to_free = NULL;\n+\tint ret = 1;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage(builtin_check_ref_format_usage);\n@@ -81,11 +83,14 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \n \trefname = argv[i];\n \tif (normalize)\n-\t\trefname = collapse_slashes(refname);\n+\t\trefname = to_free = collapse_slashes(refname);\n \tif (check_refname_format(refname, flags))\n-\t\treturn 1;\n+\t\tgoto cleanup;\n \tif (normalize)\n \t\tprintf(\"%s\\n\", refname);\n \n-\treturn 0;\n+\tret = 0;\n+cleanup:\n+\tfree(to_free);\n+\treturn ret;\n }\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex cabc516ae9a..5ed9d7318e0 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test git check-ref-format'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n valid_ref() {\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458367","messageId":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com","subject":"[PATCH v2 00/11] built-ins: fix common memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:49Z","receivedAt":"2022-07-01T10:43:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix common memory leaks in built-ins, for a general overview see the\nv1 CL:\nhttps://lore.kernel.org/git/cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com/\n\nChanges since v1:\n\n * Amend the commit message of 2/11, we always get copy_ref() data,\n but note that we do so via an indirection in clone.c\n * Replace 8/11, maybe that solution is going overboard, we could also\n just drop it from this series...\n\nÆvar Arnfjörð Bjarmason (11):\n check-ref-format: fix trivial memory leak\n clone: fix memory leak in wanted_peer_refs()\n submodule.c: free() memory from xgetcwd()\n revert: free \"struct replay_opts\" members\n cat-file: fix a memory leak in --batch-command mode\n merge-file: refactor for subsequent memory leak fix\n merge-file: fix memory leaks on error path\n checkout: avoid \"struct unpack_trees_options\" leak\n gc: fix a memory leak\n cat-file: fix a common \"struct object_context\" memory leak\n pull: fix a \"struct oid_array\" memory leak\n\n builtin/cat-file.c | 33 +++++++++++++++++--------\n builtin/check-ref-format.c | 11 ++++++---\n builtin/checkout.c | 36 +++++++++++++++++-----------\n builtin/clone.c | 1 +\n builtin/gc.c | 8 ++++++-\n builtin/merge-file.c | 32 ++++++++++++++-----------\n builtin/pull.c | 16 ++++++++-----\n builtin/revert.c | 3 +++\n submodule.c | 3 ++-\n t/t0028-working-tree-encoding.sh | 1 +\n t/t1051-large-conversion.sh | 2 ++\n t/t1402-check-ref-format.sh | 1 +\n t/t3304-notes-mixed.sh | 1 +\n t/t4044-diff-index-unique-abbrev.sh | 2 ++\n t/t4140-apply-ita.sh | 1 +\n t/t5314-pack-cycle-detection.sh | 4 ++--\n t/t5524-pull-msg.sh | 1 +\n t/t6403-merge-file.sh | 2 ++\n t/t6417-merge-ours-theirs.sh | 1 +\n t/t6422-merge-rename-corner-cases.sh | 1 +\n t/t8007-cat-file-textconv.sh | 2 ++\n t/t8010-cat-file-filters.sh | 2 ++\n t/t9101-git-svn-props.sh | 1 -\n t/t9104-git-svn-follow-parent.sh | 1 -\n t/t9132-git-svn-broken-symlink.sh | 1 -\n t/t9301-fast-import-notes.sh | 1 +\n 26 files changed, 114 insertions(+), 54 deletions(-)\n\nRange-diff against v1:\n 1: f35cf7c6ee9 = 1: f35cf7c6ee9 check-ref-format: fix trivial memory leak\n 2: 8e424238071 ! 2: 24a1eaa51a3 clone: fix memory leak in copy_ref() call\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - clone: fix memory leak in copy_ref() call\n + clone: fix memory leak in wanted_peer_refs()\n \n Fix a memory leak added in 0ec4b1650cc (clone: fix ref selection in\n - --single-branch --branch=xxx, 2012-06-22), we need to free_refs() the\n - result of copy_ref().\n + --single-branch --branch=xxx, 2012-06-22).\n +\n + Whether we get our \"remote_head\" from copy_ref() directly, or with a\n + call to guess_remote_head() it'll be the result of a copy_ref() in\n + either case, as guess_remote_head() is a wrapper for copy_ref() (or it\n + returns NULL).\n \n We can't mark any tests passing passing with SANITIZE=leak using\n \"TEST_PASSES_SANITIZE_LEAK=true\" as a result of this change, but\n 3: a687d1281f8 = 3: 691b178aaf0 submodule.c: free() memory from xgetcwd()\n 4: c9c2380be34 = 4: 6fc895676f4 revert: free \"struct replay_opts\" members\n 5: 9ba267377ee = 5: 692d6e0e3d8 cat-file: fix a memory leak in --batch-command mode\n 6: 17e66130b94 = 6: d894e97b5ae merge-file: refactor for subsequent memory leak fix\n 7: 8803a0df799 = 7: 8658f3ad020 merge-file: fix memory leaks on error path\n 8: 94e28aa7ab3 ! 8: e21d7e4e9df checkout: add a missing clear_unpack_trees_porcelain()\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - checkout: add a missing clear_unpack_trees_porcelain()\n + checkout: avoid \"struct unpack_trees_options\" leak\n \n In 1c41d2805e4 (unpack_trees_options: free messages when done,\n 2018-05-21) we started calling clear_unpack_trees_porcelain() on this\n - codepath, but missed this error path, let's also clear what we've\n - allocated in that case.\n + codepath, but missed this error path.\n +\n + We could call clear_unpack_trees_porcelain() just before we error()\n + and return when unmerged_cache() fails, but the more correct fix is to\n + not have the unmerged_cache() check happen in the middle of our\n + \"topts\" setup.\n +\n + Before 23cbf11b5c0 (merge-recursive: porcelain messages for checkout,\n + 2010-08-11) we would not malloc() to setup our \"topts\", which is when\n + this started to leak on the error path.\n +\n + Before that this code wasn't conflating the setup of \"topts\" and the\n + unmerged_cache() call in any meaningful way. The initial version in\n + 782c2d65c24 (Build in checkout, 2008-02-07) just does a \"memset\" of\n + it, and initializes a single struct member.\n +\n + Then in 8ccba008ee3 (unpack-trees: allow Porcelain to give different\n + error messages, 2008-05-17) we added the initialization of the error\n + message, which as noted above finally started leaking in 23cbf11b5c0.\n +\n + Let's fix the memory leak, and avoid future issues by initializing the\n + \"topts\" with a helper function. There are no functional changes here.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## builtin/checkout.c ##\n +@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)\n + \tbranch->path = strbuf_detach(&buf, NULL);\n + }\n + \n ++static void init_topts(struct unpack_trees_options *topts, int merge,\n ++\t\t int show_progress, int overwrite_ignore,\n ++\t\t struct commit *old_commit)\n ++{\n ++\tmemset(topts, 0, sizeof(*topts));\n ++\ttopts->head_idx = -1;\n ++\ttopts->src_index = &the_index;\n ++\ttopts->dst_index = &the_index;\n ++\n ++\tsetup_unpack_trees_porcelain(topts, \"checkout\");\n ++\n ++\ttopts->initial_checkout = is_cache_unborn();\n ++\ttopts->update = 1;\n ++\ttopts->merge = 1;\n ++\ttopts->quiet = merge && old_commit;\n ++\ttopts->verbose_update = show_progress;\n ++\ttopts->fn = twoway_merge;\n ++\ttopts->preserve_ignored = !overwrite_ignore;\n ++}\n ++\n + static int merge_working_tree(const struct checkout_opts *opts,\n + \t\t\t struct branch_info *old_branch_info,\n + \t\t\t struct branch_info *new_branch_info,\n @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,\n + \t\tstruct unpack_trees_options topts;\n + \t\tconst struct object_id *old_commit_oid;\n + \n +-\t\tmemset(&topts, 0, sizeof(topts));\n +-\t\ttopts.head_idx = -1;\n +-\t\ttopts.src_index = &the_index;\n +-\t\ttopts.dst_index = &the_index;\n +-\n +-\t\tsetup_unpack_trees_porcelain(&topts, \"checkout\");\n +-\n \t\trefresh_cache(REFRESH_QUIET);\n \n \t\tif (unmerged_cache()) {\n -+\t\t\tclear_unpack_trees_porcelain(&topts);\n - \t\t\terror(_(\"you need to resolve your current index first\"));\n - \t\t\treturn 1;\n +@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,\n \t\t}\n + \n + \t\t/* 2-way merge to the new branch */\n +-\t\ttopts.initial_checkout = is_cache_unborn();\n +-\t\ttopts.update = 1;\n +-\t\ttopts.merge = 1;\n +-\t\ttopts.quiet = opts->merge && old_branch_info->commit;\n +-\t\ttopts.verbose_update = opts->show_progress;\n +-\t\ttopts.fn = twoway_merge;\n ++\t\tinit_topts(&topts, opts->merge, opts->show_progress,\n ++\t\t\t opts->overwrite_ignore, old_branch_info->commit);\n + \t\tinit_checkout_metadata(&topts.meta, new_branch_info->refname,\n + \t\t\t\t new_branch_info->commit ?\n + \t\t\t\t &new_branch_info->commit->object.oid :\n + \t\t\t\t &new_branch_info->oid, NULL);\n +-\t\ttopts.preserve_ignored = !opts->overwrite_ignore;\n + \n + \t\told_commit_oid = old_branch_info->commit ?\n + \t\t\t&old_branch_info->commit->object.oid :\n 9: b51a275b6b1 = 9: 10a97a9cac4 gc: fix a memory leak\n10: 9474e2bcf93 = 10: 481d006378b cat-file: fix a common \"struct object_context\" memory leak\n11: 022399ad652 = 11: 5666104943f pull: fix a \"struct oid_array\" memory leak\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458368","messageId":"patch-v2-02.11-24a1eaa51a3-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 02/11] clone: fix memory leak in wanted_peer_refs()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:51Z","receivedAt":"2022-07-01T10:43:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak added in 0ec4b1650cc (clone: fix ref selection in\n--single-branch --branch=xxx, 2012-06-22).\n\nWhether we get our \"remote_head\" from copy_ref() directly, or with a\ncall to guess_remote_head() it'll be the result of a copy_ref() in\neither case, as guess_remote_head() is a wrapper for copy_ref() (or it\nreturns NULL).\n\nWe can't mark any tests passing passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\" as a result of this change, but\ne.g. \"t/t1500-rev-parse.sh\" now gets closer to passing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/clone.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 89a91b00177..c43c85dad07 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -494,6 +494,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,\n \t\t\t/* if --branch=tag, pull the requested tag explicitly */\n \t\t\tget_fetch_map(remote_head, tag_refspec, &tail, 0);\n \t\t}\n+\t\tfree_refs(remote_head);\n \t} else {\n \t\tint i;\n \t\tfor (i = 0; i < refspec->nr; i++)\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458369","messageId":"patch-v2-03.11-691b178aaf0-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 03/11] submodule.c: free() memory from xgetcwd()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:52Z","receivedAt":"2022-07-01T10:43:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in code added in bf0231c6614 (rev-parse: add\n--show-superproject-working-tree, 2017-03-08), we should never have\nmade the result of xgetcwd() a \"const char *\", as we return a\nstrbuf_detach()'d value. Let's fix that and free() it when we're done\nwith it.\n\nWe can't mark any tests passing passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\" as a result of this change, but\ne.g. \"t/t1500-rev-parse.sh\" now gets closer to passing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n submodule.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 4e299f578f9..06073b2e7be 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2388,7 +2388,7 @@ int get_superproject_working_tree(struct strbuf *buf)\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf one_up = STRBUF_INIT;\n-\tconst char *cwd = xgetcwd();\n+\tchar *cwd = xgetcwd();\n \tint ret = 0;\n \tconst char *subpath;\n \tint code;\n@@ -2451,6 +2451,7 @@ int get_superproject_working_tree(struct strbuf *buf)\n \t\tret = 1;\n \t\tfree(super_wt);\n \t}\n+\tfree(cwd);\n \tstrbuf_release(&sb);\n \n \tcode = finish_command(&cp);\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458370","messageId":"patch-v2-04.11-6fc895676f4-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 04/11] revert: free \"struct replay_opts\" members","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:53Z","receivedAt":"2022-07-01T10:43:22Z","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_revert(), as well as freeing\nthe xmalloc()'d \"revs\" member itself.\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 f84c253f4c6..2554f9099cc 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -246,6 +246,9 @@ int cmd_revert(int argc, const char **argv, const char *prefix)\n \tres = run_sequencer(argc, argv, &opts);\n \tif (res < 0)\n \t\tdie(_(\"revert failed\"));\n+\tif (opts.revs)\n+\t\trelease_revisions(opts.revs);\n+\tfree(opts.revs);\n \treturn res;\n }\n \n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458371","messageId":"patch-v2-05.11-692d6e0e3d8-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 05/11] cat-file: fix a memory leak in --batch-command mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:54Z","receivedAt":"2022-07-01T10:43:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak introduced in 440c705ea63 (cat-file: add\n--batch-command mode, 2022-02-18). The free_cmds() function was only\ncalled on \"queued_nr\" if we had a \"flush\" command. As the \"without\nflush for blob info\" test added in the same commit shows we can't rely\non that, so let's call free_cmds() again at the end.\n\nSince \"nr\" follows the usual pattern of being set to 0 if we've\nfree()'d the memory already it's OK to call it twice, even in cases\nwhere we are doing a \"flush\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/cat-file.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 50cf38999d1..ac0459f96be 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -655,6 +655,7 @@ static void batch_objects_command(struct batch_options *opt,\n \t\tfree_cmds(queued_cmd, &nr);\n \t}\n \n+\tfree_cmds(queued_cmd, &nr);\n \tfree(queued_cmd);\n \tstrbuf_release(&input);\n }\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458372","messageId":"patch-v2-06.11-d894e97b5ae-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 06/11] merge-file: refactor for subsequent memory leak fix","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:55Z","receivedAt":"2022-07-01T10:43:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor the code in builtin/merge-file.c to:\n\n * Use the initializer to zero out \"mmfs\", and use modern C syntax for\n the rest.\n\n * Refactor the the inner loop to use a variable and \"if/else if\"\n pattern followed by \"return\". This will make a change to change it to\n a \"goto cleanup\" pattern smaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/merge-file.c | 26 ++++++++++++++------------\n 1 file changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex e695867ee54..793817f3cb9 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -25,10 +25,10 @@ static int label_cb(const struct option *opt, const char *arg, int unset)\n \n int cmd_merge_file(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *names[3] = { NULL, NULL, NULL };\n-\tmmfile_t mmfs[3];\n-\tmmbuffer_t result = {NULL, 0};\n-\txmparam_t xmp = {{0}};\n+\tconst char *names[3] = { 0 };\n+\tmmfile_t mmfs[3] = { 0 };\n+\tmmbuffer_t result = { 0 };\n+\txmparam_t xmp = { 0 };\n \tint ret = 0, i = 0, to_stdout = 0;\n \tint quiet = 0;\n \tstruct option options[] = {\n@@ -71,21 +71,23 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 0; i < 3; i++) {\n \t\tchar *fname;\n-\t\tint ret;\n+\t\tmmfile_t *mmf = mmfs + i;\n \n \t\tif (!names[i])\n \t\t\tnames[i] = argv[i];\n \n \t\tfname = prefix_filename(prefix, argv[i]);\n-\t\tret = read_mmfile(mmfs + i, fname);\n+\n+\t\tif (read_mmfile(mmf, fname))\n+\t\t\tret = -1;\n+\t\telse if (mmf->size > MAX_XDIFF_SIZE ||\n+\t\t\t buffer_is_binary(mmf->ptr, mmf->size))\n+\t\t\tret = error(\"Cannot merge binary files: %s\",\n+\t\t\t\t argv[i]);\n+\n \t\tfree(fname);\n \t\tif (ret)\n-\t\t\treturn -1;\n-\n-\t\tif (mmfs[i].size > MAX_XDIFF_SIZE ||\n-\t\t buffer_is_binary(mmfs[i].ptr, mmfs[i].size))\n-\t\t\treturn error(\"Cannot merge binary files: %s\",\n-\t\t\t\t\targv[i]);\n+\t\t\treturn ret;\n \t}\n \n \txmp.ancestor = names[1];\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458373","messageId":"patch-v2-09.11-10a97a9cac4-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 09/11] gc: fix a memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:58Z","receivedAt":"2022-07-01T10:43:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in code added in 41abfe15d95 (maintenance: add\npack-refs task, 2021-02-09), we need to call strvec_clear() on the\n\"struct strvec\" that we initialized.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/gc.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 021e9256ae2..eeff2b760e0 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -168,9 +168,15 @@ struct maintenance_run_opts;\n static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)\n {\n \tstruct strvec pack_refs_cmd = STRVEC_INIT;\n+\tint ret;\n+\n \tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n \n-\treturn run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);\n+\tret = run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);\n+\n+\tstrvec_clear(&pack_refs_cmd);\n+\n+\treturn ret;\n }\n \n static int too_many_loose_objects(void)\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458374","messageId":"patch-v2-08.11-e21d7e4e9df-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 08/11] checkout: avoid \"struct unpack_trees_options\" leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:57Z","receivedAt":"2022-07-01T10:43:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In 1c41d2805e4 (unpack_trees_options: free messages when done,\n2018-05-21) we started calling clear_unpack_trees_porcelain() on this\ncodepath, but missed this error path.\n\nWe could call clear_unpack_trees_porcelain() just before we error()\nand return when unmerged_cache() fails, but the more correct fix is to\nnot have the unmerged_cache() check happen in the middle of our\n\"topts\" setup.\n\nBefore 23cbf11b5c0 (merge-recursive: porcelain messages for checkout,\n2010-08-11) we would not malloc() to setup our \"topts\", which is when\nthis started to leak on the error path.\n\nBefore that this code wasn't conflating the setup of \"topts\" and the\nunmerged_cache() call in any meaningful way. The initial version in\n782c2d65c24 (Build in checkout, 2008-02-07) just does a \"memset\" of\nit, and initializes a single struct member.\n\nThen in 8ccba008ee3 (unpack-trees: allow Porcelain to give different\nerror messages, 2008-05-17) we added the initialization of the error\nmessage, which as noted above finally started leaking in 23cbf11b5c0.\n\nLet's fix the memory leak, and avoid future issues by initializing the\n\"topts\" with a helper function. There are no functional changes here.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/checkout.c | 36 ++++++++++++++++++++++--------------\n 1 file changed, 22 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2eefda81d8c..1109f1301f4 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -710,6 +710,26 @@ static void setup_branch_path(struct branch_info *branch)\n \tbranch->path = strbuf_detach(&buf, NULL);\n }\n \n+static void init_topts(struct unpack_trees_options *topts, int merge,\n+\t\t int show_progress, int overwrite_ignore,\n+\t\t struct commit *old_commit)\n+{\n+\tmemset(topts, 0, sizeof(*topts));\n+\ttopts->head_idx = -1;\n+\ttopts->src_index = &the_index;\n+\ttopts->dst_index = &the_index;\n+\n+\tsetup_unpack_trees_porcelain(topts, \"checkout\");\n+\n+\ttopts->initial_checkout = is_cache_unborn();\n+\ttopts->update = 1;\n+\ttopts->merge = 1;\n+\ttopts->quiet = merge && old_commit;\n+\ttopts->verbose_update = show_progress;\n+\ttopts->fn = twoway_merge;\n+\ttopts->preserve_ignored = !overwrite_ignore;\n+}\n+\n static int merge_working_tree(const struct checkout_opts *opts,\n \t\t\t struct branch_info *old_branch_info,\n \t\t\t struct branch_info *new_branch_info,\n@@ -740,13 +760,6 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\tstruct unpack_trees_options topts;\n \t\tconst struct object_id *old_commit_oid;\n \n-\t\tmemset(&topts, 0, sizeof(topts));\n-\t\ttopts.head_idx = -1;\n-\t\ttopts.src_index = &the_index;\n-\t\ttopts.dst_index = &the_index;\n-\n-\t\tsetup_unpack_trees_porcelain(&topts, \"checkout\");\n-\n \t\trefresh_cache(REFRESH_QUIET);\n \n \t\tif (unmerged_cache()) {\n@@ -755,17 +768,12 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\t}\n \n \t\t/* 2-way merge to the new branch */\n-\t\ttopts.initial_checkout = is_cache_unborn();\n-\t\ttopts.update = 1;\n-\t\ttopts.merge = 1;\n-\t\ttopts.quiet = opts->merge && old_branch_info->commit;\n-\t\ttopts.verbose_update = opts->show_progress;\n-\t\ttopts.fn = twoway_merge;\n+\t\tinit_topts(&topts, opts->merge, opts->show_progress,\n+\t\t\t opts->overwrite_ignore, old_branch_info->commit);\n \t\tinit_checkout_metadata(&topts.meta, new_branch_info->refname,\n \t\t\t\t new_branch_info->commit ?\n \t\t\t\t &new_branch_info->commit->object.oid :\n \t\t\t\t &new_branch_info->oid, NULL);\n-\t\ttopts.preserve_ignored = !opts->overwrite_ignore;\n \n \t\told_commit_oid = old_branch_info->commit ?\n \t\t\t&old_branch_info->commit->object.oid :\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458375","messageId":"patch-v2-07.11-8658f3ad020-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 07/11] merge-file: fix memory leaks on error path","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:56Z","receivedAt":"2022-07-01T10:43:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in \"merge-file\", we need to loop over the \"mmfs\"\narray and free() what we've got so far when we error out. As a result\nwe can mark a test as passing with SANITIZE=leak using\n\"TEST_PASSES_SANITIZE_LEAK=true\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/merge-file.c | 10 ++++++----\n t/t6403-merge-file.sh | 2 ++\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex 793817f3cb9..c923bbf2abb 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -87,7 +87,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \n \t\tfree(fname);\n \t\tif (ret)\n-\t\t\treturn ret;\n+\t\t\tgoto cleanup;\n+\n \t}\n \n \txmp.ancestor = names[1];\n@@ -95,9 +96,6 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \txmp.file2 = names[2];\n \tret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result);\n \n-\tfor (i = 0; i < 3; i++)\n-\t\tfree(mmfs[i].ptr);\n-\n \tif (ret >= 0) {\n \t\tconst char *filename = argv[0];\n \t\tchar *fpath = prefix_filename(prefix, argv[0]);\n@@ -118,5 +116,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tif (ret > 127)\n \t\tret = 127;\n \n+cleanup:\n+\tfor (i = 0; i < 3; i++)\n+\t\tfree(mmfs[i].ptr);\n+\n \treturn ret;\n }\ndiff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh\nindex 2f421d967ab..1a7082323dd 100755\n--- a/t/t6403-merge-file.sh\n+++ b/t/t6403-merge-file.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='RCS merge replacement: merge-file'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458376","messageId":"patch-v2-10.11-481d006378b-20220701T104017Z-avarab@gmail.com","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"[PATCH v2 10/11] cat-file: fix a common \"struct object_context\" memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-01T10:42:59Z","receivedAt":"2022-07-01T10:43:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak where \"cat-file\" will leak the \"path\" member. See\ne5fba602e59 (textconv: support for cat_file, 2010-06-15) for the code\nthat introduced the offending get_oid_with_context() call (called\nget_sha1_with_context() at the time).\n\nAs a result we can mark several tests as passing with SANITIZE=leak\nusing \"TEST_PASSES_SANITIZE_LEAK=true\".\n\nAs noted in dc944b65f1d (get_sha1_with_context: dynamically allocate\noc->path, 2017-05-19) callers must free the \"path\" member. That same\ncommit added the relevant free() to this function, but we weren't\ncatching cases where we'd return early.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/cat-file.c | 32 +++++++++++++++++++---------\n t/t0028-working-tree-encoding.sh | 1 +\n t/t1051-large-conversion.sh | 2 ++\n t/t3304-notes-mixed.sh | 1 +\n t/t4044-diff-index-unique-abbrev.sh | 2 ++\n t/t4140-apply-ita.sh | 1 +\n t/t5314-pack-cycle-detection.sh | 4 ++--\n t/t6422-merge-rename-corner-cases.sh | 1 +\n t/t8007-cat-file-textconv.sh | 2 ++\n t/t8010-cat-file-filters.sh | 2 ++\n t/t9104-git-svn-follow-parent.sh | 1 -\n t/t9132-git-svn-broken-symlink.sh | 1 -\n t/t9301-fast-import-notes.sh | 1 +\n 13 files changed, 37 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex ac0459f96be..f42782e955f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -71,6 +71,7 @@ static int stream_blob(const struct object_id *oid)\n static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\t\tint unknown_type)\n {\n+\tint ret;\n \tstruct object_id oid;\n \tenum object_type type;\n \tchar *buf;\n@@ -106,7 +107,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tif (sb.len) {\n \t\t\tprintf(\"%s\\n\", sb.buf);\n \t\t\tstrbuf_release(&sb);\n-\t\t\treturn 0;\n+\t\t\tret = 0;\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tbreak;\n \n@@ -115,7 +117,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n-\t\treturn 0;\n+\t\tret = 0;\n+\t\tgoto cleanup;\n \n \tcase 'e':\n \t\treturn !has_object_file(&oid);\n@@ -123,8 +126,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \tcase 'w':\n \n \t\tif (filter_object(path, obj_context.mode,\n-\t\t\t\t &oid, &buf, &size))\n-\t\t\treturn -1;\n+\t\t\t\t &oid, &buf, &size)) {\n+\t\t\tret = -1;\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbreak;\n \n \tcase 'c':\n@@ -143,11 +148,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\t\tconst char *ls_args[3] = { NULL };\n \t\t\tls_args[0] = \"ls-tree\";\n \t\t\tls_args[1] = obj_name;\n-\t\t\treturn cmd_ls_tree(2, ls_args, NULL);\n+\t\t\tret = cmd_ls_tree(2, ls_args, NULL);\n+\t\t\tgoto cleanup;\n \t\t}\n \n-\t\tif (type == OBJ_BLOB)\n-\t\t\treturn stream_blob(&oid);\n+\t\tif (type == OBJ_BLOB) {\n+\t\t\tret = stream_blob(&oid);\n+\t\t\tgoto cleanup;\n+\t\t}\n \t\tbuf = read_object_file(&oid, &type, &size);\n \t\tif (!buf)\n \t\t\tdie(\"Cannot read object %s\", obj_name);\n@@ -172,8 +180,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\t\t} else\n \t\t\t\toidcpy(&blob_oid, &oid);\n \n-\t\t\tif (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB)\n-\t\t\t\treturn stream_blob(&blob_oid);\n+\t\t\tif (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) {\n+\t\t\t\tret = stream_blob(&blob_oid);\n+\t\t\t\tgoto cleanup;\n+\t\t\t}\n \t\t\t/*\n \t\t\t * we attempted to dereference a tag to a blob\n \t\t\t * and failed; there may be new dereference\n@@ -193,9 +203,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tdie(\"git cat-file %s: bad file\", obj_name);\n \n \twrite_or_die(1, buf, size);\n+\tret = 0;\n+cleanup:\n \tfree(buf);\n \tfree(obj_context.path);\n-\treturn 0;\n+\treturn ret;\n }\n \n struct expand_data {\ndiff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh\nindex 82905a2156f..416eeabdb94 100755\n--- a/t/t0028-working-tree-encoding.sh\n+++ b/t/t0028-working-tree-encoding.sh\n@@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-encoding.sh\"\n \ndiff --git a/t/t1051-large-conversion.sh b/t/t1051-large-conversion.sh\nindex 042b0e44292..f6709c9f569 100755\n--- a/t/t1051-large-conversion.sh\n+++ b/t/t1051-large-conversion.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test conversion filters on large files'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n set_attr() {\ndiff --git a/t/t3304-notes-mixed.sh b/t/t3304-notes-mixed.sh\nindex 03dfcd3954c..2c3a2452668 100755\n--- a/t/t3304-notes-mixed.sh\n+++ b/t/t3304-notes-mixed.sh\n@@ -5,6 +5,7 @@ test_description='Test notes trees that also contain non-notes'\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 number_of_commits=100\ndiff --git a/t/t4044-diff-index-unique-abbrev.sh b/t/t4044-diff-index-unique-abbrev.sh\nindex 4701796d10e..29e49d22902 100755\n--- a/t/t4044-diff-index-unique-abbrev.sh\n+++ b/t/t4044-diff-index-unique-abbrev.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test unique sha1 abbreviation on \"index from..to\" line'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh\nindex c614eaf04cc..b375aca0d74 100755\n--- a/t/t4140-apply-ita.sh\n+++ b/t/t4140-apply-ita.sh\n@@ -2,6 +2,7 @@\n \n test_description='git apply of i-t-a file'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t5314-pack-cycle-detection.sh b/t/t5314-pack-cycle-detection.sh\nindex 0aec8619e22..73a241743aa 100755\n--- a/t/t5314-pack-cycle-detection.sh\n+++ b/t/t5314-pack-cycle-detection.sh\n@@ -49,9 +49,9 @@ Then no matter which order we start looking at the packs in, we know that we\n will always find a delta for \"file\", because its lookup will always come\n immediately after the lookup for \"dummy\".\n '\n-. ./test-lib.sh\n-\n \n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n \n # Create a pack containing the tree $1 and blob $1:file, with\n # the latter stored as a delta against $2:file.\ndiff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh\nindex bf4ce3c63d4..9b65768aed6 100755\n--- a/t/t6422-merge-rename-corner-cases.sh\n+++ b/t/t6422-merge-rename-corner-cases.sh\n@@ -6,6 +6,7 @@ test_description=\"recursive merge corner cases w/ renames but not criss-crosses\"\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-merge.sh\n \ndiff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh\nindex b067983ba1c..c8266f17f14 100755\n--- a/t/t8007-cat-file-textconv.sh\n+++ b/t/t8007-cat-file-textconv.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='git cat-file textconv support'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n cat >helper <<'EOF'\ndiff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh\nindex 31de4b64dc0..ca04242ca01 100755\n--- a/t/t8010-cat-file-filters.sh\n+++ b/t/t8010-cat-file-filters.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='git cat-file filters support'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup ' '\ndiff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh\nindex 5cf2ef4b8b0..85d735861fc 100755\n--- a/t/t9104-git-svn-follow-parent.sh\n+++ b/t/t9104-git-svn-follow-parent.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn fetching'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n test_expect_success 'initialize repo' '\ndiff --git a/t/t9132-git-svn-broken-symlink.sh b/t/t9132-git-svn-broken-symlink.sh\nindex 4d8d0584b79..aeceffaf7b0 100755\n--- a/t/t9132-git-svn-broken-symlink.sh\n+++ b/t/t9132-git-svn-broken-symlink.sh\n@@ -2,7 +2,6 @@\n \n test_description='test that git handles an svn repository with empty symlinks'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n test_expect_success 'load svn dumpfile' '\n \tsvnadmin load \"$rawsvnrepo\" <\n---\n builtin/pull.c | 16 ++++++++++------\n t/t5524-pull-msg.sh | 1 +\n t/t6417-merge-ours-theirs.sh | 1 +\n t/t9101-git-svn-props.sh | 1 -\n 4 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 01155ba67b2..403a24d7ca6 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -990,6 +990,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tint rebase_unspecified = 0;\n \tint can_ff;\n \tint divergent;\n+\tint ret;\n \n \tif (!getenv(\"GIT_REFLOG_ACTION\"))\n \t\tset_reflog_message(argc, argv);\n@@ -1100,7 +1101,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \tif (is_null_oid(&orig_head)) {\n \t\tif (merge_heads.nr > 1)\n \t\t\tdie(_(\"Cannot merge multiple branches into empty head.\"));\n-\t\treturn pull_into_void(merge_heads.oid, &curr_head);\n+\t\tret = pull_into_void(merge_heads.oid, &curr_head);\n+\t\tgoto cleanup;\n \t}\n \tif (merge_heads.nr > 1) {\n \t\tif (opt_rebase)\n@@ -1125,8 +1127,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (opt_rebase) {\n-\t\tint ret = 0;\n-\n \t\tstruct object_id newbase;\n \t\tstruct object_id upstream;\n \t\tget_rebase_newbase_and_upstream(&newbase, &upstream, &curr_head,\n@@ -1149,12 +1149,16 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\t\t recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))\n \t\t\tret = rebase_submodules();\n \n-\t\treturn ret;\n+\t\tgoto cleanup;\n \t} else {\n-\t\tint ret = run_merge();\n+\t\tret = run_merge();\n \t\tif (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||\n \t\t\t recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))\n \t\t\tret = update_submodules();\n-\t\treturn ret;\n+\t\tgoto cleanup;\n \t}\n+\n+cleanup:\n+\toid_array_clear(&merge_heads);\n+\treturn ret;\n }\ndiff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\nindex b2be3605f5a..56716e29ddf 100755\n--- a/t/t5524-pull-msg.sh\n+++ b/t/t5524-pull-msg.sh\n@@ -2,6 +2,7 @@\n \n test_description='git pull message generation'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n dollar='$Dollar'\ndiff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh\nindex 62d1406119e..482b73a998f 100755\n--- a/t/t6417-merge-ours-theirs.sh\n+++ b/t/t6417-merge-ours-theirs.sh\n@@ -4,6 +4,7 @@ test_description='Merge-recursive ours and theirs variants'\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/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh\nindex d043e80fc34..52046e60d51 100755\n--- a/t/t9101-git-svn-props.sh\n+++ b/t/t9101-git-svn-props.sh\n@@ -5,7 +5,6 @@\n \n test_description='git svn property tests'\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n mkdir import\n-- \n2.37.0.900.g4d0de1cceb2\n\n"},{"id":"458408","messageId":"xmqqh7408xsj.fsf@gitster.g","threadId":"58092","inReplyTo":"cover-v2-00.11-00000000000-20220701T104017Z-avarab@gmail.com","subject":"Re: [PATCH v2 00/11] built-ins: fix common memory leaks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-01T18:58:52Z","receivedAt":"2022-07-01T18:58:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> * Replace 8/11, maybe that solution is going overboard, we could also\n> just drop it from this series...\n\nThe \"solution\" part is to forbid merge_working_tree() to use\nanything that relies on the topts before it bails out due to\nunmerged_cache(), that could be done with only a minor reordering of\nthe logic.\n\nAnd I think that is a reasonable thing to do---if we are looking at\nan unmerged index, we would not want to run unpack-trees on it at\nall, so setting up the topts structure does not make much sense\nbefore we fully know we cannot bail out early.\n\nOn the other hand, it does make the patch go near overboard to\nextract the topt initialization logic for this single user into a\nstatic helper function. If we later find out that my \"that is a\nreasonable thing to do\" above is not true, or some setting of topts\nmembers need to become conditional, we would probably need to undo\nthat part of this patch.\n\nThe latter \"going overboard\" part is the majority of change. We\ndefinitely should *not* discard the essence of this step to stop\nleaking, but should think about stopping at the smaller change,\nperhaps?\n\nHaving said that, all the other patches in the series looked good.\n\nThanks, will queue.\n\n"},{"id":"458415","messageId":"xmqqbku88try.fsf@gitster.g","threadId":"58092","inReplyTo":"patch-v2-08.11-e21d7e4e9df-20220701T104017Z-avarab@gmail.com","subject":"Re: [PATCH v2 08/11] checkout: avoid \"struct unpack_trees_options\" leak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-01T20:25:37Z","receivedAt":"2022-07-01T20:25:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 2eefda81d8c..1109f1301f4 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -710,6 +710,26 @@ static void setup_branch_path(struct branch_info *branch)\n> \tbranch->path = strbuf_detach(&buf, NULL);\n> }\n> \n> +static void init_topts(struct unpack_trees_options *topts, int merge,\n> +\t\t int show_progress, int overwrite_ignore,\n> +\t\t struct commit *old_commit)\n> +{\n> +\tmemset(topts, 0, sizeof(*topts));\n> +\ttopts->head_idx = -1;\n> +\ttopts->src_index = &the_index;\n> +\ttopts->dst_index = &the_index;\n> +\n> +\tsetup_unpack_trees_porcelain(topts, \"checkout\");\n> +\n> +\ttopts->initial_checkout = is_cache_unborn();\n> +\ttopts->update = 1;\n> +\ttopts->merge = 1;\n> +\ttopts->quiet = merge && old_commit;\n> +\ttopts->verbose_update = show_progress;\n> +\ttopts->fn = twoway_merge;\n> +\ttopts->preserve_ignored = !overwrite_ignore;\n> +}\n> +\n\nI've already expressed my opinion on this step in my response to\n[00/11], but I'd say we take the patch as-is. It is pretty much Meh\nbetween moving some code that is only used once to a helper function\nthat is called from only one places, and not doing so at all. Once\nthe code is written in one way, it is not worth the bandwidth and\nbrain cycles to replace it with a variant, the difference with which\nis mostly irrelevant.\n\nTHanks.\n\n"}]}