{"thread":{"id":"57498","subject":"[PATCH 00/14] tree-wide: small fixes for memory leaks","startedAt":"2022-03-02T17:10:30Z","lastAt":"2022-03-04T19:11:29Z","messageCount":37,"participants":["Ævar Arnfjörð Bjarmason","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":14},"messages":[{"id":"450114","messageId":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":null,"subject":"[PATCH 00/14] tree-wide: small fixes for memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:06Z","receivedAt":"2022-03-02T17:10:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a collection of various otherwise unrelated tree-wide fixes\nfor memory leaks.\n\nIn fixing more targeted memory leaks in specific areas I've run into\nsmall leaks here & there. Rather than submit e.g. a 2-series patch for\njust small bundle leaks, the same for range-diff etc. I thought it\nmade sense to submit these together.\n\nÆvar Arnfjörð Bjarmason (14):\n  index-pack: fix memory leaks\n  merge-base: free() allocated \"struct commit **\" list\n  diff.c: free \"buf\" in diff_words_flush()\n  urlmatch.c: add and use a *_release() function\n  remote-curl.c: free memory in cmd_main()\n  bundle: call strvec_clear() on allocated strvec\n  transport: stop needlessly copying bundle header references\n  submodule--helper: fix trivial leak in module_add()\n  commit-graph: fix memory leak in misused string_list API\n  commit-graph: stop fill_oids_from_packs() progress on error and free()\n  lockfile API users: simplify and don't leak \"path\"\n  range-diff: plug memory leak in common invocation\n  range-diff: plug memory leak in read_patches()\n  repository.c: free the \"path cache\" in repo_clear()\n\n apply.c                     |  7 ++++++-\n apply.h                     |  2 ++\n builtin/bundle.c            |  1 +\n builtin/commit-graph.c      |  6 +++---\n builtin/config.c            |  2 +-\n builtin/index-pack.c        |  5 +++++\n builtin/merge-base.c        |  5 ++++-\n builtin/sparse-checkout.c   |  3 +--\n builtin/submodule--helper.c |  5 ++++-\n commit-graph.c              | 18 +++++++++++-------\n commit-graph.h              |  2 +-\n credential.c                |  1 +\n diff.c                      |  1 +\n path.h                      | 14 --------------\n range-diff.c                | 30 +++++++++++++-----------------\n remote-curl.c               | 12 ++++++++----\n repository.c                | 16 ++++++++++++++++\n repository.h                | 14 +++++++++++++-\n transport.c                 | 27 ++++++++++++++++-----------\n urlmatch.c                  |  5 +++++\n urlmatch.h                  |  1 +\n 21 files changed, 113 insertions(+), 64 deletions(-)\n\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450115","messageId":"patch-01.14-bcba06e1d28-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 01/14] index-pack: fix memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:07Z","receivedAt":"2022-03-02T17:10:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix various memory leaks in \"git index-pack\", due to how tightly\ncoupled this command is with the revision walking this doesn't make\nany new tests pass, but e.g. this now passes, and had several failures before:\n\n    ./t5300-pack-object.sh --run=1-2,4,6-27,30-42\n\nit is a bit odd that we'll free \"opts.anomaly\", since the \"opts\" is a\n\"struct pack_idx_option\" declared in pack.h. In pack-write.c there's a\nreset_pack_idx_option(), but it only wipes the contents, but doesn't\nfree() anything.\n\nDoing this here in cmd_index_pack() is correct because while the\nstruct is declared in pack.h, this code in builtin/index-pack.c (in\nread_v2_anomalous_offsets()) is what allocates the \"opts.anomaly\", so\nwe should also free it here.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/index-pack.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 3c2e6aee3cc..5fe1adb3681 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1109,6 +1109,7 @@ static void *threaded_second_pass(void *data)\n \t\t\tlist_add(&child->list, &work_head);\n \t\t\tbase_cache_used += child->size;\n \t\t\tprune_base_data(NULL);\n+\t\t\tfree_base_data(child);\n \t\t} else {\n \t\t\t/*\n \t\t\t * This child does not have its own children. It may be\n@@ -1131,6 +1132,7 @@ static void *threaded_second_pass(void *data)\n \n \t\t\t\tp = next_p;\n \t\t\t}\n+\t\t\tFREE_AND_NULL(child);\n \t\t}\n \t\twork_unlock();\n \t}\n@@ -1424,6 +1426,7 @@ static void fix_unresolved_deltas(struct hashfile *f)\n \t\t * object).\n \t\t */\n \t\tappend_obj_to_pack(f, d->oid.hash, data, size, type);\n+\t\tfree(data);\n \t\tthreaded_second_pass(NULL);\n \n \t\tdisplay_progress(progress, nr_resolved_deltas);\n@@ -1703,6 +1706,7 @@ static void show_pack_info(int stat_only)\n \t\t\t  i + 1,\n \t\t\t  chain_histogram[i]);\n \t}\n+\tfree(chain_histogram);\n }\n \n int cmd_index_pack(int argc, const char **argv, const char *prefix)\n@@ -1932,6 +1936,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (do_fsck_object && fsck_finish(&fsck_options))\n \t\tdie(_(\"fsck error in pack objects\"));\n \n+\tfree(opts.anomaly);\n \tfree(objects);\n \tstrbuf_release(&index_name_buf);\n \tstrbuf_release(&rev_index_name_buf);\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450116","messageId":"patch-02.14-4c28f056ec2-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 02/14] merge-base: free() allocated \"struct commit **\" list","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:08Z","receivedAt":"2022-03-02T17:10:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in 53eda89b2fe (merge-base: teach \"git merge-base\"\nto drive underlying merge_bases_many(), 2008-07-30) by calling free()\non the \"struct commit **\" list used by \"git merge-base\".\n\nThis gets e.g. \"t6010-merge-base.sh\" closer to passing under\nSANITIZE=leak, it failed 8 tests before when compiled with that\noption, and now fails only 5 tests.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/merge-base.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex 26b84980dbd..a11f8c6e4bb 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -138,6 +138,7 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tint rev_nr = 0;\n \tint show_all = 0;\n \tint cmdmode = 0;\n+\tint ret;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL('a', \"all\", &show_all, N_(\"output all common ancestors\")),\n@@ -186,5 +187,7 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tALLOC_ARRAY(rev, argc);\n \twhile (argc-- > 0)\n \t\trev[rev_nr++] = get_commit_reference(*argv++);\n-\treturn show_merge_base(rev, rev_nr, show_all);\n+\tret = show_merge_base(rev, rev_nr, show_all);\n+\tfree(rev);\n+\treturn ret;\n }\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450117","messageId":"patch-03.14-5d2793039ad-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 03/14] diff.c: free \"buf\" in diff_words_flush()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:09Z","receivedAt":"2022-03-02T17:10:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the freeing logic added in e6e045f8031 (diff.c: buffer all\noutput if asked to, 2017-06-29) to free the containing \"buf\" in\naddition to its members.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n diff.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/diff.c b/diff.c\nindex c4ccb6b1a34..c5bc9bc5128 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2150,6 +2150,7 @@ static void diff_words_flush(struct emit_callback *ecbdata)\n \n \t\tfor (i = 0; i < wol->nr; i++)\n \t\t\tfree((void *)wol->buf[i].line);\n+\t\tfree(wol->buf);\n \n \t\twol->nr = 0;\n \t}\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450118","messageId":"patch-05.14-8891fd44c7c-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 05/14] remote-curl.c: free memory in cmd_main()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:11Z","receivedAt":"2022-03-02T17:10:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Plug a trivial memory leak in code added in a2d725b7bdf (Use an\nexternal program to implement fetching with curl, 2009-08-05).\n\nTo do this have the cmd_main() use a \"goto cleanup\" pattern, and to\nreturn an error of 1 unless we can fall through to the http_cleanup()\nat the end.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n remote-curl.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 0dabef2dd7c..ff44f41011e 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1472,11 +1472,12 @@ int cmd_main(int argc, const char **argv)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint nongit;\n+\tint ret = 1;\n \n \tsetup_git_directory_gently(&nongit);\n \tif (argc < 2) {\n \t\terror(_(\"remote-curl: usage: git remote-curl <remote> [<url>]\"));\n-\t\treturn 1;\n+\t\tgoto cleanup;\n \t}\n \n \toptions.verbosity = 1;\n@@ -1508,7 +1509,7 @@ int cmd_main(int argc, const char **argv)\n \t\tif (strbuf_getline_lf(&buf, stdin) == EOF) {\n \t\t\tif (ferror(stdin))\n \t\t\t\terror(_(\"remote-curl: error reading command stream from git\"));\n-\t\t\treturn 1;\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tif (buf.len == 0)\n \t\t\tbreak;\n@@ -1556,12 +1557,15 @@ int cmd_main(int argc, const char **argv)\n \t\t\t\tbreak;\n \t\t} else {\n \t\t\terror(_(\"remote-curl: unknown command '%s' from git\"), buf.buf);\n-\t\t\treturn 1;\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tstrbuf_reset(&buf);\n \t} while (1);\n \n \thttp_cleanup();\n+\tret = 0;\n+cleanup:\n+\tstrbuf_release(&buf);\n \n-\treturn 0;\n+\treturn ret;\n }\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450119","messageId":"patch-04.14-7f7077e8476-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 04/14] urlmatch.c: add and use a *_release() function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:10Z","receivedAt":"2022-03-02T17:10:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Plug a memory leak in credential_apply_config() by adding and using a\nnew urlmatch_config_release() function. This just does a\nstring_list_clear() on the \"vars\" member.\n\nThis finished up work on normalizing the init/free pattern in this\nAPI, started in 73ee449bbf2 (urlmatch.[ch]: add and use\nURLMATCH_CONFIG_INIT, 2021-10-01).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/config.c | 2 +-\n credential.c     | 1 +\n urlmatch.c       | 5 +++++\n urlmatch.h       | 1 +\n 4 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 542d8d02b2b..ebec61868be 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -612,7 +612,7 @@ static int get_urlmatch(const char *var, const char *url)\n \n \t\tstrbuf_release(&matched->value);\n \t}\n-\tstring_list_clear(&config.vars, 1);\n+\turlmatch_config_release(&config);\n \tstring_list_clear(&values, 1);\n \tfree(config.url.url);\n \ndiff --git a/credential.c b/credential.c\nindex e7240f3f636..f6389a50684 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -130,6 +130,7 @@ static void credential_apply_config(struct credential *c)\n \tgit_config(urlmatch_config_entry, &config);\n \tstring_list_clear(&config.vars, 1);\n \tfree(normalized_url);\n+\turlmatch_config_release(&config);\n \tstrbuf_release(&url);\n \n \tc->configured = 1;\ndiff --git a/urlmatch.c b/urlmatch.c\nindex 03ad3f30a9c..b615adc923a 100644\n--- a/urlmatch.c\n+++ b/urlmatch.c\n@@ -611,3 +611,8 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)\n \tstrbuf_release(&synthkey);\n \treturn retval;\n }\n+\n+void urlmatch_config_release(struct urlmatch_config *config)\n+{\n+\tstring_list_clear(&config->vars, 1);\n+}\ndiff --git a/urlmatch.h b/urlmatch.h\nindex 34a3ba6d197..9f40b00bfb8 100644\n--- a/urlmatch.h\n+++ b/urlmatch.h\n@@ -71,5 +71,6 @@ struct urlmatch_config {\n }\n \n int urlmatch_config_entry(const char *var, const char *value, void *cb);\n+void urlmatch_config_release(struct urlmatch_config *config);\n \n #endif /* URL_MATCH_H */\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450120","messageId":"patch-06.14-52e2c2a8281-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 06/14] bundle: call strvec_clear() on allocated strvec","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:12Z","receivedAt":"2022-03-02T17:10:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fixing this small memory leak in cmd_bundle_create() gets\n\"t5607-clone-bundle.sh\" closer to passing under SANITIZE=leak.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/bundle.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 5a85d7cd0fe..2adad545a2e 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -93,6 +93,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n \tif (!startup_info->have_repository)\n \t\tdie(_(\"Need a repository to create a bundle.\"));\n \tret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version);\n+\tstrvec_clear(&pack_opts);\n \tfree(bundle_file);\n \treturn ret;\n }\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450121","messageId":"patch-08.14-122fdf7bb41-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 08/14] submodule--helper: fix trivial leak in module_add()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:14Z","receivedAt":"2022-03-02T17:10:42Z","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 a6226fd772b (submodule--helper:\nconvert the bulk of cmd_add() to C, 2021-08-10). If \"realrepo\" isn't a\ncopy of the \"repo\" member we should free() it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/submodule--helper.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex eeacefcc383..13b4841327d 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3309,6 +3309,7 @@ static int module_add(int argc, const char **argv, const char *prefix)\n {\n \tint force = 0, quiet = 0, progress = 0, dissociate = 0;\n \tstruct add_data add_data = ADD_DATA_INIT;\n+\tchar *to_free = NULL;\n \n \tstruct option options[] = {\n \t\tOPT_STRING('b', \"branch\", &add_data.branch, N_(\"branch\"),\n@@ -3360,7 +3361,8 @@ static int module_add(int argc, const char **argv, const char *prefix)\n \t\t\t      \"of the working tree\"));\n \n \t\t/* dereference source url relative to parent's url */\n-\t\tadd_data.realrepo = resolve_relative_url(add_data.repo, NULL, 1);\n+\t\tto_free = resolve_relative_url(add_data.repo, NULL, 1);\n+\t\tadd_data.realrepo = to_free;\n \t} else if (is_dir_sep(add_data.repo[0]) || strchr(add_data.repo, ':')) {\n \t\tadd_data.realrepo = add_data.repo;\n \t} else {\n@@ -3413,6 +3415,7 @@ static int module_add(int argc, const char **argv, const char *prefix)\n \t}\n \tconfigure_added_submodule(&add_data);\n \tfree(add_data.sm_path);\n+\tfree(to_free);\n \n \treturn 0;\n }\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450122","messageId":"patch-07.14-be62ca89bf5-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 07/14] transport: stop needlessly copying bundle header references","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:13Z","receivedAt":"2022-03-02T17:10:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the logic added in fddf2ebe388 (transport: teach all vtables to\nallow fetch first, 2019-08-21) and save ourselves pointless work in\nfetch_refs_from_bundle().\n\nThe fetch_refs_from_bundle() caller doesn't care about the \"struct\nref *result\" return value of get_refs_from_bundle(), and doesn't need\nany of the work we were doing in looping over the\n\"data->header.references\" in get_refs_from_bundle().\n\nSo this change saves us work, and also fixes a memory leak that we had\nwhen called from fetch_refs_from_bundle(). The other caller of\nget_refs_from_bundle() is the \"get_refs_list\" member we set up for the\n\"struct transport_vtable bundle_vtable\". That caller does care about\nthe \"struct ref *result\" return value.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n transport.c | 27 ++++++++++++++++-----------\n 1 file changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 253d6671b1f..9a7d32b4b7d 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -125,6 +125,19 @@ struct bundle_transport_data {\n \tunsigned get_refs_from_bundle_called : 1;\n };\n \n+static void get_refs_from_bundle_inner(struct transport *transport)\n+{\n+\tstruct bundle_transport_data *data = transport->data;\n+\n+\tif (data->fd > 0)\n+\t\tclose(data->fd);\n+\tdata->fd = read_bundle_header(transport->url, &data->header);\n+\tif (data->fd < 0)\n+\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n+\n+\ttransport->hash_algo = data->header.hash_algo;\n+}\n+\n static struct ref *get_refs_from_bundle(struct transport *transport,\n \t\t\t\t\tint for_push,\n \t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n@@ -136,15 +149,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport,\n \tif (for_push)\n \t\treturn NULL;\n \n-\tdata->get_refs_from_bundle_called = 1;\n-\n-\tif (data->fd > 0)\n-\t\tclose(data->fd);\n-\tdata->fd = read_bundle_header(transport->url, &data->header);\n-\tif (data->fd < 0)\n-\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n-\n-\ttransport->hash_algo = data->header.hash_algo;\n+\tget_refs_from_bundle_inner(transport);\n \n \tfor (i = 0; i < data->header.references.nr; i++) {\n \t\tstruct string_list_item *e = data->header.references.items + i;\n@@ -168,8 +173,8 @@ static int fetch_refs_from_bundle(struct transport *transport,\n \tif (transport->progress)\n \t\tstrvec_push(&extra_index_pack_args, \"-v\");\n \n-\tif (!data->get_refs_from_bundle_called)\n-\t\tget_refs_from_bundle(transport, 0, NULL);\n+\tif (!data->get_refs_from_bundle_called++)\n+\t\tget_refs_from_bundle_inner(transport);\n \tret = unbundle(the_repository, &data->header, data->fd,\n \t\t       &extra_index_pack_args);\n \ttransport->hash_algo = data->header.hash_algo;\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450123","messageId":"patch-09.14-b5512deb26f-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 09/14] commit-graph: fix memory leak in misused string_list API","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:15Z","receivedAt":"2022-03-02T17:10:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When this code was migrated to the string_list API in\nd88b14b3fd6 (commit-graph: use string-list API for input, 2018-06-27)\nit was made to use used both STRING_LIST_INIT_NODUP and a\nstrbuf_detach() pattern.\n\nThose should not be used together if string_list_clear() is expected\nto free the memory, instead we need to either use STRING_LIST_INIT_DUP\nwith a string_list_append_nodup(), or a STRING_LIST_INIT_NODUP and\nmanually fiddle with the \"strdup_strings\" member before calling\nstring_list_clear(). Let's do the former.\n\nSince \"strdup_strings = 1\" is set now other code might be broken by\nrelying on \"pack_indexes\" not to duplicate it strings, but that\ndoesn't happen. When we pass this down to write_commit_graph() that\ncode uses the \"struct string_list\" without modifying it. Let's add a\n\"const\" to the variable to have the compiler enforce that assumption.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/commit-graph.c | 6 +++---\n commit-graph.c         | 4 ++--\n commit-graph.h         | 2 +-\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 4247fbde95a..51c4040ea6c 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -192,7 +192,7 @@ static int git_commit_graph_write_config(const char *var, const char *value,\n \n static int graph_write(int argc, const char **argv)\n {\n-\tstruct string_list pack_indexes = STRING_LIST_INIT_NODUP;\n+\tstruct string_list pack_indexes = STRING_LIST_INIT_DUP;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct oidset commits = OIDSET_INIT;\n \tstruct object_directory *odb = NULL;\n@@ -273,8 +273,8 @@ static int graph_write(int argc, const char **argv)\n \n \tif (opts.stdin_packs) {\n \t\twhile (strbuf_getline(&buf, stdin) != EOF)\n-\t\t\tstring_list_append(&pack_indexes,\n-\t\t\t\t\t   strbuf_detach(&buf, NULL));\n+\t\t\tstring_list_append_nodup(&pack_indexes,\n+\t\t\t\t\t\t strbuf_detach(&buf, NULL));\n \t} else if (opts.stdin_commits) {\n \t\toidset_init(&commits, 0);\n \t\tif (opts.progress)\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 265c010122e..d0c94600bab 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1679,7 +1679,7 @@ int write_commit_graph_reachable(struct object_directory *odb,\n }\n \n static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n-\t\t\t\tstruct string_list *pack_indexes)\n+\t\t\t\tconst struct string_list *pack_indexes)\n {\n \tuint32_t i;\n \tstruct strbuf progress_title = STRBUF_INIT;\n@@ -2259,7 +2259,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)\n }\n \n int write_commit_graph(struct object_directory *odb,\n-\t\t       struct string_list *pack_indexes,\n+\t\t       const struct string_list *const pack_indexes,\n \t\t       struct oidset *commits,\n \t\t       enum commit_graph_write_flags flags,\n \t\t       const struct commit_graph_opts *opts)\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 04a94e18302..2e3ac35237e 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -142,7 +142,7 @@ int write_commit_graph_reachable(struct object_directory *odb,\n \t\t\t\t enum commit_graph_write_flags flags,\n \t\t\t\t const struct commit_graph_opts *opts);\n int write_commit_graph(struct object_directory *odb,\n-\t\t       struct string_list *pack_indexes,\n+\t\t       const struct string_list *pack_indexes,\n \t\t       struct oidset *commits,\n \t\t       enum commit_graph_write_flags flags,\n \t\t       const struct commit_graph_opts *opts);\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450124","messageId":"patch-10.14-27f0883e8d8-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 10/14] commit-graph: stop fill_oids_from_packs() progress on error and free()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:16Z","receivedAt":"2022-03-02T17:10:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a bug in fill_oids_from_packs(), we should always stop_progress(),\nbut did not do so if we returned an error here. This also plugs a\nmemory leak in those cases by releasing the two \"struct strbuf\"\nvariables the function uses.\n\nWhile I'm at it stop hardcoding \"-1\" here and just use the return\nvalue of error() instead, which happens to be \"-1\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n commit-graph.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex d0c94600bab..aab0b292774 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1685,6 +1685,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \tstruct strbuf progress_title = STRBUF_INIT;\n \tstruct strbuf packname = STRBUF_INIT;\n \tint dirlen;\n+\tint ret = 0;\n \n \tstrbuf_addf(&packname, \"%s/pack/\", ctx->odb->path);\n \tdirlen = packname.len;\n@@ -1703,12 +1704,12 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \t\tstrbuf_addstr(&packname, pack_indexes->items[i].string);\n \t\tp = add_packed_git(packname.buf, packname.len, 1);\n \t\tif (!p) {\n-\t\t\terror(_(\"error adding pack %s\"), packname.buf);\n-\t\t\treturn -1;\n+\t\t\tret = error(_(\"error adding pack %s\"), packname.buf);\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tif (open_pack_index(p)) {\n-\t\t\terror(_(\"error opening index for %s\"), packname.buf);\n-\t\t\treturn -1;\n+\t\t\tret = error(_(\"error opening index for %s\"), packname.buf);\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tfor_each_object_in_pack(p, add_packed_commits, ctx,\n \t\t\t\t\tFOR_EACH_OBJECT_PACK_ORDER);\n@@ -1716,11 +1717,12 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \t\tfree(p);\n \t}\n \n+cleanup:\n \tstop_progress(&ctx->progress);\n \tstrbuf_release(&progress_title);\n \tstrbuf_release(&packname);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static int fill_oids_from_commits(struct write_commit_graph_context *ctx,\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450125","messageId":"patch-11.14-cc8beed10be-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 11/14] lockfile API users: simplify and don't leak \"path\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:17Z","receivedAt":"2022-03-02T17:10:56Z","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 6c622f9f0bb (commit-graph: write\ncommit-graph chains, 2019-06-18). We needed to free the \"lock_name\" if\nwe encounter errors, and the \"graph_name\" after we'd run unlink() on\nit.\n\nFor the case of write_commit_graph_file() refactoring the code to free\nthe \"lock_name\" after we were done using the \"struct lock_file lk\"\nwould have made the control flow more complex. Luckily we can free the\n\"lock_file\" right after the hold_lock_file_for_update() call, if it\nmakes use of \"path\" at all it'll have copied its contents to a \"struct\nstrbuf\" of its own.\n\nWhile I'm at it let's fix code added in fb10ca5b543 (sparse-checkout:\nwrite using lockfile, 2019-11-21) in write_patterns_and_update() to\navoid the same complexity that I thought I needed when I wrote the\ninitial fix for write_commit_graph_file(). We can free the\n\"sparse_filename\" right after calling hold_lock_file_for_update(), we\ndon't need to wait until we're exiting the function to do so.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/sparse-checkout.c | 3 +--\n commit-graph.c            | 2 ++\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 9c338d33ea2..270ad49c2b8 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -328,11 +328,11 @@ static int write_patterns_and_update(struct pattern_list *pl)\n \n \tfd = hold_lock_file_for_update(&lk, sparse_filename,\n \t\t\t\t      LOCK_DIE_ON_ERROR);\n+\tfree(sparse_filename);\n \n \tresult = update_working_directory(pl);\n \tif (result) {\n \t\trollback_lock_file(&lk);\n-\t\tfree(sparse_filename);\n \t\tclear_pattern_list(pl);\n \t\tupdate_working_directory(NULL);\n \t\treturn result;\n@@ -348,7 +348,6 @@ static int write_patterns_and_update(struct pattern_list *pl)\n \tfflush(fp);\n \tcommit_lock_file(&lk);\n \n-\tfree(sparse_filename);\n \tclear_pattern_list(pl);\n \n \treturn 0;\ndiff --git a/commit-graph.c b/commit-graph.c\nindex aab0b292774..b8cde7ea27d 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1854,6 +1854,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)\n \n \t\thold_lock_file_for_update_mode(&lk, lock_name,\n \t\t\t\t\t       LOCK_DIE_ON_ERROR, 0444);\n+\t\tfree(lock_name);\n \n \t\tfd = git_mkstemp_mode(ctx->graph_name, 0444);\n \t\tif (fd < 0) {\n@@ -1978,6 +1979,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)\n \t\t} else {\n \t\t\tchar *graph_name = get_commit_graph_filename(ctx->odb);\n \t\t\tunlink(graph_name);\n+\t\t\tfree(graph_name);\n \t\t}\n \n \t\tctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(hash_to_hex(file_hash));\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450126","messageId":"patch-12.14-6d13c2530db-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 12/14] range-diff: plug memory leak in common invocation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:18Z","receivedAt":"2022-03-02T17:10:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Create a public release_patch() version of the private free_patch()\nfunction added in 13b5af22f39 (apply: move libified code from\nbuiltin/apply.c to apply.{c,h}, 2016-04-22). Unlike the existing\nfunction this one doesn't free() the \"struct patch\" itself, so we can\nuse it for variables on the stack.\n\nUse it in range-diff.c to fix a memory leak in common range-diff\ninvocations, e.g.:\n\n    git -P range-diff origin/master origin/next origin/seen\n\nWould emit several errors when compiled with SANITIZE=leak, but now\nruns cleanly.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n apply.c      | 7 ++++++-\n apply.h      | 2 ++\n range-diff.c | 1 +\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex 0912307bd91..01f91816428 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -219,13 +219,18 @@ static void free_fragment_list(struct fragment *list)\n \t}\n }\n \n-static void free_patch(struct patch *patch)\n+void release_patch(struct patch *patch)\n {\n \tfree_fragment_list(patch->fragments);\n \tfree(patch->def_name);\n \tfree(patch->old_name);\n \tfree(patch->new_name);\n \tfree(patch->result);\n+}\n+\n+static void free_patch(struct patch *patch)\n+{\n+\trelease_patch(patch);\n \tfree(patch);\n }\n \ndiff --git a/apply.h b/apply.h\nindex 4052da50c06..b9f18ce87d1 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -173,6 +173,8 @@ int parse_git_diff_header(struct strbuf *root,\n \t\t\t  unsigned int size,\n \t\t\t  struct patch *patch);\n \n+void release_patch(struct patch *patch);\n+\n /*\n  * Some aspects of the apply behavior are controlled by the following\n  * bits in the \"options\" parameter passed to apply_all_patches().\ndiff --git a/range-diff.c b/range-diff.c\nindex 30a4de5c2d8..b2a2961f521 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -165,6 +165,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\t\t\t    patch.old_mode, patch.new_mode);\n \n \t\t\tstrbuf_addstr(&buf, \" ##\");\n+\t\t\trelease_patch(&patch);\n \t\t} else if (in_header) {\n \t\t\tif (starts_with(line, \"Author: \")) {\n \t\t\t\tstrbuf_addstr(&buf, \" ## Metadata ##\\n\");\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450127","messageId":"patch-14.14-954de5191c3-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 14/14] repository.c: free the \"path cache\" in repo_clear()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:20Z","receivedAt":"2022-03-02T17:11:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"struct path_cache\" added in 102de880d24 (path.c: migrate global\ngit_path_* to take a repository argument, 2018-05-17) is only used\ndirectly by code in repository.[ch] (but populated in path.[ch]).\n\nLet's move this code to repository.[ch], and stop leaking this memory\nwhen we run repo_clear(). To avoid the cast change it from a \"const\nchar *\" to a \"char *\".\n\nThis also removes the \"PATH_CACHE_INIT\" macro, which has never been\nused for anything. For the \"struct repository\" we already make a hard\nassumption that it (and \"the_repository\") can be identically\ninitialized by making it a \"static\" variable, so making use of a\n\"PATH_CACHE_INIT\" somewhere would have been confusing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n path.h       | 14 --------------\n repository.c | 16 ++++++++++++++++\n repository.h | 14 +++++++++++++-\n 3 files changed, 29 insertions(+), 15 deletions(-)\n\ndiff --git a/path.h b/path.h\nindex b68691a86b8..0a59c85a62e 100644\n--- a/path.h\n+++ b/path.h\n@@ -169,20 +169,6 @@ void report_linked_checkout_garbage(void);\n \t\treturn r->cached_paths.var; \\\n \t}\n \n-struct path_cache {\n-\tconst char *squash_msg;\n-\tconst char *merge_msg;\n-\tconst char *merge_rr;\n-\tconst char *merge_mode;\n-\tconst char *merge_head;\n-\tconst char *merge_autostash;\n-\tconst char *auto_merge;\n-\tconst char *fetch_head;\n-\tconst char *shallow;\n-};\n-\n-#define PATH_CACHE_INIT { 0 }\n-\n const char *git_path_squash_msg(struct repository *r);\n const char *git_path_merge_msg(struct repository *r);\n const char *git_path_merge_rr(struct repository *r);\ndiff --git a/repository.c b/repository.c\nindex 34610c5a33e..9b86f3f1214 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -240,6 +240,20 @@ int repo_submodule_init(struct repository *subrepo,\n \treturn ret;\n }\n \n+static void repo_clear_path_cache(struct repo_path_cache *cache)\n+{\n+\tFREE_AND_NULL(cache->squash_msg);\n+\tFREE_AND_NULL(cache->squash_msg);\n+\tFREE_AND_NULL(cache->merge_msg);\n+\tFREE_AND_NULL(cache->merge_rr);\n+\tFREE_AND_NULL(cache->merge_mode);\n+\tFREE_AND_NULL(cache->merge_head);\n+\tFREE_AND_NULL(cache->merge_autostash);\n+\tFREE_AND_NULL(cache->auto_merge);\n+\tFREE_AND_NULL(cache->fetch_head);\n+\tFREE_AND_NULL(cache->shallow);\n+}\n+\n void repo_clear(struct repository *repo)\n {\n \tFREE_AND_NULL(repo->gitdir);\n@@ -280,6 +294,8 @@ void repo_clear(struct repository *repo)\n \t\tremote_state_clear(repo->remote_state);\n \t\tFREE_AND_NULL(repo->remote_state);\n \t}\n+\n+\trepo_clear_path_cache(&repo->cached_paths);\n }\n \n int repo_read_index(struct repository *repo)\ndiff --git a/repository.h b/repository.h\nindex ca837cb9e91..e29f361703d 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -44,6 +44,18 @@ struct repo_settings {\n \tint core_multi_pack_index;\n };\n \n+struct repo_path_cache {\n+\tchar *squash_msg;\n+\tchar *merge_msg;\n+\tchar *merge_rr;\n+\tchar *merge_mode;\n+\tchar *merge_head;\n+\tchar *merge_autostash;\n+\tchar *auto_merge;\n+\tchar *fetch_head;\n+\tchar *shallow;\n+};\n+\n struct repository {\n \t/* Environment */\n \t/*\n@@ -82,7 +94,7 @@ struct repository {\n \t/*\n \t * Contains path to often used file names.\n \t */\n-\tstruct path_cache cached_paths;\n+\tstruct repo_path_cache cached_paths;\n \n \t/*\n \t * Path to the repository's graft file.\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450128","messageId":"patch-13.14-e7b823f70c8-20220302T170718Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH 13/14] range-diff: plug memory leak in read_patches()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:10:19Z","receivedAt":"2022-03-02T17:11:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend code added in d9c66f0b5bf (range-diff: first rudimentary\nimplementation, 2018-08-13) to use a \"goto cleanup\" pattern. This\nmakes for less code, and frees memory that we'd previously leak.\n\nThe reason for changing free(util) to FREE_AND_NULL(util) is because\nat the end of the function we append the contents of \"util\" to a\n\"struct string_list\" if it's non-NULL.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n range-diff.c | 29 ++++++++++++-----------------\n 1 file changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex b2a2961f521..b72eb9fdbee 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -40,6 +40,7 @@ static int read_patches(const char *range, struct string_list *list,\n \tchar *line, *current_filename = NULL;\n \tssize_t len;\n \tsize_t size;\n+\tint ret = -1;\n \n \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n@@ -68,10 +69,10 @@ static int read_patches(const char *range, struct string_list *list,\n \tif (strbuf_read(&contents, cp.out, 0) < 0) {\n \t\terror_errno(_(\"could not read `log` output\"));\n \t\tfinish_command(&cp);\n-\t\treturn -1;\n+\t\tgoto cleanup;\n \t}\n \tif (finish_command(&cp))\n-\t\treturn -1;\n+\t\tgoto cleanup;\n \n \tline = contents.buf;\n \tsize = contents.len;\n@@ -95,12 +96,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tCALLOC_ARRAY(util, 1);\n \t\t\tif (get_oid(p, &util->oid)) {\n \t\t\t\terror(_(\"could not parse commit '%s'\"), p);\n-\t\t\t\tfree(util);\n-\t\t\t\tfree(current_filename);\n+\t\t\t\tFREE_AND_NULL(util);\n \t\t\t\tstring_list_clear(list, 1);\n-\t\t\t\tstrbuf_release(&buf);\n-\t\t\t\tstrbuf_release(&contents);\n-\t\t\t\treturn -1;\n+\t\t\t\tgoto cleanup;\n \t\t\t}\n \t\t\tutil->matching = -1;\n \t\t\tin_header = 1;\n@@ -111,11 +109,8 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\terror(_(\"could not parse first line of `log` output: \"\n \t\t\t\t\"did not start with 'commit ': '%s'\"),\n \t\t\t      line);\n-\t\t\tfree(current_filename);\n \t\t\tstring_list_clear(list, 1);\n-\t\t\tstrbuf_release(&buf);\n-\t\t\tstrbuf_release(&contents);\n-\t\t\treturn -1;\n+\t\t\tgoto cleanup;\n \t\t}\n \n \t\tif (starts_with(line, \"diff --git\")) {\n@@ -136,12 +131,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tif (len < 0) {\n \t\t\t\terror(_(\"could not parse git header '%.*s'\"),\n \t\t\t\t      orig_len, line);\n-\t\t\t\tfree(util);\n-\t\t\t\tfree(current_filename);\n+\t\t\t\tFREE_AND_NULL(util);\n \t\t\t\tstring_list_clear(list, 1);\n-\t\t\t\tstrbuf_release(&buf);\n-\t\t\t\tstrbuf_release(&contents);\n-\t\t\t\treturn -1;\n+\t\t\t\tgoto cleanup;\n \t\t\t}\n \t\t\tstrbuf_addstr(&buf, \" ## \");\n \t\t\tif (patch.is_new > 0)\n@@ -219,6 +211,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\tstrbuf_addch(&buf, '\\n');\n \t\tutil->diffsize++;\n \t}\n+\n+\tret = 0;\n+cleanup:\n \tstrbuf_release(&contents);\n \n \tif (util)\n@@ -226,7 +221,7 @@ static int read_patches(const char *range, struct string_list *list,\n \tstrbuf_release(&buf);\n \tfree(current_filename);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static int patch_util_cmp(const void *dummy, const struct patch_util *a,\n-- \n2.35.1.1228.g56895c6ee86\n\n"},{"id":"450166","messageId":"e3ad8853-a33d-a70b-4ace-92873c9d1401@github.com","threadId":"57498","inReplyTo":"patch-01.14-bcba06e1d28-20220302T170718Z-avarab@gmail.com","subject":"Re: [PATCH 01/14] index-pack: fix memory leaks","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-02T20:36:36Z","receivedAt":"2022-03-02T20:36:42Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2022 12:10 PM, Ævar Arnfjörð Bjarmason wrote:\n> Fix various memory leaks in \"git index-pack\", due to how tightly\n> coupled this command is with the revision walking this doesn't make\n> any new tests pass, but e.g. this now passes, and had several failures before:\n> \n>     ./t5300-pack-object.sh --run=1-2,4,6-27,30-42\n\nDo you mean that these tests now pass under leak check?\n \n> it is a bit odd that we'll free \"opts.anomaly\", since the \"opts\" is a\n\ns/it/It/\n\n> \"struct pack_idx_option\" declared in pack.h. In pack-write.c there's a\n> reset_pack_idx_option(), but it only wipes the contents, but doesn't\n> free() anything.\n> \n> Doing this here in cmd_index_pack() is correct because while the\n> struct is declared in pack.h, this code in builtin/index-pack.c (in\n> read_v2_anomalous_offsets()) is what allocates the \"opts.anomaly\", so\n> we should also free it here.\n\nMakes sense. Code diff looks good.\n\nThanks,\n-Stolee\n"},{"id":"450169","messageId":"dfa87840-e04b-972f-2468-f7f0afa0ad61@github.com","threadId":"57498","inReplyTo":"patch-07.14-be62ca89bf5-20220302T170718Z-avarab@gmail.com","subject":"Re: [PATCH 07/14] transport: stop needlessly copying bundle header references","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-02T20:47:09Z","receivedAt":"2022-03-02T20:47:18Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2022 12:10 PM, Ævar Arnfjörð Bjarmason wrote:\n> Amend the logic added in fddf2ebe388 (transport: teach all vtables to\n> allow fetch first, 2019-08-21) and save ourselves pointless work in\n> fetch_refs_from_bundle().\n...\n> +static void get_refs_from_bundle_inner(struct transport *transport)\n> +{\n> +\tstruct bundle_transport_data *data = transport->data;\n> +\n> +\tif (data->fd > 0)\n> +\t\tclose(data->fd);\n> +\tdata->fd = read_bundle_header(transport->url, &data->header);\n> +\tif (data->fd < 0)\n> +\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n> +\n> +\ttransport->hash_algo = data->header.hash_algo;\n> +}\n> +\n\nThere are some subtle changes here that look odd, but it actually\ncould present a behavior change.\n\n>  static struct ref *get_refs_from_bundle(struct transport *transport,\n>  \t\t\t\t\tint for_push,\n>  \t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n> @@ -136,15 +149,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport,\n>  \tif (for_push)\n>  \t\treturn NULL;\n>  \n> -\tdata->get_refs_from_bundle_called = 1;\n> -\n> -\tif (data->fd > 0)\n> -\t\tclose(data->fd);\n> -\tdata->fd = read_bundle_header(transport->url, &data->header);\n> -\tif (data->fd < 0)\n> -\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n> -\n> -\ttransport->hash_algo = data->header.hash_algo;\n> +\tget_refs_from_bundle_inner(transport);\n\nThe implementation of get_refs_from_bundle_inner() is very close\nto these deleted lines, except you removed\n\n\tdata->get_refs_from_bundle_called = 1;\n\nand instead you added this change:\n\n> -\tif (!data->get_refs_from_bundle_called)\n> -\t\tget_refs_from_bundle(transport, 0, NULL);\n> +\tif (!data->get_refs_from_bundle_called++)\n> +\t\tget_refs_from_bundle_inner(transport);\n\nSo, this is checking to see if data->get_refs_from_bundle_called\n_was_ zero while also incrementing it. However, this member is\nan unsigned:1, so you're really toggling it on and off each time\nwe reach this 'if' statement.\n\nUsing this \"++\" would still bother me stylistically, even if the\ntype was uint64_t and the risk of overflow was minimal. If you\nput the \"= 1\" line into the inner method, then things go back to\nworking as they did before.\n\nThanks,\n-Stolee\n"},{"id":"450171","messageId":"1ff00775-c6da-2616-4032-9eb3a453669c@github.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"Re: [PATCH 00/14] tree-wide: small fixes for memory leaks","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-02T20:58:28Z","receivedAt":"2022-03-02T20:58:33Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2022 12:10 PM, Ævar Arnfjörð Bjarmason wrote:\n> This is a collection of various otherwise unrelated tree-wide fixes\n> for memory leaks.\n> \n> In fixing more targeted memory leaks in specific areas I've run into\n> small leaks here & there. Rather than submit e.g. a 2-series patch for\n> just small bundle leaks, the same for range-diff etc. I thought it\n> made sense to submit these together.\n\nI read these patches, and they were quite small, so it wasn't hard.\nThe most difficult thing was going into the code just to get more\ncontext than the patches allowed.\n\nThank you for fixing all of those memory leaks I introduced in the\ncommit-graph code.\n\nI had a few nitpick comments on the first commit message, but I\nthink what I pointed out in patch 7 might be a problem that needs\nfixing before this merges.\n\nThanks,\n-Stolee\n"},{"id":"450285","messageId":"220303.86r17j2ekz.gmgdl@evledraar.gmail.com","threadId":"57498","inReplyTo":"e3ad8853-a33d-a70b-4ace-92873c9d1401@github.com","subject":"Re: [PATCH 01/14] index-pack: fix memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-03T16:19:01Z","receivedAt":"2022-03-03T16:20:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 02 2022, Derrick Stolee wrote:\n\n> On 3/2/2022 12:10 PM, Ævar Arnfjörð Bjarmason wrote:\n>> Fix various memory leaks in \"git index-pack\", due to how tightly\n>> coupled this command is with the revision walking this doesn't make\n>> any new tests pass, but e.g. this now passes, and had several failures before:\n>> \n>>     ./t5300-pack-object.sh --run=1-2,4,6-27,30-42\n>\n> Do you mean that these tests now pass under leak check?\n\nYes, i.e. test 3, 5 etc. fails before & after, and this change makes\ntests in these ranges pass.\n\n>> it is a bit odd that we'll free \"opts.anomaly\", since the \"opts\" is a\n>\n> s/it/It/\n\nThanks, will fix!\n\n>> \"struct pack_idx_option\" declared in pack.h. In pack-write.c there's a\n>> reset_pack_idx_option(), but it only wipes the contents, but doesn't\n>> free() anything.\n>> \n>> Doing this here in cmd_index_pack() is correct because while the\n>> struct is declared in pack.h, this code in builtin/index-pack.c (in\n>> read_v2_anomalous_offsets()) is what allocates the \"opts.anomaly\", so\n>> we should also free it here.\n>\n> Makes sense. Code diff looks good.\n\nThanks!\n"},{"id":"450286","messageId":"220303.86mti72eiv.gmgdl@evledraar.gmail.com","threadId":"57498","inReplyTo":"dfa87840-e04b-972f-2468-f7f0afa0ad61@github.com","subject":"Re: [PATCH 07/14] transport: stop needlessly copying bundle header references","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-03T16:20:49Z","receivedAt":"2022-03-03T16:21:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 02 2022, Derrick Stolee wrote:\n\n> On 3/2/2022 12:10 PM, Ævar Arnfjörð Bjarmason wrote:\n>> Amend the logic added in fddf2ebe388 (transport: teach all vtables to\n>> allow fetch first, 2019-08-21) and save ourselves pointless work in\n>> fetch_refs_from_bundle().\n> ...\n>> +static void get_refs_from_bundle_inner(struct transport *transport)\n>> +{\n>> +\tstruct bundle_transport_data *data = transport->data;\n>> +\n>> +\tif (data->fd > 0)\n>> +\t\tclose(data->fd);\n>> +\tdata->fd = read_bundle_header(transport->url, &data->header);\n>> +\tif (data->fd < 0)\n>> +\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n>> +\n>> +\ttransport->hash_algo = data->header.hash_algo;\n>> +}\n>> +\n>\n> There are some subtle changes here that look odd, but it actually\n> could present a behavior change.\n>\n>>  static struct ref *get_refs_from_bundle(struct transport *transport,\n>>  \t\t\t\t\tint for_push,\n>>  \t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n>> @@ -136,15 +149,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport,\n>>  \tif (for_push)\n>>  \t\treturn NULL;\n>>  \n>> -\tdata->get_refs_from_bundle_called = 1;\n>> -\n>> -\tif (data->fd > 0)\n>> -\t\tclose(data->fd);\n>> -\tdata->fd = read_bundle_header(transport->url, &data->header);\n>> -\tif (data->fd < 0)\n>> -\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n>> -\n>> -\ttransport->hash_algo = data->header.hash_algo;\n>> +\tget_refs_from_bundle_inner(transport);\n>\n> The implementation of get_refs_from_bundle_inner() is very close\n> to these deleted lines, except you removed\n>\n> \tdata->get_refs_from_bundle_called = 1;\n>\n> and instead you added this change:\n>\n>> -\tif (!data->get_refs_from_bundle_called)\n>> -\t\tget_refs_from_bundle(transport, 0, NULL);\n>> +\tif (!data->get_refs_from_bundle_called++)\n>> +\t\tget_refs_from_bundle_inner(transport);\n>\n> So, this is checking to see if data->get_refs_from_bundle_called\n> _was_ zero while also incrementing it. However, this member is\n> an unsigned:1, so you're really toggling it on and off each time\n> we reach this 'if' statement.\n>\n> Using this \"++\" would still bother me stylistically, even if the\n> type was uint64_t and the risk of overflow was minimal. If you\n> put the \"= 1\" line into the inner method, then things go back to\n> working as they did before.\n\nOuch, well spotted! Will fix.\n"},{"id":"450414","messageId":"patch-v2-01.14-f46af9ad13f-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 01/14] index-pack: fix memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:04Z","receivedAt":"2022-03-04T18:32:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix various memory leaks in \"git index-pack\", due to how tightly\ncoupled this command is with the revision walking this doesn't make\nany new tests pass.\n\nBut e.g. this now passes, and had several failures before, i.e. we\nstill have failures in tests 3, 5 etc., which are being skipped here.\n\n    ./t5300-pack-object.sh --run=1-2,4,6-27,30-42\n\nIt is a bit odd that we'll free \"opts.anomaly\", since the \"opts\" is a\n\"struct pack_idx_option\" declared in pack.h. In pack-write.c there's a\nreset_pack_idx_option(), but it only wipes the contents, but doesn't\nfree() anything.\n\nDoing this here in cmd_index_pack() is correct because while the\nstruct is declared in pack.h, this code in builtin/index-pack.c (in\nread_v2_anomalous_offsets()) is what allocates the \"opts.anomaly\", so\nwe should also free it here.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/index-pack.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 3c2e6aee3cc..5fe1adb3681 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1109,6 +1109,7 @@ static void *threaded_second_pass(void *data)\n \t\t\tlist_add(&child->list, &work_head);\n \t\t\tbase_cache_used += child->size;\n \t\t\tprune_base_data(NULL);\n+\t\t\tfree_base_data(child);\n \t\t} else {\n \t\t\t/*\n \t\t\t * This child does not have its own children. It may be\n@@ -1131,6 +1132,7 @@ static void *threaded_second_pass(void *data)\n \n \t\t\t\tp = next_p;\n \t\t\t}\n+\t\t\tFREE_AND_NULL(child);\n \t\t}\n \t\twork_unlock();\n \t}\n@@ -1424,6 +1426,7 @@ static void fix_unresolved_deltas(struct hashfile *f)\n \t\t * object).\n \t\t */\n \t\tappend_obj_to_pack(f, d->oid.hash, data, size, type);\n+\t\tfree(data);\n \t\tthreaded_second_pass(NULL);\n \n \t\tdisplay_progress(progress, nr_resolved_deltas);\n@@ -1703,6 +1706,7 @@ static void show_pack_info(int stat_only)\n \t\t\t  i + 1,\n \t\t\t  chain_histogram[i]);\n \t}\n+\tfree(chain_histogram);\n }\n \n int cmd_index_pack(int argc, const char **argv, const char *prefix)\n@@ -1932,6 +1936,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (do_fsck_object && fsck_finish(&fsck_options))\n \t\tdie(_(\"fsck error in pack objects\"));\n \n+\tfree(opts.anomaly);\n \tfree(objects);\n \tstrbuf_release(&index_name_buf);\n \tstrbuf_release(&rev_index_name_buf);\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450415","messageId":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com","subject":"[PATCH v2 00/14] tree-wide: small fixes for memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:03Z","receivedAt":"2022-03-04T18:32:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a collection of various otherwise unrelated tree-wide fixes\nfor memory leaks. See v1 for a (short) overview:\nhttps://lore.kernel.org/git/cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com/\n\nThis re-roll addresses issues Derrick Stolee noted. There's a trivial\ncommit-message change in 1/14, and a rather trivial (but important)\nchange in how a variable is incremented in 7/14.\n\nThat change to v1 is actually small, but the range-diff is big since\n\"git diff\" picks a different way to \"anchor\" the diff as a result.\n\nÆvar Arnfjörð Bjarmason (14):\n  index-pack: fix memory leaks\n  merge-base: free() allocated \"struct commit **\" list\n  diff.c: free \"buf\" in diff_words_flush()\n  urlmatch.c: add and use a *_release() function\n  remote-curl.c: free memory in cmd_main()\n  bundle: call strvec_clear() on allocated strvec\n  transport: stop needlessly copying bundle header references\n  submodule--helper: fix trivial leak in module_add()\n  commit-graph: fix memory leak in misused string_list API\n  commit-graph: stop fill_oids_from_packs() progress on error and free()\n  lockfile API users: simplify and don't leak \"path\"\n  range-diff: plug memory leak in common invocation\n  range-diff: plug memory leak in read_patches()\n  repository.c: free the \"path cache\" in repo_clear()\n\n apply.c                     |  7 ++++++-\n apply.h                     |  2 ++\n builtin/bundle.c            |  1 +\n builtin/commit-graph.c      |  6 +++---\n builtin/config.c            |  2 +-\n builtin/index-pack.c        |  5 +++++\n builtin/merge-base.c        |  5 ++++-\n builtin/sparse-checkout.c   |  3 +--\n builtin/submodule--helper.c |  5 ++++-\n commit-graph.c              | 18 +++++++++++-------\n commit-graph.h              |  2 +-\n credential.c                |  1 +\n diff.c                      |  1 +\n path.h                      | 14 --------------\n range-diff.c                | 30 +++++++++++++-----------------\n remote-curl.c               | 12 ++++++++----\n repository.c                | 16 ++++++++++++++++\n repository.h                | 14 +++++++++++++-\n transport.c                 | 25 ++++++++++++++++---------\n urlmatch.c                  |  5 +++++\n urlmatch.h                  |  1 +\n 21 files changed, 113 insertions(+), 62 deletions(-)\n\nRange-diff against v1:\n 1:  bcba06e1d28 !  1:  f46af9ad13f index-pack: fix memory leaks\n    @@ Commit message\n     \n         Fix various memory leaks in \"git index-pack\", due to how tightly\n         coupled this command is with the revision walking this doesn't make\n    -    any new tests pass, but e.g. this now passes, and had several failures before:\n    +    any new tests pass.\n    +\n    +    But e.g. this now passes, and had several failures before, i.e. we\n    +    still have failures in tests 3, 5 etc., which are being skipped here.\n     \n             ./t5300-pack-object.sh --run=1-2,4,6-27,30-42\n     \n    -    it is a bit odd that we'll free \"opts.anomaly\", since the \"opts\" is a\n    +    It is a bit odd that we'll free \"opts.anomaly\", since the \"opts\" is a\n         \"struct pack_idx_option\" declared in pack.h. In pack-write.c there's a\n         reset_pack_idx_option(), but it only wipes the contents, but doesn't\n         free() anything.\n 2:  4c28f056ec2 =  2:  4ee2881adfb merge-base: free() allocated \"struct commit **\" list\n 3:  5d2793039ad =  3:  90517a05582 diff.c: free \"buf\" in diff_words_flush()\n 4:  7f7077e8476 =  4:  d51f6ae0963 urlmatch.c: add and use a *_release() function\n 5:  8891fd44c7c =  5:  f0a26db8a87 remote-curl.c: free memory in cmd_main()\n 6:  52e2c2a8281 =  6:  c636770b5d6 bundle: call strvec_clear() on allocated strvec\n 7:  be62ca89bf5 !  7:  b3f7753a790 transport: stop needlessly copying bundle header references\n    @@ transport.c: struct bundle_transport_data {\n      \tunsigned get_refs_from_bundle_called : 1;\n      };\n      \n    +-static struct ref *get_refs_from_bundle(struct transport *transport,\n    +-\t\t\t\t\tint for_push,\n    +-\t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n     +static void get_refs_from_bundle_inner(struct transport *transport)\n    + {\n    + \tstruct bundle_transport_data *data = transport->data;\n    +-\tstruct ref *result = NULL;\n    +-\tint i;\n    +-\n    +-\tif (for_push)\n    +-\t\treturn NULL;\n    + \n    + \tdata->get_refs_from_bundle_called = 1;\n    + \n    +@@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport,\n    + \t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n    + \n    + \ttransport->hash_algo = data->header.hash_algo;\n    ++}\n    ++\n    ++static struct ref *get_refs_from_bundle(struct transport *transport,\n    ++\t\t\t\t\tint for_push,\n    ++\t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n     +{\n     +\tstruct bundle_transport_data *data = transport->data;\n    ++\tstruct ref *result = NULL;\n    ++\tint i;\n     +\n    -+\tif (data->fd > 0)\n    -+\t\tclose(data->fd);\n    -+\tdata->fd = read_bundle_header(transport->url, &data->header);\n    -+\tif (data->fd < 0)\n    -+\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n    -+\n    -+\ttransport->hash_algo = data->header.hash_algo;\n    -+}\n    ++\tif (for_push)\n    ++\t\treturn NULL;\n     +\n    - static struct ref *get_refs_from_bundle(struct transport *transport,\n    - \t\t\t\t\tint for_push,\n    - \t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n    -@@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport,\n    - \tif (for_push)\n    - \t\treturn NULL;\n    - \n    --\tdata->get_refs_from_bundle_called = 1;\n    --\n    --\tif (data->fd > 0)\n    --\t\tclose(data->fd);\n    --\tdata->fd = read_bundle_header(transport->url, &data->header);\n    --\tif (data->fd < 0)\n    --\t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n    --\n    --\ttransport->hash_algo = data->header.hash_algo;\n     +\tget_refs_from_bundle_inner(transport);\n      \n      \tfor (i = 0; i < data->header.references.nr; i++) {\n      \t\tstruct string_list_item *e = data->header.references.items + i;\n     @@ transport.c: static int fetch_refs_from_bundle(struct transport *transport,\n    - \tif (transport->progress)\n      \t\tstrvec_push(&extra_index_pack_args, \"-v\");\n      \n    --\tif (!data->get_refs_from_bundle_called)\n    + \tif (!data->get_refs_from_bundle_called)\n     -\t\tget_refs_from_bundle(transport, 0, NULL);\n    -+\tif (!data->get_refs_from_bundle_called++)\n     +\t\tget_refs_from_bundle_inner(transport);\n      \tret = unbundle(the_repository, &data->header, data->fd,\n      \t\t       &extra_index_pack_args);\n 8:  122fdf7bb41 =  8:  af3ca2f0b5f submodule--helper: fix trivial leak in module_add()\n 9:  b5512deb26f =  9:  3fadb265d13 commit-graph: fix memory leak in misused string_list API\n10:  27f0883e8d8 = 10:  27f5190ce59 commit-graph: stop fill_oids_from_packs() progress on error and free()\n11:  cc8beed10be = 11:  217754edc62 lockfile API users: simplify and don't leak \"path\"\n12:  6d13c2530db = 12:  148382d9529 range-diff: plug memory leak in common invocation\n13:  e7b823f70c8 = 13:  c6e61b85491 range-diff: plug memory leak in read_patches()\n14:  954de5191c3 = 14:  d70a4394f2b repository.c: free the \"path cache\" in repo_clear()\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450416","messageId":"patch-v2-02.14-4ee2881adfb-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 02/14] merge-base: free() allocated \"struct commit **\" list","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:05Z","receivedAt":"2022-03-04T18:32:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a memory leak in 53eda89b2fe (merge-base: teach \"git merge-base\"\nto drive underlying merge_bases_many(), 2008-07-30) by calling free()\non the \"struct commit **\" list used by \"git merge-base\".\n\nThis gets e.g. \"t6010-merge-base.sh\" closer to passing under\nSANITIZE=leak, it failed 8 tests before when compiled with that\noption, and now fails only 5 tests.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/merge-base.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex 26b84980dbd..a11f8c6e4bb 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -138,6 +138,7 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tint rev_nr = 0;\n \tint show_all = 0;\n \tint cmdmode = 0;\n+\tint ret;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL('a', \"all\", &show_all, N_(\"output all common ancestors\")),\n@@ -186,5 +187,7 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tALLOC_ARRAY(rev, argc);\n \twhile (argc-- > 0)\n \t\trev[rev_nr++] = get_commit_reference(*argv++);\n-\treturn show_merge_base(rev, rev_nr, show_all);\n+\tret = show_merge_base(rev, rev_nr, show_all);\n+\tfree(rev);\n+\treturn ret;\n }\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450417","messageId":"patch-v2-03.14-90517a05582-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 03/14] diff.c: free \"buf\" in diff_words_flush()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:06Z","receivedAt":"2022-03-04T18:32:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the freeing logic added in e6e045f8031 (diff.c: buffer all\noutput if asked to, 2017-06-29) to free the containing \"buf\" in\naddition to its members.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n diff.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/diff.c b/diff.c\nindex c4ccb6b1a34..c5bc9bc5128 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2150,6 +2150,7 @@ static void diff_words_flush(struct emit_callback *ecbdata)\n \n \t\tfor (i = 0; i < wol->nr; i++)\n \t\t\tfree((void *)wol->buf[i].line);\n+\t\tfree(wol->buf);\n \n \t\twol->nr = 0;\n \t}\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450418","messageId":"patch-v2-04.14-d51f6ae0963-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 04/14] urlmatch.c: add and use a *_release() function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:07Z","receivedAt":"2022-03-04T18:32:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Plug a memory leak in credential_apply_config() by adding and using a\nnew urlmatch_config_release() function. This just does a\nstring_list_clear() on the \"vars\" member.\n\nThis finished up work on normalizing the init/free pattern in this\nAPI, started in 73ee449bbf2 (urlmatch.[ch]: add and use\nURLMATCH_CONFIG_INIT, 2021-10-01).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/config.c | 2 +-\n credential.c     | 1 +\n urlmatch.c       | 5 +++++\n urlmatch.h       | 1 +\n 4 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 542d8d02b2b..ebec61868be 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -612,7 +612,7 @@ static int get_urlmatch(const char *var, const char *url)\n \n \t\tstrbuf_release(&matched->value);\n \t}\n-\tstring_list_clear(&config.vars, 1);\n+\turlmatch_config_release(&config);\n \tstring_list_clear(&values, 1);\n \tfree(config.url.url);\n \ndiff --git a/credential.c b/credential.c\nindex e7240f3f636..f6389a50684 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -130,6 +130,7 @@ static void credential_apply_config(struct credential *c)\n \tgit_config(urlmatch_config_entry, &config);\n \tstring_list_clear(&config.vars, 1);\n \tfree(normalized_url);\n+\turlmatch_config_release(&config);\n \tstrbuf_release(&url);\n \n \tc->configured = 1;\ndiff --git a/urlmatch.c b/urlmatch.c\nindex 03ad3f30a9c..b615adc923a 100644\n--- a/urlmatch.c\n+++ b/urlmatch.c\n@@ -611,3 +611,8 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)\n \tstrbuf_release(&synthkey);\n \treturn retval;\n }\n+\n+void urlmatch_config_release(struct urlmatch_config *config)\n+{\n+\tstring_list_clear(&config->vars, 1);\n+}\ndiff --git a/urlmatch.h b/urlmatch.h\nindex 34a3ba6d197..9f40b00bfb8 100644\n--- a/urlmatch.h\n+++ b/urlmatch.h\n@@ -71,5 +71,6 @@ struct urlmatch_config {\n }\n \n int urlmatch_config_entry(const char *var, const char *value, void *cb);\n+void urlmatch_config_release(struct urlmatch_config *config);\n \n #endif /* URL_MATCH_H */\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450419","messageId":"patch-v2-05.14-f0a26db8a87-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 05/14] remote-curl.c: free memory in cmd_main()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:08Z","receivedAt":"2022-03-04T18:32:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Plug a trivial memory leak in code added in a2d725b7bdf (Use an\nexternal program to implement fetching with curl, 2009-08-05).\n\nTo do this have the cmd_main() use a \"goto cleanup\" pattern, and to\nreturn an error of 1 unless we can fall through to the http_cleanup()\nat the end.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n remote-curl.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 0dabef2dd7c..ff44f41011e 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1472,11 +1472,12 @@ int cmd_main(int argc, const char **argv)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint nongit;\n+\tint ret = 1;\n \n \tsetup_git_directory_gently(&nongit);\n \tif (argc < 2) {\n \t\terror(_(\"remote-curl: usage: git remote-curl <remote> [<url>]\"));\n-\t\treturn 1;\n+\t\tgoto cleanup;\n \t}\n \n \toptions.verbosity = 1;\n@@ -1508,7 +1509,7 @@ int cmd_main(int argc, const char **argv)\n \t\tif (strbuf_getline_lf(&buf, stdin) == EOF) {\n \t\t\tif (ferror(stdin))\n \t\t\t\terror(_(\"remote-curl: error reading command stream from git\"));\n-\t\t\treturn 1;\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tif (buf.len == 0)\n \t\t\tbreak;\n@@ -1556,12 +1557,15 @@ int cmd_main(int argc, const char **argv)\n \t\t\t\tbreak;\n \t\t} else {\n \t\t\terror(_(\"remote-curl: unknown command '%s' from git\"), buf.buf);\n-\t\t\treturn 1;\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tstrbuf_reset(&buf);\n \t} while (1);\n \n \thttp_cleanup();\n+\tret = 0;\n+cleanup:\n+\tstrbuf_release(&buf);\n \n-\treturn 0;\n+\treturn ret;\n }\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450420","messageId":"patch-v2-06.14-c636770b5d6-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 06/14] bundle: call strvec_clear() on allocated strvec","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:09Z","receivedAt":"2022-03-04T18:32:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fixing this small memory leak in cmd_bundle_create() gets\n\"t5607-clone-bundle.sh\" closer to passing under SANITIZE=leak.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/bundle.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex 5a85d7cd0fe..2adad545a2e 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -93,6 +93,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {\n \tif (!startup_info->have_repository)\n \t\tdie(_(\"Need a repository to create a bundle.\"));\n \tret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version);\n+\tstrvec_clear(&pack_opts);\n \tfree(bundle_file);\n \treturn ret;\n }\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450421","messageId":"patch-v2-07.14-b3f7753a790-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 07/14] transport: stop needlessly copying bundle header references","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:10Z","receivedAt":"2022-03-04T18:32:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the logic added in fddf2ebe388 (transport: teach all vtables to\nallow fetch first, 2019-08-21) and save ourselves pointless work in\nfetch_refs_from_bundle().\n\nThe fetch_refs_from_bundle() caller doesn't care about the \"struct\nref *result\" return value of get_refs_from_bundle(), and doesn't need\nany of the work we were doing in looping over the\n\"data->header.references\" in get_refs_from_bundle().\n\nSo this change saves us work, and also fixes a memory leak that we had\nwhen called from fetch_refs_from_bundle(). The other caller of\nget_refs_from_bundle() is the \"get_refs_list\" member we set up for the\n\"struct transport_vtable bundle_vtable\". That caller does care about\nthe \"struct ref *result\" return value.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n transport.c | 25 ++++++++++++++++---------\n 1 file changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 253d6671b1f..70e9840a90e 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -125,16 +125,9 @@ struct bundle_transport_data {\n \tunsigned get_refs_from_bundle_called : 1;\n };\n \n-static struct ref *get_refs_from_bundle(struct transport *transport,\n-\t\t\t\t\tint for_push,\n-\t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n+static void get_refs_from_bundle_inner(struct transport *transport)\n {\n \tstruct bundle_transport_data *data = transport->data;\n-\tstruct ref *result = NULL;\n-\tint i;\n-\n-\tif (for_push)\n-\t\treturn NULL;\n \n \tdata->get_refs_from_bundle_called = 1;\n \n@@ -145,6 +138,20 @@ static struct ref *get_refs_from_bundle(struct transport *transport,\n \t\tdie(_(\"could not read bundle '%s'\"), transport->url);\n \n \ttransport->hash_algo = data->header.hash_algo;\n+}\n+\n+static struct ref *get_refs_from_bundle(struct transport *transport,\n+\t\t\t\t\tint for_push,\n+\t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n+{\n+\tstruct bundle_transport_data *data = transport->data;\n+\tstruct ref *result = NULL;\n+\tint i;\n+\n+\tif (for_push)\n+\t\treturn NULL;\n+\n+\tget_refs_from_bundle_inner(transport);\n \n \tfor (i = 0; i < data->header.references.nr; i++) {\n \t\tstruct string_list_item *e = data->header.references.items + i;\n@@ -169,7 +176,7 @@ static int fetch_refs_from_bundle(struct transport *transport,\n \t\tstrvec_push(&extra_index_pack_args, \"-v\");\n \n \tif (!data->get_refs_from_bundle_called)\n-\t\tget_refs_from_bundle(transport, 0, NULL);\n+\t\tget_refs_from_bundle_inner(transport);\n \tret = unbundle(the_repository, &data->header, data->fd,\n \t\t       &extra_index_pack_args);\n \ttransport->hash_algo = data->header.hash_algo;\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450422","messageId":"patch-v2-09.14-3fadb265d13-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 09/14] commit-graph: fix memory leak in misused string_list API","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:12Z","receivedAt":"2022-03-04T18:32:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When this code was migrated to the string_list API in\nd88b14b3fd6 (commit-graph: use string-list API for input, 2018-06-27)\nit was made to use used both STRING_LIST_INIT_NODUP and a\nstrbuf_detach() pattern.\n\nThose should not be used together if string_list_clear() is expected\nto free the memory, instead we need to either use STRING_LIST_INIT_DUP\nwith a string_list_append_nodup(), or a STRING_LIST_INIT_NODUP and\nmanually fiddle with the \"strdup_strings\" member before calling\nstring_list_clear(). Let's do the former.\n\nSince \"strdup_strings = 1\" is set now other code might be broken by\nrelying on \"pack_indexes\" not to duplicate it strings, but that\ndoesn't happen. When we pass this down to write_commit_graph() that\ncode uses the \"struct string_list\" without modifying it. Let's add a\n\"const\" to the variable to have the compiler enforce that assumption.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/commit-graph.c | 6 +++---\n commit-graph.c         | 4 ++--\n commit-graph.h         | 2 +-\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 4247fbde95a..51c4040ea6c 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -192,7 +192,7 @@ static int git_commit_graph_write_config(const char *var, const char *value,\n \n static int graph_write(int argc, const char **argv)\n {\n-\tstruct string_list pack_indexes = STRING_LIST_INIT_NODUP;\n+\tstruct string_list pack_indexes = STRING_LIST_INIT_DUP;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct oidset commits = OIDSET_INIT;\n \tstruct object_directory *odb = NULL;\n@@ -273,8 +273,8 @@ static int graph_write(int argc, const char **argv)\n \n \tif (opts.stdin_packs) {\n \t\twhile (strbuf_getline(&buf, stdin) != EOF)\n-\t\t\tstring_list_append(&pack_indexes,\n-\t\t\t\t\t   strbuf_detach(&buf, NULL));\n+\t\t\tstring_list_append_nodup(&pack_indexes,\n+\t\t\t\t\t\t strbuf_detach(&buf, NULL));\n \t} else if (opts.stdin_commits) {\n \t\toidset_init(&commits, 0);\n \t\tif (opts.progress)\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 265c010122e..d0c94600bab 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1679,7 +1679,7 @@ int write_commit_graph_reachable(struct object_directory *odb,\n }\n \n static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n-\t\t\t\tstruct string_list *pack_indexes)\n+\t\t\t\tconst struct string_list *pack_indexes)\n {\n \tuint32_t i;\n \tstruct strbuf progress_title = STRBUF_INIT;\n@@ -2259,7 +2259,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)\n }\n \n int write_commit_graph(struct object_directory *odb,\n-\t\t       struct string_list *pack_indexes,\n+\t\t       const struct string_list *const pack_indexes,\n \t\t       struct oidset *commits,\n \t\t       enum commit_graph_write_flags flags,\n \t\t       const struct commit_graph_opts *opts)\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 04a94e18302..2e3ac35237e 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -142,7 +142,7 @@ int write_commit_graph_reachable(struct object_directory *odb,\n \t\t\t\t enum commit_graph_write_flags flags,\n \t\t\t\t const struct commit_graph_opts *opts);\n int write_commit_graph(struct object_directory *odb,\n-\t\t       struct string_list *pack_indexes,\n+\t\t       const struct string_list *pack_indexes,\n \t\t       struct oidset *commits,\n \t\t       enum commit_graph_write_flags flags,\n \t\t       const struct commit_graph_opts *opts);\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450423","messageId":"patch-v2-12.14-148382d9529-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 12/14] range-diff: plug memory leak in common invocation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:15Z","receivedAt":"2022-03-04T18:32:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Create a public release_patch() version of the private free_patch()\nfunction added in 13b5af22f39 (apply: move libified code from\nbuiltin/apply.c to apply.{c,h}, 2016-04-22). Unlike the existing\nfunction this one doesn't free() the \"struct patch\" itself, so we can\nuse it for variables on the stack.\n\nUse it in range-diff.c to fix a memory leak in common range-diff\ninvocations, e.g.:\n\n    git -P range-diff origin/master origin/next origin/seen\n\nWould emit several errors when compiled with SANITIZE=leak, but now\nruns cleanly.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n apply.c      | 7 ++++++-\n apply.h      | 2 ++\n range-diff.c | 1 +\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex 0912307bd91..01f91816428 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -219,13 +219,18 @@ static void free_fragment_list(struct fragment *list)\n \t}\n }\n \n-static void free_patch(struct patch *patch)\n+void release_patch(struct patch *patch)\n {\n \tfree_fragment_list(patch->fragments);\n \tfree(patch->def_name);\n \tfree(patch->old_name);\n \tfree(patch->new_name);\n \tfree(patch->result);\n+}\n+\n+static void free_patch(struct patch *patch)\n+{\n+\trelease_patch(patch);\n \tfree(patch);\n }\n \ndiff --git a/apply.h b/apply.h\nindex 4052da50c06..b9f18ce87d1 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -173,6 +173,8 @@ int parse_git_diff_header(struct strbuf *root,\n \t\t\t  unsigned int size,\n \t\t\t  struct patch *patch);\n \n+void release_patch(struct patch *patch);\n+\n /*\n  * Some aspects of the apply behavior are controlled by the following\n  * bits in the \"options\" parameter passed to apply_all_patches().\ndiff --git a/range-diff.c b/range-diff.c\nindex 30a4de5c2d8..b2a2961f521 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -165,6 +165,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\t\t\t    patch.old_mode, patch.new_mode);\n \n \t\t\tstrbuf_addstr(&buf, \" ##\");\n+\t\t\trelease_patch(&patch);\n \t\t} else if (in_header) {\n \t\t\tif (starts_with(line, \"Author: \")) {\n \t\t\t\tstrbuf_addstr(&buf, \" ## Metadata ##\\n\");\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450424","messageId":"patch-v2-13.14-c6e61b85491-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 13/14] range-diff: plug memory leak in read_patches()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:16Z","receivedAt":"2022-03-04T18:32:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend code added in d9c66f0b5bf (range-diff: first rudimentary\nimplementation, 2018-08-13) to use a \"goto cleanup\" pattern. This\nmakes for less code, and frees memory that we'd previously leak.\n\nThe reason for changing free(util) to FREE_AND_NULL(util) is because\nat the end of the function we append the contents of \"util\" to a\n\"struct string_list\" if it's non-NULL.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n range-diff.c | 29 ++++++++++++-----------------\n 1 file changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex b2a2961f521..b72eb9fdbee 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -40,6 +40,7 @@ static int read_patches(const char *range, struct string_list *list,\n \tchar *line, *current_filename = NULL;\n \tssize_t len;\n \tsize_t size;\n+\tint ret = -1;\n \n \tstrvec_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t     \"--reverse\", \"--date-order\", \"--decorate=no\",\n@@ -68,10 +69,10 @@ static int read_patches(const char *range, struct string_list *list,\n \tif (strbuf_read(&contents, cp.out, 0) < 0) {\n \t\terror_errno(_(\"could not read `log` output\"));\n \t\tfinish_command(&cp);\n-\t\treturn -1;\n+\t\tgoto cleanup;\n \t}\n \tif (finish_command(&cp))\n-\t\treturn -1;\n+\t\tgoto cleanup;\n \n \tline = contents.buf;\n \tsize = contents.len;\n@@ -95,12 +96,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tCALLOC_ARRAY(util, 1);\n \t\t\tif (get_oid(p, &util->oid)) {\n \t\t\t\terror(_(\"could not parse commit '%s'\"), p);\n-\t\t\t\tfree(util);\n-\t\t\t\tfree(current_filename);\n+\t\t\t\tFREE_AND_NULL(util);\n \t\t\t\tstring_list_clear(list, 1);\n-\t\t\t\tstrbuf_release(&buf);\n-\t\t\t\tstrbuf_release(&contents);\n-\t\t\t\treturn -1;\n+\t\t\t\tgoto cleanup;\n \t\t\t}\n \t\t\tutil->matching = -1;\n \t\t\tin_header = 1;\n@@ -111,11 +109,8 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\terror(_(\"could not parse first line of `log` output: \"\n \t\t\t\t\"did not start with 'commit ': '%s'\"),\n \t\t\t      line);\n-\t\t\tfree(current_filename);\n \t\t\tstring_list_clear(list, 1);\n-\t\t\tstrbuf_release(&buf);\n-\t\t\tstrbuf_release(&contents);\n-\t\t\treturn -1;\n+\t\t\tgoto cleanup;\n \t\t}\n \n \t\tif (starts_with(line, \"diff --git\")) {\n@@ -136,12 +131,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tif (len < 0) {\n \t\t\t\terror(_(\"could not parse git header '%.*s'\"),\n \t\t\t\t      orig_len, line);\n-\t\t\t\tfree(util);\n-\t\t\t\tfree(current_filename);\n+\t\t\t\tFREE_AND_NULL(util);\n \t\t\t\tstring_list_clear(list, 1);\n-\t\t\t\tstrbuf_release(&buf);\n-\t\t\t\tstrbuf_release(&contents);\n-\t\t\t\treturn -1;\n+\t\t\t\tgoto cleanup;\n \t\t\t}\n \t\t\tstrbuf_addstr(&buf, \" ## \");\n \t\t\tif (patch.is_new > 0)\n@@ -219,6 +211,9 @@ static int read_patches(const char *range, struct string_list *list,\n \t\tstrbuf_addch(&buf, '\\n');\n \t\tutil->diffsize++;\n \t}\n+\n+\tret = 0;\n+cleanup:\n \tstrbuf_release(&contents);\n \n \tif (util)\n@@ -226,7 +221,7 @@ static int read_patches(const char *range, struct string_list *list,\n \tstrbuf_release(&buf);\n \tfree(current_filename);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static int patch_util_cmp(const void *dummy, const struct patch_util *a,\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450425","messageId":"patch-v2-11.14-217754edc62-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 11/14] lockfile API users: simplify and don't leak \"path\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:14Z","receivedAt":"2022-03-04T18:33:00Z","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 6c622f9f0bb (commit-graph: write\ncommit-graph chains, 2019-06-18). We needed to free the \"lock_name\" if\nwe encounter errors, and the \"graph_name\" after we'd run unlink() on\nit.\n\nFor the case of write_commit_graph_file() refactoring the code to free\nthe \"lock_name\" after we were done using the \"struct lock_file lk\"\nwould have made the control flow more complex. Luckily we can free the\n\"lock_file\" right after the hold_lock_file_for_update() call, if it\nmakes use of \"path\" at all it'll have copied its contents to a \"struct\nstrbuf\" of its own.\n\nWhile I'm at it let's fix code added in fb10ca5b543 (sparse-checkout:\nwrite using lockfile, 2019-11-21) in write_patterns_and_update() to\navoid the same complexity that I thought I needed when I wrote the\ninitial fix for write_commit_graph_file(). We can free the\n\"sparse_filename\" right after calling hold_lock_file_for_update(), we\ndon't need to wait until we're exiting the function to do so.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/sparse-checkout.c | 3 +--\n commit-graph.c            | 2 ++\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 9c338d33ea2..270ad49c2b8 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -328,11 +328,11 @@ static int write_patterns_and_update(struct pattern_list *pl)\n \n \tfd = hold_lock_file_for_update(&lk, sparse_filename,\n \t\t\t\t      LOCK_DIE_ON_ERROR);\n+\tfree(sparse_filename);\n \n \tresult = update_working_directory(pl);\n \tif (result) {\n \t\trollback_lock_file(&lk);\n-\t\tfree(sparse_filename);\n \t\tclear_pattern_list(pl);\n \t\tupdate_working_directory(NULL);\n \t\treturn result;\n@@ -348,7 +348,6 @@ static int write_patterns_and_update(struct pattern_list *pl)\n \tfflush(fp);\n \tcommit_lock_file(&lk);\n \n-\tfree(sparse_filename);\n \tclear_pattern_list(pl);\n \n \treturn 0;\ndiff --git a/commit-graph.c b/commit-graph.c\nindex aab0b292774..b8cde7ea27d 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1854,6 +1854,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)\n \n \t\thold_lock_file_for_update_mode(&lk, lock_name,\n \t\t\t\t\t       LOCK_DIE_ON_ERROR, 0444);\n+\t\tfree(lock_name);\n \n \t\tfd = git_mkstemp_mode(ctx->graph_name, 0444);\n \t\tif (fd < 0) {\n@@ -1978,6 +1979,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)\n \t\t} else {\n \t\t\tchar *graph_name = get_commit_graph_filename(ctx->odb);\n \t\t\tunlink(graph_name);\n+\t\t\tfree(graph_name);\n \t\t}\n \n \t\tctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(hash_to_hex(file_hash));\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450426","messageId":"patch-v2-14.14-d70a4394f2b-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 14/14] repository.c: free the \"path cache\" in repo_clear()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:17Z","receivedAt":"2022-03-04T18:33:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"struct path_cache\" added in 102de880d24 (path.c: migrate global\ngit_path_* to take a repository argument, 2018-05-17) is only used\ndirectly by code in repository.[ch] (but populated in path.[ch]).\n\nLet's move this code to repository.[ch], and stop leaking this memory\nwhen we run repo_clear(). To avoid the cast change it from a \"const\nchar *\" to a \"char *\".\n\nThis also removes the \"PATH_CACHE_INIT\" macro, which has never been\nused for anything. For the \"struct repository\" we already make a hard\nassumption that it (and \"the_repository\") can be identically\ninitialized by making it a \"static\" variable, so making use of a\n\"PATH_CACHE_INIT\" somewhere would have been confusing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n path.h       | 14 --------------\n repository.c | 16 ++++++++++++++++\n repository.h | 14 +++++++++++++-\n 3 files changed, 29 insertions(+), 15 deletions(-)\n\ndiff --git a/path.h b/path.h\nindex b68691a86b8..0a59c85a62e 100644\n--- a/path.h\n+++ b/path.h\n@@ -169,20 +169,6 @@ void report_linked_checkout_garbage(void);\n \t\treturn r->cached_paths.var; \\\n \t}\n \n-struct path_cache {\n-\tconst char *squash_msg;\n-\tconst char *merge_msg;\n-\tconst char *merge_rr;\n-\tconst char *merge_mode;\n-\tconst char *merge_head;\n-\tconst char *merge_autostash;\n-\tconst char *auto_merge;\n-\tconst char *fetch_head;\n-\tconst char *shallow;\n-};\n-\n-#define PATH_CACHE_INIT { 0 }\n-\n const char *git_path_squash_msg(struct repository *r);\n const char *git_path_merge_msg(struct repository *r);\n const char *git_path_merge_rr(struct repository *r);\ndiff --git a/repository.c b/repository.c\nindex 34610c5a33e..9b86f3f1214 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -240,6 +240,20 @@ int repo_submodule_init(struct repository *subrepo,\n \treturn ret;\n }\n \n+static void repo_clear_path_cache(struct repo_path_cache *cache)\n+{\n+\tFREE_AND_NULL(cache->squash_msg);\n+\tFREE_AND_NULL(cache->squash_msg);\n+\tFREE_AND_NULL(cache->merge_msg);\n+\tFREE_AND_NULL(cache->merge_rr);\n+\tFREE_AND_NULL(cache->merge_mode);\n+\tFREE_AND_NULL(cache->merge_head);\n+\tFREE_AND_NULL(cache->merge_autostash);\n+\tFREE_AND_NULL(cache->auto_merge);\n+\tFREE_AND_NULL(cache->fetch_head);\n+\tFREE_AND_NULL(cache->shallow);\n+}\n+\n void repo_clear(struct repository *repo)\n {\n \tFREE_AND_NULL(repo->gitdir);\n@@ -280,6 +294,8 @@ void repo_clear(struct repository *repo)\n \t\tremote_state_clear(repo->remote_state);\n \t\tFREE_AND_NULL(repo->remote_state);\n \t}\n+\n+\trepo_clear_path_cache(&repo->cached_paths);\n }\n \n int repo_read_index(struct repository *repo)\ndiff --git a/repository.h b/repository.h\nindex ca837cb9e91..e29f361703d 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -44,6 +44,18 @@ struct repo_settings {\n \tint core_multi_pack_index;\n };\n \n+struct repo_path_cache {\n+\tchar *squash_msg;\n+\tchar *merge_msg;\n+\tchar *merge_rr;\n+\tchar *merge_mode;\n+\tchar *merge_head;\n+\tchar *merge_autostash;\n+\tchar *auto_merge;\n+\tchar *fetch_head;\n+\tchar *shallow;\n+};\n+\n struct repository {\n \t/* Environment */\n \t/*\n@@ -82,7 +94,7 @@ struct repository {\n \t/*\n \t * Contains path to often used file names.\n \t */\n-\tstruct path_cache cached_paths;\n+\tstruct repo_path_cache cached_paths;\n \n \t/*\n \t * Path to the repository's graft file.\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450427","messageId":"patch-v2-10.14-27f5190ce59-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 10/14] commit-graph: stop fill_oids_from_packs() progress on error and free()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:13Z","receivedAt":"2022-03-04T18:33:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a bug in fill_oids_from_packs(), we should always stop_progress(),\nbut did not do so if we returned an error here. This also plugs a\nmemory leak in those cases by releasing the two \"struct strbuf\"\nvariables the function uses.\n\nWhile I'm at it stop hardcoding \"-1\" here and just use the return\nvalue of error() instead, which happens to be \"-1\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n commit-graph.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex d0c94600bab..aab0b292774 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1685,6 +1685,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \tstruct strbuf progress_title = STRBUF_INIT;\n \tstruct strbuf packname = STRBUF_INIT;\n \tint dirlen;\n+\tint ret = 0;\n \n \tstrbuf_addf(&packname, \"%s/pack/\", ctx->odb->path);\n \tdirlen = packname.len;\n@@ -1703,12 +1704,12 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \t\tstrbuf_addstr(&packname, pack_indexes->items[i].string);\n \t\tp = add_packed_git(packname.buf, packname.len, 1);\n \t\tif (!p) {\n-\t\t\terror(_(\"error adding pack %s\"), packname.buf);\n-\t\t\treturn -1;\n+\t\t\tret = error(_(\"error adding pack %s\"), packname.buf);\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tif (open_pack_index(p)) {\n-\t\t\terror(_(\"error opening index for %s\"), packname.buf);\n-\t\t\treturn -1;\n+\t\t\tret = error(_(\"error opening index for %s\"), packname.buf);\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tfor_each_object_in_pack(p, add_packed_commits, ctx,\n \t\t\t\t\tFOR_EACH_OBJECT_PACK_ORDER);\n@@ -1716,11 +1717,12 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \t\tfree(p);\n \t}\n \n+cleanup:\n \tstop_progress(&ctx->progress);\n \tstrbuf_release(&progress_title);\n \tstrbuf_release(&packname);\n \n-\treturn 0;\n+\treturn ret;\n }\n \n static int fill_oids_from_commits(struct write_commit_graph_context *ctx,\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450428","messageId":"patch-v2-08.14-af3ca2f0b5f-20220304T182902Z-avarab@gmail.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"[PATCH v2 08/14] submodule--helper: fix trivial leak in module_add()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T18:32:11Z","receivedAt":"2022-03-04T18:33:06Z","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 a6226fd772b (submodule--helper:\nconvert the bulk of cmd_add() to C, 2021-08-10). If \"realrepo\" isn't a\ncopy of the \"repo\" member we should free() it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/submodule--helper.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex eeacefcc383..13b4841327d 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3309,6 +3309,7 @@ static int module_add(int argc, const char **argv, const char *prefix)\n {\n \tint force = 0, quiet = 0, progress = 0, dissociate = 0;\n \tstruct add_data add_data = ADD_DATA_INIT;\n+\tchar *to_free = NULL;\n \n \tstruct option options[] = {\n \t\tOPT_STRING('b', \"branch\", &add_data.branch, N_(\"branch\"),\n@@ -3360,7 +3361,8 @@ static int module_add(int argc, const char **argv, const char *prefix)\n \t\t\t      \"of the working tree\"));\n \n \t\t/* dereference source url relative to parent's url */\n-\t\tadd_data.realrepo = resolve_relative_url(add_data.repo, NULL, 1);\n+\t\tto_free = resolve_relative_url(add_data.repo, NULL, 1);\n+\t\tadd_data.realrepo = to_free;\n \t} else if (is_dir_sep(add_data.repo[0]) || strchr(add_data.repo, ':')) {\n \t\tadd_data.realrepo = add_data.repo;\n \t} else {\n@@ -3413,6 +3415,7 @@ static int module_add(int argc, const char **argv, const char *prefix)\n \t}\n \tconfigure_added_submodule(&add_data);\n \tfree(add_data.sm_path);\n+\tfree(to_free);\n \n \treturn 0;\n }\n-- \n2.35.1.1248.gb68c9165ad8\n\n"},{"id":"450433","messageId":"70a882d2-5a57-696a-b079-ee738df57eaa@github.com","threadId":"57498","inReplyTo":"patch-v2-07.14-b3f7753a790-20220304T182902Z-avarab@gmail.com","subject":"Re: [PATCH v2 07/14] transport: stop needlessly copying bundle header references","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-04T19:10:55Z","receivedAt":"2022-03-04T19:11:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/4/2022 1:32 PM, Ævar Arnfjörð Bjarmason wrote:\n> Amend the logic added in fddf2ebe388 (transport: teach all vtables to\n> allow fetch first, 2019-08-21) and save ourselves pointless work in\n> fetch_refs_from_bundle().\n\n> -static struct ref *get_refs_from_bundle(struct transport *transport,\n> -\t\t\t\t\tint for_push,\n> -\t\t\t\t\tstruct transport_ls_refs_options *transport_options)\n> +static void get_refs_from_bundle_inner(struct transport *transport)\n>  {\n>  \tstruct bundle_transport_data *data = transport->data;\n> -\tstruct ref *result = NULL;\n> -\tint i;\n> -\n> -\tif (for_push)\n> -\t\treturn NULL;\n>  \n>  \tdata->get_refs_from_bundle_called = 1;\n\nThe inclusion of this line dramatically changed the shape of the\ndiff (different areas got selected as adds and deletes) so the\nrange-diff looked strange, but the end result is good.\n  \nThanks,\n-Stolee\n"},{"id":"450434","messageId":"2e32b2cf-0208-1476-57f0-88f68a5936a1@github.com","threadId":"57498","inReplyTo":"cover-v2-00.14-00000000000-20220304T182902Z-avarab@gmail.com","subject":"Re: [PATCH v2 00/14] tree-wide: small fixes for memory leaks","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-04T19:11:19Z","receivedAt":"2022-03-04T19:11:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/4/2022 1:32 PM, Ævar Arnfjörð Bjarmason wrote:\n> This is a collection of various otherwise unrelated tree-wide fixes\n> for memory leaks. See v1 for a (short) overview:\n> https://lore.kernel.org/git/cover-00.14-00000000000-20220302T170718Z-avarab@gmail.com/\n> \n> This re-roll addresses issues Derrick Stolee noted. There's a trivial\n> commit-message change in 1/14, and a rather trivial (but important)\n> change in how a variable is incremented in 7/14.\n> \n> That change to v1 is actually small, but the range-diff is big since\n> \"git diff\" picks a different way to \"anchor\" the diff as a result.\n\nYep. This version looks good to go. Thanks for the re-roll.\n\n-Stolee\n\n"}]}