{"thread":{"id":"61982","subject":"[PATCH 00/20] Memory leak fixes (pt.5)","startedAt":"2024-08-20T14:05:03Z","lastAt":"2024-08-22T19:01:07Z","messageCount":60,"participants":["Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":20},"messages":[{"id":"501353","messageId":"cover.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":null,"subject":"[PATCH 00/20] Memory leak fixes (pt.5)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:04:58Z","receivedAt":"2024-08-20T14:05:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nanother week, another set of memory leak fixes. With this series we're\ndown to ~128 leaking test suites, down from 359 before starting with\nthis effort. So we're about 2/3rds done. Naturally, part 6 is also\nalmost ready.\n\nThis series is built on top of bb9c16bd4f (The sixth batch, 2024-08-19)\nwith Junio's ps/leakfixes-part-4 at 77d4b3dd73 (builtin/diff: free\nsymmetric diff members, 2024-08-14) merged into it.\n\nThanks!\n\nPatrick\n\nPatrick Steinhardt (20):\n mailinfo: fix leaking header data\n convert: fix leaks when resetting attributes\n pretty: fix memory leaks when parsing pretty formats\n pretty: fix leaking key/value separator buffer\n builtin/merge-tree: fix leaking `-X` strategy options\n builtin/upload-archive: fix leaking args passed to `write_archive()`\n builtin/archive: fix leaking `OPT_FILENAME()` value\n midx-write: fix leaking hashfile on error cases\n builtin/repack: fix leaks when computing packs to repack\n t/helper: fix leaking multi-pack-indices in \"read-midx\"\n transport: fix leaking OID arrays in git:// transport data\n builtin/send-pack: fix leaking refspecs\n sideband: fix leaks when configuring sideband colors\n builtin/fetch-pack: fix leaking refs\n remote: fix leaking config strings\n remote: fix leaks when matching refspecs\n remote: fix leaking peer ref when expanding refmap\n builtin/fetch: fix leaking transaction with `--atomic`\n transport: fix leaking arguments when fetching from bundle\n transport: fix leaking negotiation tips\n\n archive.c | 10 ++++++\n builtin/archive.c | 7 ++--\n builtin/fetch-pack.c | 20 ++++++-----\n builtin/fetch.c | 8 ++---\n builtin/merge-tree.c | 13 +++++--\n builtin/repack.c | 36 ++++++++++++++-----\n builtin/send-pack.c | 1 +\n builtin/upload-archive.c | 8 +++--\n convert.c | 3 ++\n mailinfo.c | 17 +++++++--\n midx-write.c | 24 ++++++-------\n pretty.c | 13 +++++--\n remote.c | 55 +++++++++++++++++++++--------\n sideband.c | 8 +++--\n t/helper/test-read-midx.c | 8 ++++-\n t/t4150-am.sh | 1 +\n t/t4205-log-pretty-formats.sh | 2 ++\n t/t4301-merge-tree-write-tree.sh | 1 +\n t/t5000-tar-tree.sh | 1 +\n t/t5003-archive-zip.sh | 1 +\n t/t5100-mailinfo.sh | 1 +\n t/t5319-multi-pack-index.sh | 2 ++\n t/t5400-send-pack.sh | 1 +\n t/t5401-update-hooks.sh | 2 ++\n t/t5408-send-pack-stdin.sh | 2 ++\n t/t5409-colorize-remote-messages.sh | 1 +\n t/t5501-fetch-push-alternates.sh | 1 +\n t/t5505-remote.sh | 1 +\n t/t5510-fetch.sh | 1 +\n t/t5519-push-alternates.sh | 1 +\n t/t5536-fetch-conflicts.sh | 1 +\n t/t5548-push-porcelain.sh | 1 +\n t/t5553-set-upstream.sh | 1 +\n t/t5574-fetch-output.sh | 1 +\n t/t5703-upload-pack-ref-in-want.sh | 1 +\n t/t5812-proto-disable-http.sh | 2 ++\n t/t6050-replace.sh | 1 +\n t/t7704-repack-cruft.sh | 1 +\n transport.c | 8 +++++\n 39 files changed, 203 insertions(+), 64 deletions(-)\n\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501354","messageId":"69e30ea5179eff6472be54ebba64ebca3e562f32.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 01/20] mailinfo: fix leaking header data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:02Z","receivedAt":"2024-08-20T14:05:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We populate the `mailinfo` arrays `p_hdr_data` and `s_hdr_data` with\ndata parsed from the mail headers. These arrays may end up being only\npartially populated with gaps in case some of the headers do not parse\nproperly. This causes memory leaks because `strbuf_list_free()` will\nstop iterating once it hits the first `NULL` pointer in the backing\narray.\n\nFix this by open-coding a variant of `strbuf_list_free()` that knows to\niterate through all headers.\n\nSigned-off-by: Patrick Steinhardt \n---\n mailinfo.c | 17 +++++++++++++++--\n t/t5100-mailinfo.sh | 1 +\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/mailinfo.c b/mailinfo.c\nindex 94b9b0abf22..a4fa64994ac 100644\n--- a/mailinfo.c\n+++ b/mailinfo.c\n@@ -1290,8 +1290,21 @@ void clear_mailinfo(struct mailinfo *mi)\n \tstrbuf_release(&mi->inbody_header_accum);\n \tfree(mi->message_id);\n \n-\tstrbuf_list_free(mi->p_hdr_data);\n-\tstrbuf_list_free(mi->s_hdr_data);\n+\tfor (size_t i = 0; header[i]; i++) {\n+\t\tif (!mi->p_hdr_data[i])\n+\t\t\tcontinue;\n+\t\tstrbuf_release(mi->p_hdr_data[i]);\n+\t\tfree(mi->p_hdr_data[i]);\n+\t}\n+\tfree(mi->p_hdr_data);\n+\n+\tfor (size_t i = 0; header[i]; i++) {\n+\t\tif (!mi->s_hdr_data[i])\n+\t\t\tcontinue;\n+\t\tstrbuf_release(mi->s_hdr_data[i]);\n+\t\tfree(mi->s_hdr_data[i]);\n+\t}\n+\tfree(mi->s_hdr_data);\n \n \twhile (mi->content < mi->content_top) {\n \t\tfree(*(mi->content_top));\ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex c8d06554541..065156c1f39 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -5,6 +5,7 @@\n \n test_description='git mailinfo and git mailsplit test'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n DATA=\"$TEST_DIRECTORY/t5100\"\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501355","messageId":"ed0f01bf92c83fb7371326bd1a59933619de2c47.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 02/20] convert: fix leaks when resetting attributes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:05Z","receivedAt":"2024-08-20T14:05:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When resetting parsed gitattributes, we free the list of convert drivers\nparsed from the config. We only free some of the drivers' fields though\nand thus have memory leaks.\n\nFix this by freeing all allocated convert driver fields to plug these\nmemory leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n convert.c | 3 +++\n t/t4150-am.sh | 1 +\n 2 files changed, 4 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex e6184d21f26..c9a31eb4f03 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1371,6 +1371,9 @@ void reset_parsed_attributes(void)\n \tfor (drv = user_convert; drv; drv = next) {\n \t\tnext = drv->next;\n \t\tfree((void *)drv->name);\n+\t\tfree((void *)drv->smudge);\n+\t\tfree((void *)drv->clean);\n+\t\tfree((void *)drv->process);\n \t\tfree(drv);\n \t}\n \tuser_convert = NULL;\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 5e2b6c80eae..232e1394e8d 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -5,6 +5,7 @@ test_description='git am running'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup: messages' '\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501356","messageId":"82f3908f9620cee29e36a51f6d18ddcc8392b966.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 03/20] pretty: fix memory leaks when parsing pretty formats","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:07Z","receivedAt":"2024-08-20T14:05:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When parsing pretty formats from the config we leak the name and user\nformat whenever these are set multiple times. This is because we do not\nfree any already-set value in case there is one.\n\nPlugging this leak for the name is trivial. For the user format we need\nto be a bit more careful, because we may end up assigning a pointer into\nthe allocated region when the string is prefixed with either \"format\" or\n\"tformat:\". In order to make it safe to unconditionally free the user\nformat we thus strdup the stripped string into the field instead of a\npointer into the string.\n\nSigned-off-by: Patrick Steinhardt \n---\n pretty.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 44222fb83c6..af8f433cdcb 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -63,7 +63,7 @@ static int git_pretty_formats_config(const char *var, const char *value,\n \t\t\t\t void *cb UNUSED)\n {\n \tstruct cmt_fmt_map *commit_format = NULL;\n-\tconst char *name;\n+\tconst char *name, *stripped;\n \tchar *fmt;\n \tint i;\n \n@@ -90,15 +90,21 @@ static int git_pretty_formats_config(const char *var, const char *value,\n \t\tcommit_formats_len++;\n \t}\n \n+\tfree((char *) commit_format->name);\n \tcommit_format->name = xstrdup(name);\n \tcommit_format->format = CMIT_FMT_USERFORMAT;\n \tif (git_config_string(&fmt, var, value))\n \t\treturn -1;\n \n-\tif (skip_prefix(fmt, \"format:\", &commit_format->user_format)) {\n+\tfree((char *) commit_format->user_format);\n+\tif (skip_prefix(fmt, \"format:\", &stripped)) {\n \t\tcommit_format->is_tformat = 0;\n-\t} else if (skip_prefix(fmt, \"tformat:\", &commit_format->user_format)) {\n+\t\tcommit_format->user_format = xstrdup(stripped);\n+\t\tfree(fmt);\n+\t} else if (skip_prefix(fmt, \"tformat:\", &stripped)) {\n \t\tcommit_format->is_tformat = 1;\n+\t\tcommit_format->user_format = xstrdup(stripped);\n+\t\tfree(fmt);\n \t} else if (strchr(fmt, '%')) {\n \t\tcommit_format->is_tformat = 1;\n \t\tcommit_format->user_format = fmt;\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501357","messageId":"696467780e6a1b3d6050987c9305ff48213e306d.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 04/20] pretty: fix leaking key/value separator buffer","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:10Z","receivedAt":"2024-08-20T14:05:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `format_set_trailers_options()` function is responsible for parsing\na custom pretty format for trailers. It puts the parsed options into a\n`struct process_trailer_options` structure, while the allocated memory\nrequired for this will be put into separate caller-provided arguments.\nIt is thus the caller's responsibility to free the memory not via the\noptions structure, but via the other parameters.\n\nWhile we do this alright for the separator and filter keys, we do not\nfree the memory associated with the key/value separator. Fix this to\nplug this memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n pretty.c | 1 +\n t/t4205-log-pretty-formats.sh | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/pretty.c b/pretty.c\nindex af8f433cdcb..6b684d7b828 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1776,6 +1776,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t}\n \ttrailer_out:\n \t\tstring_list_clear(&filter_list, 0);\n+\t\tstrbuf_release(&kvsepbuf);\n \t\tstrbuf_release(&sepbuf);\n \t\treturn ret;\n \t}\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 158b49d4b60..eb63ce011fa 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -5,6 +5,8 @@\n #\n \n test_description='Test pretty formats'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Tested non-UTF-8 encoding\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501358","messageId":"53db2fc7206cc11cae4930eba8c413fce4715afb.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 05/20] builtin/merge-tree: fix leaking `-X` strategy options","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:12Z","receivedAt":"2024-08-20T14:05:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `-X` switch for git-merge-tree(1) will push each option into a local\n`xopts` vector that we then end up parsing. The vector never gets freed\nthough, causing a memory leak. Plug it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/merge-tree.c | 13 ++++++++++---\n t/t4301-merge-tree-write-tree.sh | 1 +\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 9bca9b5f33c..c00469ed3db 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -533,6 +533,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tint expected_remaining_argc;\n \tint original_argc;\n \tconst char *merge_base = NULL;\n+\tint ret;\n \n \tconst char * const merge_tree_usage[] = {\n \t\tN_(\"git merge-tree [--write-tree] [] \"),\n@@ -625,7 +626,9 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tstrbuf_list_free(split);\n \t\t}\n \t\tstrbuf_release(&buf);\n-\t\treturn 0;\n+\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \t/* Figure out which mode to use */\n@@ -664,7 +667,11 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Do the relevant type of merge */\n \tif (o.mode == MODE_REAL)\n-\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n+\t\tret = real_merge(&o, merge_base, argv[0], argv[1], prefix);\n \telse\n-\t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n+\t\tret = trivial_merge(argv[0], argv[1], argv[2]);\n+\n+out:\n+\tstrvec_clear(&xopts);\n+\treturn ret;\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex eea19907b55..37f1cd7364c 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -2,6 +2,7 @@\n \n test_description='git merge-tree --write-tree'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # This test is ort-specific\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501359","messageId":"5b05a3252185d2cc817006ef587a44b423b8bea6.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 06/20] builtin/upload-archive: fix leaking args passed to `write_archive()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:15Z","receivedAt":"2024-08-20T14:05:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In git-upload-archive(1), we pass an array of arguments to\n`write_archive()` to tell it what exactly to do. We don't ever clear the\nvector though, causing a memory leak. Furthermore though, the call to\n`write_archive()` may cause contents of the array to be modified, which\nwould cause us to leak memory to allocated strings held by it.\n\nFix the issue by having `write_archive()` create a shallow copy of\n`argv` before parsing the arguments. Like this, we won't modify the\ncaller's array and can easily `strvec_clear()` it to plug these memory\nleaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n archive.c | 10 ++++++++++\n builtin/upload-archive.c | 8 ++++++--\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 7bd60d0632a..9ba96aae4f7 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -736,6 +736,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tstruct pretty_print_describe_status describe_status = {0};\n \tstruct pretty_print_context ctx = {0};\n \tstruct archiver_args args;\n+\tconst char **argv_copy;\n \tint rc;\n \n \tgit_config_get_bool(\"uploadarchive.allowunreachable\", &remote_allow_unreachable);\n@@ -749,6 +750,14 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \targs.repo = repo;\n \targs.prefix = prefix;\n \tstring_list_init_dup(&args.extra_files);\n+\n+\t/*\n+\t * `parse_archive_args()` modifies contents of `argv`, which is what we\n+\t * want. Our callers may not want it though, so we create a copy here.\n+\t */\n+\tDUP_ARRAY(argv_copy, argv, argc);\n+\targv = argv_copy;\n+\n \targc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n \tif (!startup_info->have_repository) {\n \t\t/*\n@@ -767,6 +776,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tstring_list_clear_func(&args.extra_files, extra_file_info_clear);\n \tfree(args.refname);\n \tclear_pathspec(&args.pathspec);\n+\tfree(argv_copy);\n \n \treturn rc;\n }\ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex 1b09e5e1aa3..313a8dfa81c 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -22,6 +22,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)\n {\n \tstruct strvec sent_argv = STRVEC_INIT;\n \tconst char *arg_cmd = \"argument \";\n+\tint ret;\n \n \tif (argc != 2 || !strcmp(argv[1], \"-h\"))\n \t\tusage(upload_archive_usage);\n@@ -46,8 +47,11 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* parse all options sent by the client */\n-\treturn write_archive(sent_argv.nr, sent_argv.v, prefix,\n-\t\t\t the_repository, NULL, 1);\n+\tret = write_archive(sent_argv.nr, sent_argv.v, prefix,\n+\t\t\t the_repository, NULL, 1);\n+\n+\tstrvec_clear(&sent_argv);\n+\treturn ret;\n }\n \n __attribute__((format (printf, 1, 2)))\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501360","messageId":"bad575df12658ea63fddf082a60be6cb87c34972.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 07/20] builtin/archive: fix leaking `OPT_FILENAME()` value","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:20Z","receivedAt":"2024-08-20T14:05:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"--output\" switch is an `OPT_FILENAME()` option, which allocates\nmemory when specified by the user. But while we free the string when\nexecuted without the \"--remote\" switch, we don't otherwise because we\nreturn via a separate exit path that doesn't know to free it.\n\nFix this by creating a common exit path.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/archive.c | 7 +++++--\n t/t5000-tar-tree.sh | 1 +\n t/t5003-archive-zip.sh | 1 +\n 3 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex b50981504f3..63f02990d11 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -100,13 +100,16 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \tif (output)\n \t\tcreate_output_file(output);\n \n-\tif (remote)\n-\t\treturn run_remote_archiver(argc, argv, remote, exec, output);\n+\tif (remote) {\n+\t\tret = run_remote_archiver(argc, argv, remote, exec, output);\n+\t\tgoto out;\n+\t}\n \n \tsetvbuf(stderr, NULL, _IOLBF, BUFSIZ);\n \n \tret = write_archive(argc, argv, prefix, the_repository, output, 0);\n \n+out:\n \tfree(output);\n \treturn ret;\n }\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 72b8d0ff02e..7abba8a4b20 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -25,6 +25,7 @@ commit id embedding:\n '\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n SUBSTFORMAT=%H%n\ndiff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\nindex 961c6aac256..01f591c99b9 100755\n--- a/t/t5003-archive-zip.sh\n+++ b/t/t5003-archive-zip.sh\n@@ -3,6 +3,7 @@\n test_description='git archive --format=zip test'\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n SUBSTFORMAT=%H%n\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501361","messageId":"5f042ce5098563aa0662026006c356c278dad0b8.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 08/20] midx-write: fix leaking hashfile on error cases","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:23Z","receivedAt":"2024-08-20T14:05:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When writing the MIDX file we first create the `struct hashfile` used to\nwrite the trailer hash, and then afterwards we verify whether we can\nactually write the MIDX in the first place. When we decide that we\ncan't, this leads to a memory leak because we never free the hash file\ncontents.\n\nWe could fix this by freeing the hashfile on the exit path. There is a\nbetter option though: we can simply move the checks for the error\ncondition earlier. As there is no early exit between creating the\nhashfile and finalizing it anymore this is sufficient to fix the memory\nleak.\n\nSigned-off-by: Patrick Steinhardt \n---\n midx-write.c | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/midx-write.c b/midx-write.c\nindex e3fa33203fa..07d98d494aa 100644\n--- a/midx-write.c\n+++ b/midx-write.c\n@@ -1308,6 +1308,18 @@ static int write_midx_internal(const char *object_dir,\n \t\tpack_name_concat_len += MIDX_CHUNK_ALIGNMENT -\n \t\t\t\t\t(pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);\n \n+\tif (ctx.nr - dropped_packs == 0) {\n+\t\terror(_(\"no pack files to index.\"));\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif (!ctx.entries_nr) {\n+\t\tif (flags & MIDX_WRITE_BITMAP)\n+\t\t\twarning(_(\"refusing to write multi-pack .bitmap without any objects\"));\n+\t\tflags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);\n+\t}\n+\n \tif (ctx.incremental) {\n \t\tstruct strbuf lock_name = STRBUF_INIT;\n \n@@ -1333,18 +1345,6 @@ static int write_midx_internal(const char *object_dir,\n \t\tf = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));\n \t}\n \n-\tif (ctx.nr - dropped_packs == 0) {\n-\t\terror(_(\"no pack files to index.\"));\n-\t\tresult = 1;\n-\t\tgoto cleanup;\n-\t}\n-\n-\tif (!ctx.entries_nr) {\n-\t\tif (flags & MIDX_WRITE_BITMAP)\n-\t\t\twarning(_(\"refusing to write multi-pack .bitmap without any objects\"));\n-\t\tflags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);\n-\t}\n-\n \tcf = init_chunkfile(f);\n \n \tadd_chunk(cf, MIDX_CHUNKID_PACKNAMES, pack_name_concat_len,\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501362","messageId":"5c820da97616466dd903f35981033a4fbf431dbc.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 09/20] builtin/repack: fix leaks when computing packs to repack","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:26Z","receivedAt":"2024-08-20T14:05:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When writing an MIDX in git-repack(1) we first collect all the pack\nnames that we want to add to it in a string list. This list is marked as\n`NODUP`, which indicates that it will neither duplicate nor own strings\nadded to it. In `write_midx_included_packs()` we then `insert()` strings\nvia `xstrdup()` or `strbuf_detach()`, but the resulting strings will not\nbe owned by anything and thus leak.\n\nFix this issue by marking the list as `DUP` and using a local buffer to\ncompute the pack names.\n\nThis leak is hit in t5319, but plugging it is not sufficient to make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/repack.c | 36 +++++++++++++++++++++++++++---------\n 1 file changed, 27 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 62cfa50c50f..8bb875532b4 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -732,14 +732,23 @@ static void midx_included_packs(struct string_list *include,\n \t\t\t\tstruct pack_geometry *geometry)\n {\n \tstruct string_list_item *item;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tfor_each_string_list_item(item, &existing->kept_packs) {\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"%s.idx\", item->string);\n+\t\tstring_list_insert(include, buf.buf);\n+\t}\n+\n+\tfor_each_string_list_item(item, names) {\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"pack-%s.idx\", item->string);\n+\t\tstring_list_insert(include, buf.buf);\n+\t}\n \n-\tfor_each_string_list_item(item, &existing->kept_packs)\n-\t\tstring_list_insert(include, xstrfmt(\"%s.idx\", item->string));\n-\tfor_each_string_list_item(item, names)\n-\t\tstring_list_insert(include, xstrfmt(\"pack-%s.idx\", item->string));\n \tif (geometry->split_factor) {\n-\t\tstruct strbuf buf = STRBUF_INIT;\n \t\tuint32_t i;\n+\n \t\tfor (i = geometry->split; i < geometry->pack_nr; i++) {\n \t\t\tstruct packed_git *p = geometry->pack[i];\n \n@@ -754,17 +763,21 @@ static void midx_included_packs(struct string_list *include,\n \t\t\tif (!p->pack_local)\n \t\t\t\tcontinue;\n \n+\t\t\tstrbuf_reset(&buf);\n \t\t\tstrbuf_addstr(&buf, pack_basename(p));\n \t\t\tstrbuf_strip_suffix(&buf, \".pack\");\n \t\t\tstrbuf_addstr(&buf, \".idx\");\n \n-\t\t\tstring_list_insert(include, strbuf_detach(&buf, NULL));\n+\t\t\tstring_list_insert(include, buf.buf);\n \t\t}\n \t} else {\n \t\tfor_each_string_list_item(item, &existing->non_kept_packs) {\n \t\t\tif (pack_is_marked_for_deletion(item))\n \t\t\t\tcontinue;\n-\t\t\tstring_list_insert(include, xstrfmt(\"%s.idx\", item->string));\n+\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_addf(&buf, \"%s.idx\", item->string);\n+\t\t\tstring_list_insert(include, buf.buf);\n \t\t}\n \t}\n \n@@ -784,8 +797,13 @@ static void midx_included_packs(struct string_list *include,\n \t\t */\n \t\tif (pack_is_marked_for_deletion(item))\n \t\t\tcontinue;\n-\t\tstring_list_insert(include, xstrfmt(\"%s.idx\", item->string));\n+\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"%s.idx\", item->string);\n+\t\tstring_list_insert(include, buf.buf);\n \t}\n+\n+\tstrbuf_release(&buf);\n }\n \n static int write_midx_included_packs(struct string_list *include,\n@@ -1476,7 +1494,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\tmark_packs_for_deletion(&existing, &names);\n \n \tif (write_midx) {\n-\t\tstruct string_list include = STRING_LIST_INIT_NODUP;\n+\t\tstruct string_list include = STRING_LIST_INIT_DUP;\n \t\tmidx_included_packs(&include, &existing, &names, &geometry);\n \n \t\tret = write_midx_included_packs(&include, &geometry, &names,\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501363","messageId":"9caf5eeea934da49608420fa09fc9179c73a4de7.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 10/20] t/helper: fix leaking multi-pack-indices in \"read-midx\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:28Z","receivedAt":"2024-08-20T14:05:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Several of the subcommands of `test-helper read-midx` do not close the\nMIDX that they have opened, leading to memory leaks. Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-read-midx.c | 8 +++++++-\n t/t5319-multi-pack-index.sh | 2 ++\n t/t7704-repack-cruft.sh | 1 +\n 3 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\nindex 69757e94fc2..438fb9fc619 100644\n--- a/t/helper/test-read-midx.c\n+++ b/t/helper/test-read-midx.c\n@@ -86,6 +86,8 @@ static int read_midx_checksum(const char *object_dir)\n \tif (!m)\n \t\treturn 1;\n \tprintf(\"%s\\n\", hash_to_hex(get_midx_checksum(m)));\n+\n+\tclose_midx(m);\n \treturn 0;\n }\n \n@@ -102,10 +104,12 @@ static int read_midx_preferred_pack(const char *object_dir)\n \n \tif (midx_preferred_pack(midx, &preferred_pack) < 0) {\n \t\twarning(_(\"could not determine MIDX preferred pack\"));\n+\t\tclose_midx(midx);\n \t\treturn 1;\n \t}\n \n \tprintf(\"%s\\n\", midx->pack_names[preferred_pack]);\n+\tclose_midx(midx);\n \treturn 0;\n }\n \n@@ -122,8 +126,10 @@ static int read_midx_bitmapped_packs(const char *object_dir)\n \t\treturn 1;\n \n \tfor (i = 0; i < midx->num_packs + midx->num_packs_in_base; i++) {\n-\t\tif (nth_bitmapped_pack(the_repository, midx, &pack, i) < 0)\n+\t\tif (nth_bitmapped_pack(the_repository, midx, &pack, i) < 0) {\n+\t\t\tclose_midx(midx);\n \t\t\treturn 1;\n+\t\t}\n \n \t\tprintf(\"%s\\n\", pack_basename(pack.p));\n \t\tprintf(\" bitmap_pos: %\"PRIuMAX\"\\n\", (uintmax_t)pack.bitmap_pos);\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex ce1b58c7323..fbbc218d04a 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='multi-pack-indexes'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-chunk.sh\n . \"$TEST_DIRECTORY\"/lib-midx.sh\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 959e6e26488..5db9f4e10f7 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -2,6 +2,7 @@\n \n test_description='git repack works correctly'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n objdir=.git/objects\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501364","messageId":"8e12c55536d8de52f5c5a972c739977b69d624ce.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 11/20] transport: fix leaking OID arrays in git:// transport data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:31Z","receivedAt":"2024-08-20T14:05:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The transport data for the \"git://\" protocol contains two OID arrays\nthat we never free, creating a memory leak. Plug them.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5501-fetch-push-alternates.sh | 1 +\n t/t5519-push-alternates.sh | 1 +\n transport.c | 2 ++\n 3 files changed, 4 insertions(+)\n\ndiff --git a/t/t5501-fetch-push-alternates.sh b/t/t5501-fetch-push-alternates.sh\nindex 66f19a4ef2b..0c8668a1b8e 100755\n--- a/t/t5501-fetch-push-alternates.sh\n+++ b/t/t5501-fetch-push-alternates.sh\n@@ -4,6 +4,7 @@ test_description='fetch/push involving alternates'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n count_objects () {\ndiff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh\nindex 20ba604dfde..72e97b15fab 100755\n--- a/t/t5519-push-alternates.sh\n+++ b/t/t5519-push-alternates.sh\n@@ -5,6 +5,7 @@ test_description='push to a repository that borrows from elsewhere'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/transport.c b/transport.c\nindex 7c4af9f56f2..f0672fdc505 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -946,6 +946,8 @@ static int disconnect_git(struct transport *transport)\n \t}\n \n \tlist_objects_filter_release(&data->options.filter_options);\n+\toid_array_clear(&data->extra_have);\n+\toid_array_clear(&data->shallow);\n \tfree(data);\n \treturn 0;\n }\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501365","messageId":"5d8e0a3d8b4c063d0c707a1bed8c692e6719c47f.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 12/20] builtin/send-pack: fix leaking refspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:35Z","receivedAt":"2024-08-20T14:05:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never free data associated with the assembled refspec in\ngit-send-pack(1), causing a memory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/send-pack.c | 1 +\n t/t5400-send-pack.sh | 1 +\n t/t5401-update-hooks.sh | 2 ++\n t/t5408-send-pack-stdin.sh | 2 ++\n t/t5548-push-porcelain.sh | 1 +\n t/t5812-proto-disable-http.sh | 2 ++\n 6 files changed, 9 insertions(+)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 17cae6bbbdf..ef0df808249 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -338,5 +338,6 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \n \tfree_refs(remote_refs);\n \tfree_refs(local_refs);\n+\trefspec_clear(&rs);\n \treturn ret;\n }\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 3f81f16e133..248c74d8ef2 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -9,6 +9,7 @@ test_description='See why rewinding head breaks send-pack\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n cnt=64\ndiff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh\nindex d8cadeec733..3c1ea6086e7 100755\n--- a/t/t5401-update-hooks.sh\n+++ b/t/t5401-update-hooks.sh\n@@ -4,6 +4,8 @@\n #\n \n test_description='Test the update hook infrastructure.'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t5408-send-pack-stdin.sh b/t/t5408-send-pack-stdin.sh\nindex e8737df6f95..c3695a4d4e3 100755\n--- a/t/t5408-send-pack-stdin.sh\n+++ b/t/t5408-send-pack-stdin.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='send-pack --stdin tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n create_ref () {\ndiff --git a/t/t5548-push-porcelain.sh b/t/t5548-push-porcelain.sh\nindex 6282728eaf3..ecb3877aa4b 100755\n--- a/t/t5548-push-porcelain.sh\n+++ b/t/t5548-push-porcelain.sh\n@@ -4,6 +4,7 @@\n #\n test_description='Test git push porcelain output'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Create commits in and assign each commit's oid to shell variables\ndiff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh\nindex 769c717e88b..f69959c64ca 100755\n--- a/t/t5812-proto-disable-http.sh\n+++ b/t/t5812-proto-disable-http.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test disabling of git-over-http in clone/fetch'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-proto-disable.sh\"\n . \"$TEST_DIRECTORY/lib-httpd.sh\"\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501366","messageId":"5d09959b6426e53a68e1bce547f9507bdf21bcde.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 13/20] sideband: fix leaks when configuring sideband colors","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:41Z","receivedAt":"2024-08-20T14:05:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We read a bunch of configs in `use_sideband_colors()` to configure the\ncolors that Git should use. We never free the strings read from the\nconfig though, causing memory leaks. Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n sideband.c | 8 +++++---\n t/t5409-colorize-remote-messages.sh | 1 +\n 2 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex 5d8907151fe..deb6ec0a8b7 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -30,7 +30,7 @@ static int use_sideband_colors(void)\n \n \tconst char *key = \"color.remote\";\n \tstruct strbuf sb = STRBUF_INIT;\n-\tchar *value;\n+\tchar *value = NULL;\n \tint i;\n \n \tif (use_sideband_colors_cached >= 0)\n@@ -43,15 +43,17 @@ static int use_sideband_colors(void)\n \t} else {\n \t\tuse_sideband_colors_cached = GIT_COLOR_AUTO;\n \t}\n+\tFREE_AND_NULL(value);\n \n \tfor (i = 0; i < ARRAY_SIZE(keywords); i++) {\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_addf(&sb, \"%s.%s\", key, keywords[i].keyword);\n \t\tif (git_config_get_string(sb.buf, &value))\n \t\t\tcontinue;\n-\t\tif (color_parse(value, keywords[i].color))\n-\t\t\tcontinue;\n+\t\tcolor_parse(value, keywords[i].color);\n+\t\tFREE_AND_NULL(value);\n \t}\n+\n \tstrbuf_release(&sb);\n \treturn use_sideband_colors_cached;\n }\ndiff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh\nindex fa5de4500a4..516b22fd963 100755\n--- a/t/t5409-colorize-remote-messages.sh\n+++ b/t/t5409-colorize-remote-messages.sh\n@@ -2,6 +2,7 @@\n \n test_description='remote messages are colorized on the client'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501367","messageId":"1c94195488d2db8ba169368a6c28171d5a2640f3.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 14/20] builtin/fetch-pack: fix leaking refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:44Z","receivedAt":"2024-08-20T14:05:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We build several ref lists in git-fetch-pack(1), but never free them.\nFix those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/fetch-pack.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex af329e8d5cf..fe404d1305b 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -46,7 +46,7 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,\n int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n {\n \tint i, ret;\n-\tstruct ref *ref = NULL;\n+\tstruct ref *fetched_refs = NULL, *remote_refs = NULL;\n \tconst char *dest = NULL;\n \tstruct ref **sought = NULL;\n \tint nr_sought = 0, alloc_sought = 0;\n@@ -228,19 +228,20 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n \tversion = discover_version(&reader);\n \tswitch (version) {\n \tcase protocol_v2:\n-\t\tget_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL,\n+\t\tget_remote_refs(fd[1], &reader, &remote_refs, 0, NULL, NULL,\n \t\t\t\targs.stateless_rpc);\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n-\t\tget_remote_heads(&reader, &ref, 0, NULL, &shallow);\n+\t\tget_remote_heads(&reader, &remote_refs, 0, NULL, &shallow);\n \t\tbreak;\n \tcase protocol_unknown_version:\n \t\tBUG(\"unknown protocol version\");\n \t}\n \n-\tref = fetch_pack(&args, fd, ref, sought, nr_sought,\n+\tfetched_refs = fetch_pack(&args, fd, remote_refs, sought, nr_sought,\n \t\t\t &shallow, pack_lockfiles_ptr, version);\n+\n \tif (pack_lockfiles.nr) {\n \t\tint i;\n \n@@ -260,7 +261,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n \tif (finish_connect(conn))\n \t\treturn 1;\n \n-\tret = !ref;\n+\tret = !fetched_refs;\n \n \t/*\n \t * If the heads to pull were given, we should have consumed\n@@ -270,11 +271,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n \t */\n \tret |= report_unmatched_refs(sought, nr_sought);\n \n-\twhile (ref) {\n+\tfor (struct ref *ref = fetched_refs; ref; ref = ref->next)\n \t\tprintf(\"%s %s\\n\",\n \t\t oid_to_hex(&ref->old_oid), ref->name);\n-\t\tref = ref->next;\n-\t}\n \n+\tfor (size_t i = 0; i < nr_sought; i++)\n+\t\tfree_one_ref(sought[i]);\n+\tfree(sought);\n+\tfree_refs(fetched_refs);\n+\tfree_refs(remote_refs);\n \treturn ret;\n }\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501368","messageId":"97346d6f944e3587a08d96a5e1b4ead8df8a0bc0.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 15/20] remote: fix leaking config strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:46Z","receivedAt":"2024-08-20T14:05:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We're leaking several config strings when assembling remotes, either\nbecause we do not free preceding values in case a config was set\nmultiple times, or because we do not free them when releasing the remote\nstate. Plug those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n remote.c | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/remote.c b/remote.c\nindex 7d5b8f750d8..3087437bc61 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -373,8 +373,10 @@ static int handle_config(const char *key, const char *value,\n \t\t\treturn -1;\n \t\tbranch = make_branch(remote_state, name, namelen);\n \t\tif (!strcmp(subkey, \"remote\")) {\n+\t\t\tFREE_AND_NULL(branch->remote_name);\n \t\t\treturn git_config_string(&branch->remote_name, key, value);\n \t\t} else if (!strcmp(subkey, \"pushremote\")) {\n+\t\t\tFREE_AND_NULL(branch->pushremote_name);\n \t\t\treturn git_config_string(&branch->pushremote_name, key, value);\n \t\t} else if (!strcmp(subkey, \"merge\")) {\n \t\t\tif (!value)\n@@ -406,9 +408,11 @@ static int handle_config(const char *key, const char *value,\n \t\treturn 0;\n \n \t/* Handle remote.* variables */\n-\tif (!name && !strcmp(subkey, \"pushdefault\"))\n+\tif (!name && !strcmp(subkey, \"pushdefault\")) {\n+\t\tFREE_AND_NULL(remote_state->pushremote_name);\n \t\treturn git_config_string(&remote_state->pushremote_name, key,\n \t\t\t\t\t value);\n+\t}\n \n \tif (!name)\n \t\treturn 0;\n@@ -475,12 +479,15 @@ static int handle_config(const char *key, const char *value,\n \t\telse if (!strcmp(value, \"--tags\"))\n \t\t\tremote->fetch_tags = 2;\n \t} else if (!strcmp(subkey, \"proxy\")) {\n+\t\tFREE_AND_NULL(remote->http_proxy);\n \t\treturn git_config_string(&remote->http_proxy,\n \t\t\t\t\t key, value);\n \t} else if (!strcmp(subkey, \"proxyauthmethod\")) {\n+\t\tFREE_AND_NULL(remote->http_proxy_authmethod);\n \t\treturn git_config_string(&remote->http_proxy_authmethod,\n \t\t\t\t\t key, value);\n \t} else if (!strcmp(subkey, \"vcs\")) {\n+\t\tFREE_AND_NULL(remote->foreign_vcs);\n \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n \t}\n \treturn 0;\n@@ -2802,6 +2809,7 @@ void remote_state_clear(struct remote_state *remote_state)\n \tfor (i = 0; i < remote_state->remotes_nr; i++)\n \t\tremote_clear(remote_state->remotes[i]);\n \tFREE_AND_NULL(remote_state->remotes);\n+\tFREE_AND_NULL(remote_state->pushremote_name);\n \tremote_state->remotes_alloc = 0;\n \tremote_state->remotes_nr = 0;\n \n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501369","messageId":"e1d0be37636a1ec46b4fe52613582ba71a47868b.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 16/20] remote: fix leaks when matching refspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:49Z","receivedAt":"2024-08-20T14:05:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `match_explicit()`, we try to match a source ref with a destination\nref according to a refspec item. This matching sometimes requires us to\nallocate a new source spec so that it looks like we expect. And while we\nin some end up assigning this allocated ref as `peer_ref`, which hands\nover ownership of it to the caller, in other cases we don't. We neither\nfree it though, causing a memory leak.\n\nFix the leak by creating a common exit path where we can easily free the\nsource ref in case it is allocated and hasn't been handed over to the\ncaller.\n\nSigned-off-by: Patrick Steinhardt \n---\n remote.c | 43 +++++++++++++++++++++++++++++--------------\n t/t5505-remote.sh | 1 +\n 2 files changed, 30 insertions(+), 14 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 3087437bc61..892a47086db 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1325,18 +1325,21 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\t struct ref ***dst_tail,\n \t\t\t struct refspec_item *rs)\n {\n-\tstruct ref *matched_src, *matched_dst;\n-\tint allocated_src;\n+\tstruct ref *matched_src = NULL, *matched_dst = NULL;\n+\tint allocated_src = 0, ret;\n \n \tconst char *dst_value = rs->dst;\n \tchar *dst_guess;\n \n-\tif (rs->pattern || rs->matching || rs->negative)\n-\t\treturn 0;\n+\tif (rs->pattern || rs->matching || rs->negative) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n \n-\tmatched_src = matched_dst = NULL;\n-\tif (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0)\n-\t\treturn -1;\n+\tif (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n \n \tif (!dst_value) {\n \t\tint flag;\n@@ -1375,18 +1378,30 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t dst_value);\n \t\tbreak;\n \t}\n-\tif (!matched_dst)\n-\t\treturn -1;\n-\tif (matched_dst->peer_ref)\n-\t\treturn error(_(\"dst ref %s receives from more than one src\"),\n-\t\t\t matched_dst->name);\n-\telse {\n+\n+\tif (!matched_dst) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\n+\tif (matched_dst->peer_ref) {\n+\t\tret = error(_(\"dst ref %s receives from more than one src\"),\n+\t\t\t matched_dst->name);\n+\t\tgoto out;\n+\t} else {\n \t\tmatched_dst->peer_ref = allocated_src ?\n \t\t\t\t\tmatched_src :\n \t\t\t\t\tcopy_ref(matched_src);\n \t\tmatched_dst->force = rs->force;\n+\t\tmatched_src = NULL;\n \t}\n-\treturn 0;\n+\n+\tret = 0;\n+\n+out:\n+\tif (allocated_src)\n+\t\tfree_one_ref(matched_src);\n+\treturn ret;\n }\n \n static int match_explicit_refs(struct ref *src, struct ref *dst,\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 08424e878e1..532035933f3 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -2,6 +2,7 @@\n \n test_description='git remote porcelain-ish'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n setup_repository () {\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501370","messageId":"773fe580d750e94e34499f385701794eefbc9c27.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 17/20] remote: fix leaking peer ref when expanding refmap","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:52Z","receivedAt":"2024-08-20T14:05:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When expanding remote refs via the refspec in `get_expanded_map()`, we\nfirst copy the remote ref and then override its peer ref with the\nexpanded name. This may cause a memory leak though in case the peer ref\nis already set, as this field is being copied by `copy_ref()`, as well.\n\nFix the leak by freeing the peer ref before we re-assign the field.\n\nSigned-off-by: Patrick Steinhardt \n---\n remote.c | 2 ++\n t/t5536-fetch-conflicts.sh | 1 +\n t/t5553-set-upstream.sh | 1 +\n t/t5703-upload-pack-ref-in-want.sh | 1 +\n t/t6050-replace.sh | 1 +\n 5 files changed, 6 insertions(+)\n\ndiff --git a/remote.c b/remote.c\nindex 892a47086db..6b50cafbbd3 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2062,6 +2062,8 @@ static struct ref *get_expanded_map(const struct ref *remote_refs,\n \t\t !ignore_symref_update(expn_name, &scratch)) {\n \t\t\tstruct ref *cpy = copy_ref(ref);\n \n+\t\t\tif (cpy->peer_ref)\n+\t\t\t\tfree_one_ref(cpy->peer_ref);\n \t\t\tcpy->peer_ref = alloc_ref(expn_name);\n \t\t\tif (refspec->force)\n \t\t\t\tcpy->peer_ref->force = 1;\ndiff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh\nindex 23bf6961700..2dcbe790523 100755\n--- a/t/t5536-fetch-conflicts.sh\n+++ b/t/t5536-fetch-conflicts.sh\n@@ -2,6 +2,7 @@\n \n test_description='fetch handles conflicting refspecs correctly'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n D=$(pwd)\ndiff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh\nindex 70e3376d31b..33e919a17e1 100755\n--- a/t/t5553-set-upstream.sh\n+++ b/t/t5553-set-upstream.sh\n@@ -4,6 +4,7 @@ test_description='\"git fetch/pull --set-upstream\" basic tests.'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n check_config () {\ndiff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh\nindex 191097171bc..f75fae52c83 100755\n--- a/t/t5703-upload-pack-ref-in-want.sh\n+++ b/t/t5703-upload-pack-ref-in-want.sh\n@@ -2,6 +2,7 @@\n \n test_description='upload-pack ref-in-want'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n get_actual_refs () {\ndiff --git a/t/t6050-replace.sh b/t/t6050-replace.sh\nindex c6e9b33e44e..d7702fc7562 100755\n--- a/t/t6050-replace.sh\n+++ b/t/t6050-replace.sh\n@@ -7,6 +7,7 @@ test_description='Tests replace refs functionality'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501371","messageId":"9ede792550e0b289d931c7f0fcf467873f09c5a5.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 18/20] builtin/fetch: fix leaking transaction with `--atomic`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:54Z","receivedAt":"2024-08-20T14:06:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"With the `--atomic` flag, we use a single ref transaction to commit all\nref updates in git-fetch(1). The lifetime of transactions is somewhat\nweird: while `ref_transaction_abort()` will free the transaction, a call\nto `ref_transaction_commit()` won't. We thus have to manually free the\ntransaction in the successful case.\n\nAdapt the code to free the transaction in the exit path to plug the\nresulting memory leak. As `ref_transaction_abort()` already freed the\ntransaction for us, we have to unset the transaction when we hit that\ncode path to not cause a double free.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/fetch.c | 8 ++++----\n t/t5574-fetch-output.sh | 1 +\n 2 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex c297569a473..0264483c0e5 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1731,11 +1731,8 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \n \t\tretcode = ref_transaction_commit(transaction, &err);\n-\t\tif (retcode) {\n-\t\t\tref_transaction_free(transaction);\n-\t\t\ttransaction = NULL;\n+\t\tif (retcode)\n \t\t\tgoto cleanup;\n-\t\t}\n \t}\n \n \tcommit_fetch_head(&fetch_head);\n@@ -1803,8 +1800,11 @@ static int do_fetch(struct transport *transport,\n \t\tif (transaction && ref_transaction_abort(transaction, &err) &&\n \t\t err.len)\n \t\t\terror(\"%s\", err.buf);\n+\t\ttransaction = NULL;\n \t}\n \n+\tif (transaction)\n+\t\tref_transaction_free(transaction);\n \tdisplay_state_release(&display_state);\n \tclose_fetch_head(&fetch_head);\n \tstrbuf_release(&err);\ndiff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh\nindex 5883839a04e..f7707326ea1 100755\n--- a/t/t5574-fetch-output.sh\n+++ b/t/t5574-fetch-output.sh\n@@ -5,6 +5,7 @@ test_description='git fetch output format'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'fetch with invalid output format configuration' '\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501372","messageId":"b72f7d1ee394291a018061c91222ea17a99c2e53.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 19/20] transport: fix leaking arguments when fetching from bundle","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:05:59Z","receivedAt":"2024-08-20T14:06:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass\nto `unbundle()`, but never free it. Fix this leak.\n\nThis memory leak gets hit in t5510, but fixing it isn't sufficient to\nmake the whole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n transport.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/transport.c b/transport.c\nindex f0672fdc505..da639d3bff0 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,\n \t\t &extra_index_pack_args,\n \t\t fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);\n \ttransport->hash_algo = data->header.hash_algo;\n+\n+\tstrvec_clear(&extra_index_pack_args);\n \treturn ret;\n }\n \n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501373","messageId":"e8f479deeb209e79a121ce58c9ec610c458aadad.1724159575.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH 20/20] transport: fix leaking negotiation tips","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-20T14:06:02Z","receivedAt":"2024-08-20T14:06:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We do not free negotiation tips in the transport's smart options. Fix\nthis by freeing them on disconnect.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5510-fetch.sh | 1 +\n transport.c | 4 ++++\n 2 files changed, 5 insertions(+)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 3b3991ab867..0890b9f61c5 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -5,6 +5,7 @@ test_description='Per branch config variables affects \"git fetch\".\n \n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-bundle.sh\n \ndiff --git a/transport.c b/transport.c\nindex da639d3bff0..0f20fc56e40 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -947,6 +947,10 @@ static int disconnect_git(struct transport *transport)\n \t\tfinish_connect(data->conn);\n \t}\n \n+\tif (data->options.negotiation_tips) {\n+\t\toid_array_clear(data->options.negotiation_tips);\n+\t\tfree(data->options.negotiation_tips);\n+\t}\n \tlist_objects_filter_release(&data->options.filter_options);\n \toid_array_clear(&data->extra_have);\n \toid_array_clear(&data->shallow);\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501388","messageId":"xmqq5xrv8099.fsf@gitster.g","threadId":"61982","inReplyTo":"69e30ea5179eff6472be54ebba64ebca3e562f32.1724159575.git.ps@pks.im","subject":"Re: [PATCH 01/20] mailinfo: fix leaking header data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-20T19:25:38Z","receivedAt":"2024-08-20T19:25:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> We populate the `mailinfo` arrays `p_hdr_data` and `s_hdr_data` with\n> data parsed from the mail headers. These arrays may end up being only\n> partially populated with gaps in case some of the headers do not parse\n> properly. This causes memory leaks because `strbuf_list_free()` will\n> stop iterating once it hits the first `NULL` pointer in the backing\n> array.\n>\n> Fix this by open-coding a variant of `strbuf_list_free()` that knows to\n> iterate through all headers.\n\nWell spotted. An array of strbuf is often a wrong data structure\nfor anything, but luckily we use it and suffer from a bug like this\nonly rarely.\n\nThe fix makes sense. Will queue.\n\n\n\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> mailinfo.c | 17 +++++++++++++++--\n> t/t5100-mailinfo.sh | 1 +\n> 2 files changed, 16 insertions(+), 2 deletions(-)\n>\n> diff --git a/mailinfo.c b/mailinfo.c\n> index 94b9b0abf22..a4fa64994ac 100644\n> --- a/mailinfo.c\n> +++ b/mailinfo.c\n> @@ -1290,8 +1290,21 @@ void clear_mailinfo(struct mailinfo *mi)\n> \tstrbuf_release(&mi->inbody_header_accum);\n> \tfree(mi->message_id);\n> \n> -\tstrbuf_list_free(mi->p_hdr_data);\n> -\tstrbuf_list_free(mi->s_hdr_data);\n> +\tfor (size_t i = 0; header[i]; i++) {\n> +\t\tif (!mi->p_hdr_data[i])\n> +\t\t\tcontinue;\n> +\t\tstrbuf_release(mi->p_hdr_data[i]);\n> +\t\tfree(mi->p_hdr_data[i]);\n> +\t}\n> +\tfree(mi->p_hdr_data);\n> +\n> +\tfor (size_t i = 0; header[i]; i++) {\n> +\t\tif (!mi->s_hdr_data[i])\n> +\t\t\tcontinue;\n> +\t\tstrbuf_release(mi->s_hdr_data[i]);\n> +\t\tfree(mi->s_hdr_data[i]);\n> +\t}\n> +\tfree(mi->s_hdr_data);\n> \n> \twhile (mi->content < mi->content_top) {\n> \t\tfree(*(mi->content_top));\n> diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\n> index c8d06554541..065156c1f39 100755\n> --- a/t/t5100-mailinfo.sh\n> +++ b/t/t5100-mailinfo.sh\n> @@ -5,6 +5,7 @@\n> \n> test_description='git mailinfo and git mailsplit test'\n> \n> +TEST_PASSES_SANITIZE_LEAK=true\n> . ./test-lib.sh\n> \n> DATA=\"$TEST_DIRECTORY/t5100\"\n"},{"id":"501389","messageId":"xmqqikvv6kii.fsf@gitster.g","threadId":"61982","inReplyTo":"ed0f01bf92c83fb7371326bd1a59933619de2c47.1724159575.git.ps@pks.im","subject":"Re: [PATCH 02/20] convert: fix leaks when resetting attributes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-20T19:51:01Z","receivedAt":"2024-08-20T19:51:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> When resetting parsed gitattributes, we free the list of convert drivers\n> parsed from the config. We only free some of the drivers' fields though\n> and thus have memory leaks.\n>\n> Fix this by freeing all allocated convert driver fields to plug these\n> memory leaks.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n\nThe helper has only one caller, which makes me wonder why we do not\nhave to call it more often. If \"git checkout\" or \"log -p\" notice an\nattribute file is updated, I wonder they should call it, for\nexample. But of course this is completely out of scope of this\ntopic.\n\nThis has been leaking even before 9642479a (convert: fix leaking\nconfig strings, 2024-08-01) straightened out the ownership rules of\nthe value obtained from git_config_string().\n\nThe fix makes sense. Will queue. Thanks.\n"},{"id":"501392","messageId":"xmqq7ccb6if8.fsf@gitster.g","threadId":"61982","inReplyTo":"82f3908f9620cee29e36a51f6d18ddcc8392b966.1724159575.git.ps@pks.im","subject":"Re: [PATCH 03/20] pretty: fix memory leaks when parsing pretty formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-20T20:36:11Z","receivedAt":"2024-08-20T20:36:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> When parsing pretty formats from the config we leak the name and user\n> format whenever these are set multiple times. This is because we do not\n> free any already-set value in case there is one.\n>\n> Plugging this leak for the name is trivial. For the user format we need\n> to be a bit more careful, because we may end up assigning a pointer into\n> the allocated region when the string is prefixed with either \"format\" or\n> \"tformat:\". In order to make it safe to unconditionally free the user\n> format we thus strdup the stripped string into the field instead of a\n> pointer into the string.\n\nYup, the stripped one is trickier, but the change looks correct.\n\nWill queue.\n\nBy the way, we tend to prefer no spaces after (cast) in our\ncodebase, but I just noticed that it is not spelled out in the\ncoding guidelines. Comparing\n\n $ git grep -E -e '\\((int|char \\*)\\) ' \\*.c ':!compat/' ':!contrib/'\n $ git grep -E -e '\\((int|char \\*)\\)[^ ]' \\*.c ':!compat/' ':!contrib/'\n\ntells me that the extra space after the (cast) is found mostly in\nborrowed or imported sources and majority of culprits are found in\nreftable library X-<.\n\nThanks.\n\n\n--- >8 ---\nSubject: CodingGuidelines: spaces around C operators\n\nAs we have operated with \"write like how your surrounding code is\nwritten\" for too long, after a huge code drop from another project,\nwe'll end up being inconsistent before such an imported code is\ncleaned up. We have many uses of cast operator with a space before\nits operand, mostly in the reftable code.\n\nSpell the convention out before it spreads to other places.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines\nindex e4bd0abdcd..ccaea39752 100644\n--- c/Documentation/CodingGuidelines\n+++ w/Documentation/CodingGuidelines\n@@ -303,7 +303,9 @@ For C programs:\n v12.01, 2022-03-28).\n \n - Variables have to be declared at the beginning of the block, before\n- the first statement (i.e. -Wdeclaration-after-statement).\n+ the first statement (i.e. -Wdeclaration-after-statement). It is\n+ encouraged to have a blank line between the end of the declarations\n+ and the first statement in the block.\n \n - NULL pointers shall be written as NULL, not as 0.\n \n@@ -323,6 +325,13 @@ For C programs:\n while( condition )\n \t\tfunc (bar+1);\n \n+ - A binary operator (other than \",\") and ternary conditional \"?:\"\n+ have a space on each side of the operator to separate it from its\n+ operands. E.g. \"A + 1\", not \"A+1\".\n+\n+ - A unary operator (other than \".\" and \"->\") have no space between it\n+ and its operand. E.g. \"(char *)ptr\", not \"(char *) ptr\".\n+\n - Do not explicitly compare an integral value with constant 0 or '\\0',\n or a pointer value with constant NULL. For instance, to validate that\n counted array is initialized but has no elements, write:\n"},{"id":"501397","messageId":"xmqqseuy4zhk.fsf@gitster.g","threadId":"61982","inReplyTo":"53db2fc7206cc11cae4930eba8c413fce4715afb.1724159575.git.ps@pks.im","subject":"Re: [PATCH 05/20] builtin/merge-tree: fix leaking `-X` strategy options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-20T22:10:31Z","receivedAt":"2024-08-20T22:10:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> The `-X` switch for git-merge-tree(1) will push each option into a local\n> `xopts` vector that we then end up parsing. The vector never gets freed\n> though, causing a memory leak. Plug it.\n\nIt is unfortunate that the ownership rules of the second parameter\nto parse_merge_opt() lets the string borrowed by the merge_options\nstruct. Otherwise we could have just cleared xopts immediately\nafter we are done with calling it.\n\nThe clean-up looks correct. Will queue.\nThanks.\n\n\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/merge-tree.c | 13 ++++++++++---\n> t/t4301-merge-tree-write-tree.sh | 1 +\n> 2 files changed, 11 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\n> index 9bca9b5f33c..c00469ed3db 100644\n> --- a/builtin/merge-tree.c\n> +++ b/builtin/merge-tree.c\n> @@ -533,6 +533,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> \tint expected_remaining_argc;\n> \tint original_argc;\n> \tconst char *merge_base = NULL;\n> +\tint ret;\n> \n> \tconst char * const merge_tree_usage[] = {\n> \t\tN_(\"git merge-tree [--write-tree] [] \"),\n> @@ -625,7 +626,9 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> \t\t\tstrbuf_list_free(split);\n> \t\t}\n> \t\tstrbuf_release(&buf);\n> -\t\treturn 0;\n> +\n> +\t\tret = 0;\n> +\t\tgoto out;\n> \t}\n> \n> \t/* Figure out which mode to use */\n> @@ -664,7 +667,11 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n> \n> \t/* Do the relevant type of merge */\n> \tif (o.mode == MODE_REAL)\n> -\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n> +\t\tret = real_merge(&o, merge_base, argv[0], argv[1], prefix);\n> \telse\n> -\t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n> +\t\tret = trivial_merge(argv[0], argv[1], argv[2]);\n> +\n> +out:\n> +\tstrvec_clear(&xopts);\n> +\treturn ret;\n> }\n> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\n> index eea19907b55..37f1cd7364c 100755\n> --- a/t/t4301-merge-tree-write-tree.sh\n> +++ b/t/t4301-merge-tree-write-tree.sh\n> @@ -2,6 +2,7 @@\n> \n> test_description='git merge-tree --write-tree'\n> \n> +TEST_PASSES_SANITIZE_LEAK=true\n> . ./test-lib.sh\n> \n> # This test is ort-specific\n"},{"id":"501399","messageId":"xmqqed6i4wb2.fsf@gitster.g","threadId":"61982","inReplyTo":"5f042ce5098563aa0662026006c356c278dad0b8.1724159575.git.ps@pks.im","subject":"Re: [PATCH 08/20] midx-write: fix leaking hashfile on error cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-20T23:19:13Z","receivedAt":"2024-08-20T23:19:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> When writing the MIDX file we first create the `struct hashfile` used to\n> write the trailer hash, and then afterwards we verify whether we can\n> actually write the MIDX in the first place. When we decide that we\n> can't, this leads to a memory leak because we never free the hash file\n> contents.\n>\n> We could fix this by freeing the hashfile on the exit path. There is a\n> better option though: we can simply move the checks for the error\n> condition earlier. As there is no early exit between creating the\n> hashfile and finalizing it anymore this is sufficient to fix the memory\n> leak.\n\nThe above is a good explanation why \"are we dropping everything\"\nblock was moved up, but does not explain why the other \"if there is\nno objects\" block has to move (it is however easy to see that the\nlatter block can be moved without any bad side effect).\n\nIn any case, the struct hashfile hashfd() gives us is now associated\nwith the struct chunkfile cf immediately after it is instanciated,\nand there is no early exit while the chunkfile is in use, which is\ngreat.\n"},{"id":"501400","messageId":"xmqqv7zu3g6c.fsf@gitster.g","threadId":"61982","inReplyTo":"5d09959b6426e53a68e1bce547f9507bdf21bcde.1724159575.git.ps@pks.im","subject":"Re: [PATCH 13/20] sideband: fix leaks when configuring sideband colors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-20T23:52:59Z","receivedAt":"2024-08-20T23:53:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> We read a bunch of configs in `use_sideband_colors()` to configure the\n> colors that Git should use. We never free the strings read from the\n> config though, causing memory leaks. Fix those.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> sideband.c | 8 +++++---\n> t/t5409-colorize-remote-messages.sh | 1 +\n> 2 files changed, 6 insertions(+), 3 deletions(-)\n>\n> diff --git a/sideband.c b/sideband.c\n> index 5d8907151fe..deb6ec0a8b7 100644\n> --- a/sideband.c\n> +++ b/sideband.c\n> @@ -30,7 +30,7 @@ static int use_sideband_colors(void)\n> \n> \tconst char *key = \"color.remote\";\n> \tstruct strbuf sb = STRBUF_INIT;\n> -\tchar *value;\n> +\tchar *value = NULL;\n\nHmph, this is a bit unfortunate. If there is no configuration\nvariable, on the first call to this function, we come to the end of\nif/else cascade and we need to free.\n\nAnother possibility to convey the intention better may be to assign\nNULL to value after the \"we already know the value, so return early\"\ntook place.\n\n> @@ -43,15 +43,17 @@ static int use_sideband_colors(void)\n> \t} else {\n> \t\tuse_sideband_colors_cached = GIT_COLOR_AUTO;\n> \t}\n> +\tFREE_AND_NULL(value);\n\nThis can be a simple \"free(value)\"; I do not see the need to clear\nthe variable at this point.\n\n> \tfor (i = 0; i < ARRAY_SIZE(keywords); i++) {\n> \t\tstrbuf_reset(&sb);\n> \t\tstrbuf_addf(&sb, \"%s.%s\", key, keywords[i].keyword);\n> \t\tif (git_config_get_string(sb.buf, &value))\n> \t\t\tcontinue;\n> -\t\tif (color_parse(value, keywords[i].color))\n> -\t\t\tcontinue;\n> +\t\tcolor_parse(value, keywords[i].color);\n> +\t\tFREE_AND_NULL(value);\n\nLikewise. I do not see the need to clear. We only come here after\nwe got something from gti_config_get_string(). That value may or\nmay not fail color_parse(), but when we reach this point, value\nalways has something we need to free, not some leftover value from\nthe previous iteration.\n\nThe patch is _not_ wrong per-se, though.\n\nThanks.\n\n\n"},{"id":"501447","messageId":"xmqq5xrtzs3c.fsf@gitster.g","threadId":"61982","inReplyTo":"1c94195488d2db8ba169368a6c28171d5a2640f3.1724159575.git.ps@pks.im","subject":"Re: [PATCH 14/20] builtin/fetch-pack: fix leaking refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-21T17:46:47Z","receivedAt":"2024-08-21T17:46:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> We build several ref lists in git-fetch-pack(1), but never free them.\n> Fix those leaks.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/fetch-pack.c | 20 ++++++++++++--------\n> 1 file changed, 12 insertions(+), 8 deletions(-)\n\nQuite straight-forward. In addition to plugging leaks, the\nresulting code is much easier to reason about by avoiding the reuse\nof the variable.\n"},{"id":"501449","messageId":"xmqqy14pycyo.fsf@gitster.g","threadId":"61982","inReplyTo":"97346d6f944e3587a08d96a5e1b4ead8df8a0bc0.1724159575.git.ps@pks.im","subject":"Re: [PATCH 15/20] remote: fix leaking config strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-21T17:58:55Z","receivedAt":"2024-08-21T17:59:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> @@ -2802,6 +2809,7 @@ void remote_state_clear(struct remote_state *remote_state)\n> \tfor (i = 0; i < remote_state->remotes_nr; i++)\n> \t\tremote_clear(remote_state->remotes[i]);\n> \tFREE_AND_NULL(remote_state->remotes);\n> +\tFREE_AND_NULL(remote_state->pushremote_name);\n> \tremote_state->remotes_alloc = 0;\n> \tremote_state->remotes_nr = 0;\n\nAs remote_state has two extra structures embedded in it, I wonder if\nwe should be clearing them in this function, but possibly it is\ncleared elsewhere or perhaps in a later series?\n\nAs the focus of this step is about strings that we obtained from the\nconfig API, it is totally outside the scope of this topic, even if\nit turns out to be needed to clear them.\n\nLooking good. Thanks.\n"},{"id":"501450","messageId":"xmqqseuxyck4.fsf@gitster.g","threadId":"61982","inReplyTo":"b72f7d1ee394291a018061c91222ea17a99c2e53.1724159575.git.ps@pks.im","subject":"Re: [PATCH 19/20] transport: fix leaking arguments when fetching from bundle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-21T18:07:39Z","receivedAt":"2024-08-21T18:07:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass\n> to `unbundle()`, but never free it. Fix this leak.\n>\n> This memory leak gets hit in t5510, but fixing it isn't sufficient to\n> make the whole test suite pass.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> transport.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/transport.c b/transport.c\n> index f0672fdc505..da639d3bff0 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,\n> \t\t &extra_index_pack_args,\n> \t\t fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);\n> \ttransport->hash_algo = data->header.hash_algo;\n> +\n> +\tstrvec_clear(&extra_index_pack_args);\n> \treturn ret;\n> }\n\nHmph. unbundle() has this:\n\n\tif (extra_index_pack_args) {\n\t\tstrvec_pushv(&ip.args, extra_index_pack_args->v);\n\t\tstrvec_clear(extra_index_pack_args);\n\t}\n\nso while I think this patch would not hurt at all, do we need this\npatch?\n\nThe other caller of unbundle() that passes the extra_index_pack_args\nis cmd_bundle_unbundle() and it does not do anything after it is\ndone.\n"},{"id":"501493","messageId":"Zsb0hjQj91aGKExI@tanuki","threadId":"61982","inReplyTo":"xmqq7ccb6if8.fsf@gitster.g","subject":"Re: [PATCH 03/20] pretty: fix memory leaks when parsing pretty formats","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T08:19:24Z","receivedAt":"2024-08-22T08:19:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Aug 20, 2024 at 01:36:11PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > When parsing pretty formats from the config we leak the name and user\n> > format whenever these are set multiple times. This is because we do not\n> > free any already-set value in case there is one.\n> >\n> > Plugging this leak for the name is trivial. For the user format we need\n> > to be a bit more careful, because we may end up assigning a pointer into\n> > the allocated region when the string is prefixed with either \"format\" or\n> > \"tformat:\". In order to make it safe to unconditionally free the user\n> > format we thus strdup the stripped string into the field instead of a\n> > pointer into the string.\n> \n> Yup, the stripped one is trickier, but the change looks correct.\n> \n> Will queue.\n> \n> By the way, we tend to prefer no spaces after (cast) in our\n> codebase, but I just noticed that it is not spelled out in the\n> coding guidelines. Comparing\n> \n> $ git grep -E -e '\\((int|char \\*)\\) ' \\*.c ':!compat/' ':!contrib/'\n> $ git grep -E -e '\\((int|char \\*)\\)[^ ]' \\*.c ':!compat/' ':!contrib/'\n> \n> tells me that the extra space after the (cast) is found mostly in\n> borrowed or imported sources and majority of culprits are found in\n> reftable library X-<.\n\nNot entirely surprising. I myself have typically favored adding a space\nafter the cast, and I remember wondering about the coding style several\ntimes here. I never wondered enough to actually check though.\n\nSo thanks for clarifying and thanks for updating the coding guidelines\naround this.\n\nPatrick\n"},{"id":"501494","messageId":"Zsb0lUkoJkbfuFiO@tanuki","threadId":"61982","inReplyTo":"xmqqed6i4wb2.fsf@gitster.g","subject":"Re: [PATCH 08/20] midx-write: fix leaking hashfile on error cases","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T08:19:33Z","receivedAt":"2024-08-22T08:19:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Aug 20, 2024 at 04:19:13PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > When writing the MIDX file we first create the `struct hashfile` used to\n> > write the trailer hash, and then afterwards we verify whether we can\n> > actually write the MIDX in the first place. When we decide that we\n> > can't, this leads to a memory leak because we never free the hash file\n> > contents.\n> >\n> > We could fix this by freeing the hashfile on the exit path. There is a\n> > better option though: we can simply move the checks for the error\n> > condition earlier. As there is no early exit between creating the\n> > hashfile and finalizing it anymore this is sufficient to fix the memory\n> > leak.\n> \n> The above is a good explanation why \"are we dropping everything\"\n> block was moved up, but does not explain why the other \"if there is\n> no objects\" block has to move (it is however easy to see that the\n> latter block can be moved without any bad side effect).\n> \n> In any case, the struct hashfile hashfd() gives us is now associated\n> with the struct chunkfile cf immediately after it is instanciated,\n> and there is no early exit while the chunkfile is in use, which is\n> great.\n\nThere basically is no reason why the other block needs to move. It just\nfelt more natural to keep massaging of parameters closly together before\nwe go off and actually do \"the thing\". I'll document this accordingly.\n\nPatrick\n"},{"id":"501495","messageId":"Zsb0nspisMs1Csl-@tanuki","threadId":"61982","inReplyTo":"xmqqy14pycyo.fsf@gitster.g","subject":"Re: [PATCH 15/20] remote: fix leaking config strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T08:19:42Z","receivedAt":"2024-08-22T08:19:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 21, 2024 at 10:58:55AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > @@ -2802,6 +2809,7 @@ void remote_state_clear(struct remote_state *remote_state)\n> > \tfor (i = 0; i < remote_state->remotes_nr; i++)\n> > \t\tremote_clear(remote_state->remotes[i]);\n> > \tFREE_AND_NULL(remote_state->remotes);\n> > +\tFREE_AND_NULL(remote_state->pushremote_name);\n> > \tremote_state->remotes_alloc = 0;\n> > \tremote_state->remotes_nr = 0;\n> \n> As remote_state has two extra structures embedded in it, I wonder if\n> we should be clearing them in this function, but possibly it is\n> cleared elsewhere or perhaps in a later series?\n\nIt is not yet part of any subsequent patch series, mostly because I\ndidn't happen to stumble over such leaks yet. Both of the rewrites very\nmuch are leaky though, and would be hit when we use \"insteadOf\" or\n\"pushInsteadOf\" configs.\n\nThe `struct branch` also needs handling and is being populated via\n\"branch\" configs.\n\n> As the focus of this step is about strings that we obtained from the\n> config API, it is totally outside the scope of this topic, even if\n> it turns out to be needed to clear them.\n\nWell, these are being populated via config strings. So I'd rather fix\nthem in this commit, as well.\n\nPatrick\n"},{"id":"501496","messageId":"Zsb0qU-G4kdlxrWa@tanuki","threadId":"61982","inReplyTo":"xmqqseuxyck4.fsf@gitster.g","subject":"Re: [PATCH 19/20] transport: fix leaking arguments when fetching from bundle","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T08:19:53Z","receivedAt":"2024-08-22T08:19:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 21, 2024 at 11:07:39AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass\n> > to `unbundle()`, but never free it. Fix this leak.\n> >\n> > This memory leak gets hit in t5510, but fixing it isn't sufficient to\n> > make the whole test suite pass.\n> >\n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> > transport.c | 2 ++\n> > 1 file changed, 2 insertions(+)\n> >\n> > diff --git a/transport.c b/transport.c\n> > index f0672fdc505..da639d3bff0 100644\n> > --- a/transport.c\n> > +++ b/transport.c\n> > @@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,\n> > \t\t &extra_index_pack_args,\n> > \t\t fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);\n> > \ttransport->hash_algo = data->header.hash_algo;\n> > +\n> > +\tstrvec_clear(&extra_index_pack_args);\n> > \treturn ret;\n> > }\n> \n> Hmph. unbundle() has this:\n> \n> \tif (extra_index_pack_args) {\n> \t\tstrvec_pushv(&ip.args, extra_index_pack_args->v);\n> \t\tstrvec_clear(extra_index_pack_args);\n> \t}\n> \n> so while I think this patch would not hurt at all, do we need this\n> patch?\n\nYes we do, because there is an early return in case `verify_bundle()`\nfails. I didn't notice that we have it in `unbundle()` though.\n\n> The other caller of unbundle() that passes the extra_index_pack_args\n> is cmd_bundle_unbundle() and it does not do anything after it is\n> done.\n\nI'd argue it's bad practice to have `unbundle()` clear the caller\nprovided args for us and somewhat surprising. While one way to fix this\nwould be to have a common exit path where we always free them. But I'd\nmuch rather make it the responsibility of the caller itself to free\nthem.\n\nI'll adapt the code in this spirit.\n\nPatrick\n"},{"id":"501497","messageId":"Zsb0rn6cghJB6V--@tanuki","threadId":"61982","inReplyTo":"xmqqv7zu3g6c.fsf@gitster.g","subject":"Re: [PATCH 13/20] sideband: fix leaks when configuring sideband colors","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T08:19:58Z","receivedAt":"2024-08-22T08:20:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Aug 20, 2024 at 04:52:59PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > We read a bunch of configs in `use_sideband_colors()` to configure the\n> > colors that Git should use. We never free the strings read from the\n> > config though, causing memory leaks. Fix those.\n> >\n> > Signed-off-by: Patrick Steinhardt \n> > ---\n> > sideband.c | 8 +++++---\n> > t/t5409-colorize-remote-messages.sh | 1 +\n> > 2 files changed, 6 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/sideband.c b/sideband.c\n> > index 5d8907151fe..deb6ec0a8b7 100644\n> > --- a/sideband.c\n> > +++ b/sideband.c\n> > @@ -30,7 +30,7 @@ static int use_sideband_colors(void)\n> > \n> > \tconst char *key = \"color.remote\";\n> > \tstruct strbuf sb = STRBUF_INIT;\n> > -\tchar *value;\n> > +\tchar *value = NULL;\n> \n> Hmph, this is a bit unfortunate. If there is no configuration\n> variable, on the first call to this function, we come to the end of\n> if/else cascade and we need to free.\n> \n> Another possibility to convey the intention better may be to assign\n> NULL to value after the \"we already know the value, so return early\"\n> took place.\n\nThere is an even better option: just use `git_config_get_string_tmp()`.\nThen we don't have to worry about freeing the strings at all.\n\nPatrick\n"},{"id":"501505","messageId":"cover.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724159575.git.ps@pks.im","subject":"[PATCH v2 00/20] Memory leak fixes (pt.5)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:08Z","receivedAt":"2024-08-22T09:17:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is version 2 of this patch series that fixes another set of memory\nleaks.\n\nChanges compared to v1:\n\n - Remove spaces between cast and variable.\n\n - Clarify why we move the `ctx.entries_nr` block.\n\n - Adapt sideband colors to use `git_config_get_string_tmp()` such that\n we do not have to allocate the strings in the first place.\n\n - Fix more memory leaks for config values in the remote state.\n\n - Refactor `unbundle()` to not free extra args passed by the caller\n anymore. Instead, this is now always done by the caler.\n\nThanks!\n\nPatrick\n\nPatrick Steinhardt (20):\n mailinfo: fix leaking header data\n convert: fix leaks when resetting attributes\n pretty: fix memory leaks when parsing pretty formats\n pretty: fix leaking key/value separator buffer\n builtin/merge-tree: fix leaking `-X` strategy options\n builtin/upload-archive: fix leaking args passed to `write_archive()`\n builtin/archive: fix leaking `OPT_FILENAME()` value\n midx-write: fix leaking hashfile on error cases\n builtin/repack: fix leaks when computing packs to repack\n t/helper: fix leaking multi-pack-indices in \"read-midx\"\n transport: fix leaking OID arrays in git:// transport data\n builtin/send-pack: fix leaking refspecs\n sideband: fix leaks when configuring sideband colors\n builtin/fetch-pack: fix leaking refs\n remote: fix leaking config strings\n remote: fix leaks when matching refspecs\n remote: fix leaking peer ref when expanding refmap\n builtin/fetch: fix leaking transaction with `--atomic`\n transport: fix leaking arguments when fetching from bundle\n transport: fix leaking negotiation tips\n\n archive.c | 10 ++++\n builtin/archive.c | 7 ++-\n builtin/bundle.c | 2 +\n builtin/fetch-pack.c | 20 ++++---\n builtin/fetch.c | 8 +--\n builtin/merge-tree.c | 13 ++++-\n builtin/repack.c | 36 +++++++++---\n builtin/send-pack.c | 1 +\n builtin/upload-archive.c | 8 ++-\n bundle.c | 4 +-\n convert.c | 3 +\n mailinfo.c | 17 +++++-\n midx-write.c | 24 ++++----\n pretty.c | 13 ++++-\n remote.c | 85 +++++++++++++++++++++++------\n sideband.c | 15 +++--\n t/helper/test-read-midx.c | 8 ++-\n t/t4150-am.sh | 1 +\n t/t4205-log-pretty-formats.sh | 2 +\n t/t4301-merge-tree-write-tree.sh | 1 +\n t/t5000-tar-tree.sh | 1 +\n t/t5003-archive-zip.sh | 1 +\n t/t5100-mailinfo.sh | 1 +\n t/t5319-multi-pack-index.sh | 2 +\n t/t5400-send-pack.sh | 1 +\n t/t5401-update-hooks.sh | 2 +\n t/t5408-send-pack-stdin.sh | 2 +\n t/t5409-colorize-remote-messages.sh | 1 +\n t/t5501-fetch-push-alternates.sh | 1 +\n t/t5505-remote.sh | 1 +\n t/t5510-fetch.sh | 1 +\n t/t5519-push-alternates.sh | 1 +\n t/t5536-fetch-conflicts.sh | 1 +\n t/t5548-push-porcelain.sh | 1 +\n t/t5553-set-upstream.sh | 1 +\n t/t5574-fetch-output.sh | 1 +\n t/t5703-upload-pack-ref-in-want.sh | 1 +\n t/t5812-proto-disable-http.sh | 2 +\n t/t6050-replace.sh | 1 +\n t/t7704-repack-cruft.sh | 1 +\n transport.c | 8 +++\n 41 files changed, 237 insertions(+), 73 deletions(-)\n\nRange-diff against v1:\n 1: 69e30ea5179 = 1: 69e30ea5179 mailinfo: fix leaking header data\n 2: ed0f01bf92c = 2: ed0f01bf92c convert: fix leaks when resetting attributes\n 3: 82f3908f962 ! 3: 8a1963685e7 pretty: fix memory leaks when parsing pretty formats\n @@ pretty.c: static int git_pretty_formats_config(const char *var, const char *valu\n \t\tcommit_formats_len++;\n \t}\n \n -+\tfree((char *) commit_format->name);\n ++\tfree((char *)commit_format->name);\n \tcommit_format->name = xstrdup(name);\n \tcommit_format->format = CMIT_FMT_USERFORMAT;\n \tif (git_config_string(&fmt, var, value))\n \t\treturn -1;\n \n -\tif (skip_prefix(fmt, \"format:\", &commit_format->user_format)) {\n -+\tfree((char *) commit_format->user_format);\n ++\tfree((char *)commit_format->user_format);\n +\tif (skip_prefix(fmt, \"format:\", &stripped)) {\n \t\tcommit_format->is_tformat = 0;\n -\t} else if (skip_prefix(fmt, \"tformat:\", &commit_format->user_format)) {\n 4: 696467780e6 = 4: 1c368a4489a pretty: fix leaking key/value separator buffer\n 5: 53db2fc7206 = 5: c62c5e9604e builtin/merge-tree: fix leaking `-X` strategy options\n 6: 5b05a325218 = 6: afdd7f90b13 builtin/upload-archive: fix leaking args passed to `write_archive()`\n 7: bad575df126 = 7: 38487f3f65b builtin/archive: fix leaking `OPT_FILENAME()` value\n 8: 5f042ce5098 ! 8: 693c93ddbf7 midx-write: fix leaking hashfile on error cases\n @@ Commit message\n hashfile and finalizing it anymore this is sufficient to fix the memory\n leak.\n \n + While at it, also move around the block checking for `ctx.entries_nr`.\n + This change is not required to fix the memory leak, but it feels natural\n + to move together all massaging of parameters before we go with them and\n + execute the actual logic.\n +\n Signed-off-by: Patrick Steinhardt \n \n ## midx-write.c ##\n 9: 5c820da9761 = 9: 6bca72e5c57 builtin/repack: fix leaks when computing packs to repack\n10: 9caf5eeea93 = 10: 48b60279d18 t/helper: fix leaking multi-pack-indices in \"read-midx\"\n11: 8e12c55536d = 11: 0cb440ef648 transport: fix leaking OID arrays in git:// transport data\n12: 5d8e0a3d8b4 = 12: f4300c9326b builtin/send-pack: fix leaking refspecs\n13: 5d09959b642 ! 13: 28805c15a42 sideband: fix leaks when configuring sideband colors\n @@ Commit message\n \n We read a bunch of configs in `use_sideband_colors()` to configure the\n colors that Git should use. We never free the strings read from the\n - config though, causing memory leaks. Fix those.\n + config though, causing memory leaks.\n +\n + Refactor the code to use `git_config_get_string_tmp()` instead, which\n + does not allocate memory. As we throw the strings away after parsing\n + them anyway there is no need to use allocated strings.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ sideband.c: static int use_sideband_colors(void)\n \tconst char *key = \"color.remote\";\n \tstruct strbuf sb = STRBUF_INIT;\n -\tchar *value;\n -+\tchar *value = NULL;\n ++\tconst char *value;\n \tint i;\n \n \tif (use_sideband_colors_cached >= 0)\n -@@ sideband.c: static int use_sideband_colors(void)\n - \t} else {\n + \t\treturn use_sideband_colors_cached;\n + \n +-\tif (!git_config_get_string(key, &value)) {\n ++\tif (!git_config_get_string_tmp(key, &value))\n + \t\tuse_sideband_colors_cached = git_config_colorbool(key, value);\n +-\t} else if (!git_config_get_string(\"color.ui\", &value)) {\n ++\telse if (!git_config_get_string_tmp(\"color.ui\", &value))\n + \t\tuse_sideband_colors_cached = git_config_colorbool(\"color.ui\", value);\n +-\t} else {\n ++\telse\n \t\tuse_sideband_colors_cached = GIT_COLOR_AUTO;\n - \t}\n -+\tFREE_AND_NULL(value);\n +-\t}\n \n \tfor (i = 0; i < ARRAY_SIZE(keywords); i++) {\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_addf(&sb, \"%s.%s\", key, keywords[i].keyword);\n - \t\tif (git_config_get_string(sb.buf, &value))\n - \t\t\tcontinue;\n --\t\tif (color_parse(value, keywords[i].color))\n +-\t\tif (git_config_get_string(sb.buf, &value))\n -\t\t\tcontinue;\n +-\t\tif (color_parse(value, keywords[i].color))\n ++\t\tif (git_config_get_string_tmp(sb.buf, &value))\n + \t\t\tcontinue;\n +\t\tcolor_parse(value, keywords[i].color);\n -+\t\tFREE_AND_NULL(value);\n \t}\n +\n \tstrbuf_release(&sb);\n14: 1c94195488d = 14: aac84c5a2b7 builtin/fetch-pack: fix leaking refs\n15: 97346d6f944 ! 15: 532328b7814 remote: fix leaking config strings\n @@ Commit message\n We're leaking several config strings when assembling remotes, either\n because we do not free preceding values in case a config was set\n multiple times, or because we do not free them when releasing the remote\n - state. Plug those leaks.\n + state. This includes config strings for \"branch\" sections, \"insteadOf\",\n + \"pushInsteadOf\", and \"pushDefault\".\n +\n + Plug those leaks.\n \n Signed-off-by: Patrick Steinhardt \n \n ## remote.c ##\n +@@ remote.c: static struct branch *make_branch(struct remote_state *remote_state,\n + \treturn ret;\n + }\n + \n ++static void branch_release(struct branch *branch)\n ++{\n ++\tfree((char *)branch->name);\n ++\tfree((char *)branch->refname);\n ++\tfree(branch->remote_name);\n ++\tfree(branch->pushremote_name);\n ++\tfor (int i = 0; i < branch->merge_nr; i++)\n ++\t\trefspec_item_clear(branch->merge[i]);\n ++\tfree(branch->merge);\n ++}\n ++\n + static struct rewrite *make_rewrite(struct rewrites *r,\n + \t\t\t\t const char *base, size_t len)\n + {\n +@@ remote.c: static struct rewrite *make_rewrite(struct rewrites *r,\n + \treturn ret;\n + }\n + \n ++static void rewrites_release(struct rewrites *r)\n ++{\n ++\tfor (int i = 0; i < r->rewrite_nr; i++)\n ++\t\tfree((char *)r->rewrite[i]->base);\n ++\tfree(r->rewrite);\n ++\tmemset(r, 0, sizeof(*r));\n ++}\n ++\n + static void add_instead_of(struct rewrite *rewrite, const char *instead_of)\n + {\n + \tALLOC_GROW(rewrite->instead_of, rewrite->instead_of_nr + 1, rewrite->instead_of_alloc);\n @@ remote.c: static int handle_config(const char *key, const char *value,\n \t\t\treturn -1;\n \t\tbranch = make_branch(remote_state, name, namelen);\n @@ remote.c: static int handle_config(const char *key, const char *value,\n \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n \t}\n \treturn 0;\n -@@ remote.c: void remote_state_clear(struct remote_state *remote_state)\n +@@ remote.c: struct remote_state *remote_state_new(void)\n + \n + void remote_state_clear(struct remote_state *remote_state)\n + {\n ++\tstruct hashmap_iter iter;\n ++\tstruct branch *b;\n + \tint i;\n + \n \tfor (i = 0; i < remote_state->remotes_nr; i++)\n \t\tremote_clear(remote_state->remotes[i]);\n \tFREE_AND_NULL(remote_state->remotes);\n @@ remote.c: void remote_state_clear(struct remote_state *remote_state)\n \tremote_state->remotes_alloc = 0;\n \tremote_state->remotes_nr = 0;\n \n ++\trewrites_release(&remote_state->rewrites);\n ++\trewrites_release(&remote_state->rewrites_push);\n ++\n + \thashmap_clear_and_free(&remote_state->remotes_hash, struct remote, ent);\n +-\thashmap_clear_and_free(&remote_state->branches_hash, struct remote, ent);\n ++\thashmap_for_each_entry(&remote_state->branches_hash, &iter, b, ent) {\n ++\t\tbranch_release(b);\n ++\t\tfree(b);\n ++\t}\n ++\thashmap_clear(&remote_state->branches_hash);\n + }\n + \n + /*\n16: e1d0be37636 = 16: 440b3d99372 remote: fix leaks when matching refspecs\n17: 773fe580d75 = 17: 662ec4e6484 remote: fix leaking peer ref when expanding refmap\n18: 9ede792550e = 18: dbd8eaa2cb1 builtin/fetch: fix leaking transaction with `--atomic`\n19: b72f7d1ee39 ! 19: 4c5740afe43 transport: fix leaking arguments when fetching from bundle\n @@ Commit message\n transport: fix leaking arguments when fetching from bundle\n \n In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass\n - to `unbundle()`, but never free it. Fix this leak.\n + to `unbundle()`, but never free it. And in theory we wouldn't have to\n + because `unbundle()` already knows to free the vector for us. But it\n + fails to do so when it exits early due to `verify_bundle()` failing.\n +\n + The calling convention that the arguments are freed by the callee and\n + not the caller feels somewhat weird. Refactor the code such that it is\n + instead the responsibility of the caller to free the vector, adapting\n + the only two callsites where we pass extra arguments. This also fixes\n + the memory leak.\n \n This memory leak gets hit in t5510, but fixing it isn't sufficient to\n make the whole test suite pass.\n \n Signed-off-by: Patrick Steinhardt \n \n + ## builtin/bundle.c ##\n +@@ builtin/bundle.c: static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)\n + \t\t\t &extra_index_pack_args, 0) ||\n + \t\tlist_bundle_refs(&header, argc, argv);\n + \tbundle_header_release(&header);\n ++\n + cleanup:\n ++\tstrvec_clear(&extra_index_pack_args);\n + \tfree(bundle_file);\n + \treturn ret;\n + }\n +\n + ## bundle.c ##\n +@@ bundle.c: int unbundle(struct repository *r, struct bundle_header *header,\n + \tif (flags & VERIFY_BUNDLE_FSCK)\n + \t\tstrvec_push(&ip.args, \"--fsck-objects\");\n + \n +-\tif (extra_index_pack_args) {\n ++\tif (extra_index_pack_args)\n + \t\tstrvec_pushv(&ip.args, extra_index_pack_args->v);\n +-\t\tstrvec_clear(extra_index_pack_args);\n +-\t}\n + \n + \tip.in = bundle_fd;\n + \tip.no_stdout = 1;\n +\n ## transport.c ##\n @@ transport.c: static int fetch_refs_from_bundle(struct transport *transport,\n \t\t &extra_index_pack_args,\n20: e8f479deeb2 = 20: c3d2b035761 transport: fix leaking negotiation tips\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501506","messageId":"69e30ea5179eff6472be54ebba64ebca3e562f32.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 01/20] mailinfo: fix leaking header data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:11Z","receivedAt":"2024-08-22T09:17:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We populate the `mailinfo` arrays `p_hdr_data` and `s_hdr_data` with\ndata parsed from the mail headers. These arrays may end up being only\npartially populated with gaps in case some of the headers do not parse\nproperly. This causes memory leaks because `strbuf_list_free()` will\nstop iterating once it hits the first `NULL` pointer in the backing\narray.\n\nFix this by open-coding a variant of `strbuf_list_free()` that knows to\niterate through all headers.\n\nSigned-off-by: Patrick Steinhardt \n---\n mailinfo.c | 17 +++++++++++++++--\n t/t5100-mailinfo.sh | 1 +\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/mailinfo.c b/mailinfo.c\nindex 94b9b0abf22..a4fa64994ac 100644\n--- a/mailinfo.c\n+++ b/mailinfo.c\n@@ -1290,8 +1290,21 @@ void clear_mailinfo(struct mailinfo *mi)\n \tstrbuf_release(&mi->inbody_header_accum);\n \tfree(mi->message_id);\n \n-\tstrbuf_list_free(mi->p_hdr_data);\n-\tstrbuf_list_free(mi->s_hdr_data);\n+\tfor (size_t i = 0; header[i]; i++) {\n+\t\tif (!mi->p_hdr_data[i])\n+\t\t\tcontinue;\n+\t\tstrbuf_release(mi->p_hdr_data[i]);\n+\t\tfree(mi->p_hdr_data[i]);\n+\t}\n+\tfree(mi->p_hdr_data);\n+\n+\tfor (size_t i = 0; header[i]; i++) {\n+\t\tif (!mi->s_hdr_data[i])\n+\t\t\tcontinue;\n+\t\tstrbuf_release(mi->s_hdr_data[i]);\n+\t\tfree(mi->s_hdr_data[i]);\n+\t}\n+\tfree(mi->s_hdr_data);\n \n \twhile (mi->content < mi->content_top) {\n \t\tfree(*(mi->content_top));\ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex c8d06554541..065156c1f39 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -5,6 +5,7 @@\n \n test_description='git mailinfo and git mailsplit test'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n DATA=\"$TEST_DIRECTORY/t5100\"\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501507","messageId":"ed0f01bf92c83fb7371326bd1a59933619de2c47.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 02/20] convert: fix leaks when resetting attributes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:13Z","receivedAt":"2024-08-22T09:17:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When resetting parsed gitattributes, we free the list of convert drivers\nparsed from the config. We only free some of the drivers' fields though\nand thus have memory leaks.\n\nFix this by freeing all allocated convert driver fields to plug these\nmemory leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n convert.c | 3 +++\n t/t4150-am.sh | 1 +\n 2 files changed, 4 insertions(+)\n\ndiff --git a/convert.c b/convert.c\nindex e6184d21f26..c9a31eb4f03 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1371,6 +1371,9 @@ void reset_parsed_attributes(void)\n \tfor (drv = user_convert; drv; drv = next) {\n \t\tnext = drv->next;\n \t\tfree((void *)drv->name);\n+\t\tfree((void *)drv->smudge);\n+\t\tfree((void *)drv->clean);\n+\t\tfree((void *)drv->process);\n \t\tfree(drv);\n \t}\n \tuser_convert = NULL;\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 5e2b6c80eae..232e1394e8d 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -5,6 +5,7 @@ test_description='git am running'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup: messages' '\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501508","messageId":"8a1963685e73fecb64bebb1154ad6205caf09a8d.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 03/20] pretty: fix memory leaks when parsing pretty formats","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:16Z","receivedAt":"2024-08-22T09:17:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When parsing pretty formats from the config we leak the name and user\nformat whenever these are set multiple times. This is because we do not\nfree any already-set value in case there is one.\n\nPlugging this leak for the name is trivial. For the user format we need\nto be a bit more careful, because we may end up assigning a pointer into\nthe allocated region when the string is prefixed with either \"format\" or\n\"tformat:\". In order to make it safe to unconditionally free the user\nformat we thus strdup the stripped string into the field instead of a\npointer into the string.\n\nSigned-off-by: Patrick Steinhardt \n---\n pretty.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 44222fb83c6..5e162d7204d 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -63,7 +63,7 @@ static int git_pretty_formats_config(const char *var, const char *value,\n \t\t\t\t void *cb UNUSED)\n {\n \tstruct cmt_fmt_map *commit_format = NULL;\n-\tconst char *name;\n+\tconst char *name, *stripped;\n \tchar *fmt;\n \tint i;\n \n@@ -90,15 +90,21 @@ static int git_pretty_formats_config(const char *var, const char *value,\n \t\tcommit_formats_len++;\n \t}\n \n+\tfree((char *)commit_format->name);\n \tcommit_format->name = xstrdup(name);\n \tcommit_format->format = CMIT_FMT_USERFORMAT;\n \tif (git_config_string(&fmt, var, value))\n \t\treturn -1;\n \n-\tif (skip_prefix(fmt, \"format:\", &commit_format->user_format)) {\n+\tfree((char *)commit_format->user_format);\n+\tif (skip_prefix(fmt, \"format:\", &stripped)) {\n \t\tcommit_format->is_tformat = 0;\n-\t} else if (skip_prefix(fmt, \"tformat:\", &commit_format->user_format)) {\n+\t\tcommit_format->user_format = xstrdup(stripped);\n+\t\tfree(fmt);\n+\t} else if (skip_prefix(fmt, \"tformat:\", &stripped)) {\n \t\tcommit_format->is_tformat = 1;\n+\t\tcommit_format->user_format = xstrdup(stripped);\n+\t\tfree(fmt);\n \t} else if (strchr(fmt, '%')) {\n \t\tcommit_format->is_tformat = 1;\n \t\tcommit_format->user_format = fmt;\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501509","messageId":"1c368a4489a23e258ad66b2b10ec8721f7409b3a.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 04/20] pretty: fix leaking key/value separator buffer","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:18Z","receivedAt":"2024-08-22T09:17:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `format_set_trailers_options()` function is responsible for parsing\na custom pretty format for trailers. It puts the parsed options into a\n`struct process_trailer_options` structure, while the allocated memory\nrequired for this will be put into separate caller-provided arguments.\nIt is thus the caller's responsibility to free the memory not via the\noptions structure, but via the other parameters.\n\nWhile we do this alright for the separator and filter keys, we do not\nfree the memory associated with the key/value separator. Fix this to\nplug this memory leak.\n\nSigned-off-by: Patrick Steinhardt \n---\n pretty.c | 1 +\n t/t4205-log-pretty-formats.sh | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/pretty.c b/pretty.c\nindex 5e162d7204d..5e5ae452530 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1776,6 +1776,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t}\n \ttrailer_out:\n \t\tstring_list_clear(&filter_list, 0);\n+\t\tstrbuf_release(&kvsepbuf);\n \t\tstrbuf_release(&sepbuf);\n \t\treturn ret;\n \t}\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 158b49d4b60..eb63ce011fa 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -5,6 +5,8 @@\n #\n \n test_description='Test pretty formats'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Tested non-UTF-8 encoding\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501510","messageId":"c62c5e9604e05ae7d65926b90832ba9aff84044a.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 05/20] builtin/merge-tree: fix leaking `-X` strategy options","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:21Z","receivedAt":"2024-08-22T09:17:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `-X` switch for git-merge-tree(1) will push each option into a local\n`xopts` vector that we then end up parsing. The vector never gets freed\nthough, causing a memory leak. Plug it.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/merge-tree.c | 13 ++++++++++---\n t/t4301-merge-tree-write-tree.sh | 1 +\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 9bca9b5f33c..c00469ed3db 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -533,6 +533,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \tint expected_remaining_argc;\n \tint original_argc;\n \tconst char *merge_base = NULL;\n+\tint ret;\n \n \tconst char * const merge_tree_usage[] = {\n \t\tN_(\"git merge-tree [--write-tree] [] \"),\n@@ -625,7 +626,9 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \t\t\tstrbuf_list_free(split);\n \t\t}\n \t\tstrbuf_release(&buf);\n-\t\treturn 0;\n+\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \t/* Figure out which mode to use */\n@@ -664,7 +667,11 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)\n \n \t/* Do the relevant type of merge */\n \tif (o.mode == MODE_REAL)\n-\t\treturn real_merge(&o, merge_base, argv[0], argv[1], prefix);\n+\t\tret = real_merge(&o, merge_base, argv[0], argv[1], prefix);\n \telse\n-\t\treturn trivial_merge(argv[0], argv[1], argv[2]);\n+\t\tret = trivial_merge(argv[0], argv[1], argv[2]);\n+\n+out:\n+\tstrvec_clear(&xopts);\n+\treturn ret;\n }\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex eea19907b55..37f1cd7364c 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -2,6 +2,7 @@\n \n test_description='git merge-tree --write-tree'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # This test is ort-specific\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501511","messageId":"afdd7f90b13511b8e842026f8ad43c9783e19231.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 06/20] builtin/upload-archive: fix leaking args passed to `write_archive()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:27Z","receivedAt":"2024-08-22T09:17:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In git-upload-archive(1), we pass an array of arguments to\n`write_archive()` to tell it what exactly to do. We don't ever clear the\nvector though, causing a memory leak. Furthermore though, the call to\n`write_archive()` may cause contents of the array to be modified, which\nwould cause us to leak memory to allocated strings held by it.\n\nFix the issue by having `write_archive()` create a shallow copy of\n`argv` before parsing the arguments. Like this, we won't modify the\ncaller's array and can easily `strvec_clear()` it to plug these memory\nleaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n archive.c | 10 ++++++++++\n builtin/upload-archive.c | 8 ++++++--\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 7bd60d0632a..9ba96aae4f7 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -736,6 +736,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tstruct pretty_print_describe_status describe_status = {0};\n \tstruct pretty_print_context ctx = {0};\n \tstruct archiver_args args;\n+\tconst char **argv_copy;\n \tint rc;\n \n \tgit_config_get_bool(\"uploadarchive.allowunreachable\", &remote_allow_unreachable);\n@@ -749,6 +750,14 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \targs.repo = repo;\n \targs.prefix = prefix;\n \tstring_list_init_dup(&args.extra_files);\n+\n+\t/*\n+\t * `parse_archive_args()` modifies contents of `argv`, which is what we\n+\t * want. Our callers may not want it though, so we create a copy here.\n+\t */\n+\tDUP_ARRAY(argv_copy, argv, argc);\n+\targv = argv_copy;\n+\n \targc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n \tif (!startup_info->have_repository) {\n \t\t/*\n@@ -767,6 +776,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tstring_list_clear_func(&args.extra_files, extra_file_info_clear);\n \tfree(args.refname);\n \tclear_pathspec(&args.pathspec);\n+\tfree(argv_copy);\n \n \treturn rc;\n }\ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex 1b09e5e1aa3..313a8dfa81c 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -22,6 +22,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)\n {\n \tstruct strvec sent_argv = STRVEC_INIT;\n \tconst char *arg_cmd = \"argument \";\n+\tint ret;\n \n \tif (argc != 2 || !strcmp(argv[1], \"-h\"))\n \t\tusage(upload_archive_usage);\n@@ -46,8 +47,11 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* parse all options sent by the client */\n-\treturn write_archive(sent_argv.nr, sent_argv.v, prefix,\n-\t\t\t the_repository, NULL, 1);\n+\tret = write_archive(sent_argv.nr, sent_argv.v, prefix,\n+\t\t\t the_repository, NULL, 1);\n+\n+\tstrvec_clear(&sent_argv);\n+\treturn ret;\n }\n \n __attribute__((format (printf, 1, 2)))\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501512","messageId":"38487f3f65b7ab0aa1351a15efa8494b6e040f8e.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 07/20] builtin/archive: fix leaking `OPT_FILENAME()` value","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:30Z","receivedAt":"2024-08-22T09:17:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"--output\" switch is an `OPT_FILENAME()` option, which allocates\nmemory when specified by the user. But while we free the string when\nexecuted without the \"--remote\" switch, we don't otherwise because we\nreturn via a separate exit path that doesn't know to free it.\n\nFix this by creating a common exit path.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/archive.c | 7 +++++--\n t/t5000-tar-tree.sh | 1 +\n t/t5003-archive-zip.sh | 1 +\n 3 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex b50981504f3..63f02990d11 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -100,13 +100,16 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \tif (output)\n \t\tcreate_output_file(output);\n \n-\tif (remote)\n-\t\treturn run_remote_archiver(argc, argv, remote, exec, output);\n+\tif (remote) {\n+\t\tret = run_remote_archiver(argc, argv, remote, exec, output);\n+\t\tgoto out;\n+\t}\n \n \tsetvbuf(stderr, NULL, _IOLBF, BUFSIZ);\n \n \tret = write_archive(argc, argv, prefix, the_repository, output, 0);\n \n+out:\n \tfree(output);\n \treturn ret;\n }\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 72b8d0ff02e..7abba8a4b20 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -25,6 +25,7 @@ commit id embedding:\n '\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n SUBSTFORMAT=%H%n\ndiff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\nindex 961c6aac256..01f591c99b9 100755\n--- a/t/t5003-archive-zip.sh\n+++ b/t/t5003-archive-zip.sh\n@@ -3,6 +3,7 @@\n test_description='git archive --format=zip test'\n \n TEST_CREATE_REPO_NO_TEMPLATE=1\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n SUBSTFORMAT=%H%n\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501513","messageId":"693c93ddbf761202bff2d7a3213b7afd80049174.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 08/20] midx-write: fix leaking hashfile on error cases","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:33Z","receivedAt":"2024-08-22T09:17:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When writing the MIDX file we first create the `struct hashfile` used to\nwrite the trailer hash, and then afterwards we verify whether we can\nactually write the MIDX in the first place. When we decide that we\ncan't, this leads to a memory leak because we never free the hash file\ncontents.\n\nWe could fix this by freeing the hashfile on the exit path. There is a\nbetter option though: we can simply move the checks for the error\ncondition earlier. As there is no early exit between creating the\nhashfile and finalizing it anymore this is sufficient to fix the memory\nleak.\n\nWhile at it, also move around the block checking for `ctx.entries_nr`.\nThis change is not required to fix the memory leak, but it feels natural\nto move together all massaging of parameters before we go with them and\nexecute the actual logic.\n\nSigned-off-by: Patrick Steinhardt \n---\n midx-write.c | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/midx-write.c b/midx-write.c\nindex e3fa33203fa..07d98d494aa 100644\n--- a/midx-write.c\n+++ b/midx-write.c\n@@ -1308,6 +1308,18 @@ static int write_midx_internal(const char *object_dir,\n \t\tpack_name_concat_len += MIDX_CHUNK_ALIGNMENT -\n \t\t\t\t\t(pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);\n \n+\tif (ctx.nr - dropped_packs == 0) {\n+\t\terror(_(\"no pack files to index.\"));\n+\t\tresult = 1;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tif (!ctx.entries_nr) {\n+\t\tif (flags & MIDX_WRITE_BITMAP)\n+\t\t\twarning(_(\"refusing to write multi-pack .bitmap without any objects\"));\n+\t\tflags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);\n+\t}\n+\n \tif (ctx.incremental) {\n \t\tstruct strbuf lock_name = STRBUF_INIT;\n \n@@ -1333,18 +1345,6 @@ static int write_midx_internal(const char *object_dir,\n \t\tf = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));\n \t}\n \n-\tif (ctx.nr - dropped_packs == 0) {\n-\t\terror(_(\"no pack files to index.\"));\n-\t\tresult = 1;\n-\t\tgoto cleanup;\n-\t}\n-\n-\tif (!ctx.entries_nr) {\n-\t\tif (flags & MIDX_WRITE_BITMAP)\n-\t\t\twarning(_(\"refusing to write multi-pack .bitmap without any objects\"));\n-\t\tflags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);\n-\t}\n-\n \tcf = init_chunkfile(f);\n \n \tadd_chunk(cf, MIDX_CHUNKID_PACKNAMES, pack_name_concat_len,\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501514","messageId":"6bca72e5c574d5f930c9cefe7fc803ac30e0c28a.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 09/20] builtin/repack: fix leaks when computing packs to repack","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:36Z","receivedAt":"2024-08-22T09:17:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When writing an MIDX in git-repack(1) we first collect all the pack\nnames that we want to add to it in a string list. This list is marked as\n`NODUP`, which indicates that it will neither duplicate nor own strings\nadded to it. In `write_midx_included_packs()` we then `insert()` strings\nvia `xstrdup()` or `strbuf_detach()`, but the resulting strings will not\nbe owned by anything and thus leak.\n\nFix this issue by marking the list as `DUP` and using a local buffer to\ncompute the pack names.\n\nThis leak is hit in t5319, but plugging it is not sufficient to make the\nwhole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/repack.c | 36 +++++++++++++++++++++++++++---------\n 1 file changed, 27 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 62cfa50c50f..8bb875532b4 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -732,14 +732,23 @@ static void midx_included_packs(struct string_list *include,\n \t\t\t\tstruct pack_geometry *geometry)\n {\n \tstruct string_list_item *item;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tfor_each_string_list_item(item, &existing->kept_packs) {\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"%s.idx\", item->string);\n+\t\tstring_list_insert(include, buf.buf);\n+\t}\n+\n+\tfor_each_string_list_item(item, names) {\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"pack-%s.idx\", item->string);\n+\t\tstring_list_insert(include, buf.buf);\n+\t}\n \n-\tfor_each_string_list_item(item, &existing->kept_packs)\n-\t\tstring_list_insert(include, xstrfmt(\"%s.idx\", item->string));\n-\tfor_each_string_list_item(item, names)\n-\t\tstring_list_insert(include, xstrfmt(\"pack-%s.idx\", item->string));\n \tif (geometry->split_factor) {\n-\t\tstruct strbuf buf = STRBUF_INIT;\n \t\tuint32_t i;\n+\n \t\tfor (i = geometry->split; i < geometry->pack_nr; i++) {\n \t\t\tstruct packed_git *p = geometry->pack[i];\n \n@@ -754,17 +763,21 @@ static void midx_included_packs(struct string_list *include,\n \t\t\tif (!p->pack_local)\n \t\t\t\tcontinue;\n \n+\t\t\tstrbuf_reset(&buf);\n \t\t\tstrbuf_addstr(&buf, pack_basename(p));\n \t\t\tstrbuf_strip_suffix(&buf, \".pack\");\n \t\t\tstrbuf_addstr(&buf, \".idx\");\n \n-\t\t\tstring_list_insert(include, strbuf_detach(&buf, NULL));\n+\t\t\tstring_list_insert(include, buf.buf);\n \t\t}\n \t} else {\n \t\tfor_each_string_list_item(item, &existing->non_kept_packs) {\n \t\t\tif (pack_is_marked_for_deletion(item))\n \t\t\t\tcontinue;\n-\t\t\tstring_list_insert(include, xstrfmt(\"%s.idx\", item->string));\n+\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_addf(&buf, \"%s.idx\", item->string);\n+\t\t\tstring_list_insert(include, buf.buf);\n \t\t}\n \t}\n \n@@ -784,8 +797,13 @@ static void midx_included_packs(struct string_list *include,\n \t\t */\n \t\tif (pack_is_marked_for_deletion(item))\n \t\t\tcontinue;\n-\t\tstring_list_insert(include, xstrfmt(\"%s.idx\", item->string));\n+\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"%s.idx\", item->string);\n+\t\tstring_list_insert(include, buf.buf);\n \t}\n+\n+\tstrbuf_release(&buf);\n }\n \n static int write_midx_included_packs(struct string_list *include,\n@@ -1476,7 +1494,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \t\tmark_packs_for_deletion(&existing, &names);\n \n \tif (write_midx) {\n-\t\tstruct string_list include = STRING_LIST_INIT_NODUP;\n+\t\tstruct string_list include = STRING_LIST_INIT_DUP;\n \t\tmidx_included_packs(&include, &existing, &names, &geometry);\n \n \t\tret = write_midx_included_packs(&include, &geometry, &names,\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501515","messageId":"48b60279d18ced93614c15772a9344ab7e337b6f.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 10/20] t/helper: fix leaking multi-pack-indices in \"read-midx\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:38Z","receivedAt":"2024-08-22T09:17:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Several of the subcommands of `test-helper read-midx` do not close the\nMIDX that they have opened, leading to memory leaks. Fix those.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-read-midx.c | 8 +++++++-\n t/t5319-multi-pack-index.sh | 2 ++\n t/t7704-repack-cruft.sh | 1 +\n 3 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\nindex 69757e94fc2..438fb9fc619 100644\n--- a/t/helper/test-read-midx.c\n+++ b/t/helper/test-read-midx.c\n@@ -86,6 +86,8 @@ static int read_midx_checksum(const char *object_dir)\n \tif (!m)\n \t\treturn 1;\n \tprintf(\"%s\\n\", hash_to_hex(get_midx_checksum(m)));\n+\n+\tclose_midx(m);\n \treturn 0;\n }\n \n@@ -102,10 +104,12 @@ static int read_midx_preferred_pack(const char *object_dir)\n \n \tif (midx_preferred_pack(midx, &preferred_pack) < 0) {\n \t\twarning(_(\"could not determine MIDX preferred pack\"));\n+\t\tclose_midx(midx);\n \t\treturn 1;\n \t}\n \n \tprintf(\"%s\\n\", midx->pack_names[preferred_pack]);\n+\tclose_midx(midx);\n \treturn 0;\n }\n \n@@ -122,8 +126,10 @@ static int read_midx_bitmapped_packs(const char *object_dir)\n \t\treturn 1;\n \n \tfor (i = 0; i < midx->num_packs + midx->num_packs_in_base; i++) {\n-\t\tif (nth_bitmapped_pack(the_repository, midx, &pack, i) < 0)\n+\t\tif (nth_bitmapped_pack(the_repository, midx, &pack, i) < 0) {\n+\t\t\tclose_midx(midx);\n \t\t\treturn 1;\n+\t\t}\n \n \t\tprintf(\"%s\\n\", pack_basename(pack.p));\n \t\tprintf(\" bitmap_pos: %\"PRIuMAX\"\\n\", (uintmax_t)pack.bitmap_pos);\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex ce1b58c7323..fbbc218d04a 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='multi-pack-indexes'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-chunk.sh\n . \"$TEST_DIRECTORY\"/lib-midx.sh\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 959e6e26488..5db9f4e10f7 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -2,6 +2,7 @@\n \n test_description='git repack works correctly'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n objdir=.git/objects\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501516","messageId":"0cb440ef6482a09c3b25435bb9f0e9b0088a7b29.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 11/20] transport: fix leaking OID arrays in git:// transport data","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:41Z","receivedAt":"2024-08-22T09:17:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The transport data for the \"git://\" protocol contains two OID arrays\nthat we never free, creating a memory leak. Plug them.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5501-fetch-push-alternates.sh | 1 +\n t/t5519-push-alternates.sh | 1 +\n transport.c | 2 ++\n 3 files changed, 4 insertions(+)\n\ndiff --git a/t/t5501-fetch-push-alternates.sh b/t/t5501-fetch-push-alternates.sh\nindex 66f19a4ef2b..0c8668a1b8e 100755\n--- a/t/t5501-fetch-push-alternates.sh\n+++ b/t/t5501-fetch-push-alternates.sh\n@@ -4,6 +4,7 @@ test_description='fetch/push involving alternates'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n count_objects () {\ndiff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh\nindex 20ba604dfde..72e97b15fab 100755\n--- a/t/t5519-push-alternates.sh\n+++ b/t/t5519-push-alternates.sh\n@@ -5,6 +5,7 @@ test_description='push to a repository that borrows from elsewhere'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/transport.c b/transport.c\nindex 7c4af9f56f2..f0672fdc505 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -946,6 +946,8 @@ static int disconnect_git(struct transport *transport)\n \t}\n \n \tlist_objects_filter_release(&data->options.filter_options);\n+\toid_array_clear(&data->extra_have);\n+\toid_array_clear(&data->shallow);\n \tfree(data);\n \treturn 0;\n }\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501517","messageId":"f4300c9326b6ab4e641cde8efd9ca87daf368628.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 12/20] builtin/send-pack: fix leaking refspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:46Z","receivedAt":"2024-08-22T09:17:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We never free data associated with the assembled refspec in\ngit-send-pack(1), causing a memory leak. Fix this.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/send-pack.c | 1 +\n t/t5400-send-pack.sh | 1 +\n t/t5401-update-hooks.sh | 2 ++\n t/t5408-send-pack-stdin.sh | 2 ++\n t/t5548-push-porcelain.sh | 1 +\n t/t5812-proto-disable-http.sh | 2 ++\n 6 files changed, 9 insertions(+)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 17cae6bbbdf..ef0df808249 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -338,5 +338,6 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \n \tfree_refs(remote_refs);\n \tfree_refs(local_refs);\n+\trefspec_clear(&rs);\n \treturn ret;\n }\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 3f81f16e133..248c74d8ef2 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -9,6 +9,7 @@ test_description='See why rewinding head breaks send-pack\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n cnt=64\ndiff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh\nindex d8cadeec733..3c1ea6086e7 100755\n--- a/t/t5401-update-hooks.sh\n+++ b/t/t5401-update-hooks.sh\n@@ -4,6 +4,8 @@\n #\n \n test_description='Test the update hook infrastructure.'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t5408-send-pack-stdin.sh b/t/t5408-send-pack-stdin.sh\nindex e8737df6f95..c3695a4d4e3 100755\n--- a/t/t5408-send-pack-stdin.sh\n+++ b/t/t5408-send-pack-stdin.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='send-pack --stdin tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n create_ref () {\ndiff --git a/t/t5548-push-porcelain.sh b/t/t5548-push-porcelain.sh\nindex 6282728eaf3..ecb3877aa4b 100755\n--- a/t/t5548-push-porcelain.sh\n+++ b/t/t5548-push-porcelain.sh\n@@ -4,6 +4,7 @@\n #\n test_description='Test git push porcelain output'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # Create commits in and assign each commit's oid to shell variables\ndiff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh\nindex 769c717e88b..f69959c64ca 100755\n--- a/t/t5812-proto-disable-http.sh\n+++ b/t/t5812-proto-disable-http.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='test disabling of git-over-http in clone/fetch'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-proto-disable.sh\"\n . \"$TEST_DIRECTORY/lib-httpd.sh\"\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501518","messageId":"28805c15a425f822ef1851affa14803a5499b6d4.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 13/20] sideband: fix leaks when configuring sideband colors","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:49Z","receivedAt":"2024-08-22T09:17:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We read a bunch of configs in `use_sideband_colors()` to configure the\ncolors that Git should use. We never free the strings read from the\nconfig though, causing memory leaks.\n\nRefactor the code to use `git_config_get_string_tmp()` instead, which\ndoes not allocate memory. As we throw the strings away after parsing\nthem anyway there is no need to use allocated strings.\n\nSigned-off-by: Patrick Steinhardt \n---\n sideband.c | 15 +++++++--------\n t/t5409-colorize-remote-messages.sh | 1 +\n 2 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex 5d8907151fe..27853736901 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -30,28 +30,27 @@ static int use_sideband_colors(void)\n \n \tconst char *key = \"color.remote\";\n \tstruct strbuf sb = STRBUF_INIT;\n-\tchar *value;\n+\tconst char *value;\n \tint i;\n \n \tif (use_sideband_colors_cached >= 0)\n \t\treturn use_sideband_colors_cached;\n \n-\tif (!git_config_get_string(key, &value)) {\n+\tif (!git_config_get_string_tmp(key, &value))\n \t\tuse_sideband_colors_cached = git_config_colorbool(key, value);\n-\t} else if (!git_config_get_string(\"color.ui\", &value)) {\n+\telse if (!git_config_get_string_tmp(\"color.ui\", &value))\n \t\tuse_sideband_colors_cached = git_config_colorbool(\"color.ui\", value);\n-\t} else {\n+\telse\n \t\tuse_sideband_colors_cached = GIT_COLOR_AUTO;\n-\t}\n \n \tfor (i = 0; i < ARRAY_SIZE(keywords); i++) {\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_addf(&sb, \"%s.%s\", key, keywords[i].keyword);\n-\t\tif (git_config_get_string(sb.buf, &value))\n-\t\t\tcontinue;\n-\t\tif (color_parse(value, keywords[i].color))\n+\t\tif (git_config_get_string_tmp(sb.buf, &value))\n \t\t\tcontinue;\n+\t\tcolor_parse(value, keywords[i].color);\n \t}\n+\n \tstrbuf_release(&sb);\n \treturn use_sideband_colors_cached;\n }\ndiff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh\nindex fa5de4500a4..516b22fd963 100755\n--- a/t/t5409-colorize-remote-messages.sh\n+++ b/t/t5409-colorize-remote-messages.sh\n@@ -2,6 +2,7 @@\n \n test_description='remote messages are colorized on the client'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501519","messageId":"aac84c5a2b7822983822f4b9dcfd65af0e808208.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 14/20] builtin/fetch-pack: fix leaking refs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:52Z","receivedAt":"2024-08-22T09:17:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We build several ref lists in git-fetch-pack(1), but never free them.\nFix those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/fetch-pack.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex af329e8d5cf..fe404d1305b 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -46,7 +46,7 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,\n int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n {\n \tint i, ret;\n-\tstruct ref *ref = NULL;\n+\tstruct ref *fetched_refs = NULL, *remote_refs = NULL;\n \tconst char *dest = NULL;\n \tstruct ref **sought = NULL;\n \tint nr_sought = 0, alloc_sought = 0;\n@@ -228,19 +228,20 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n \tversion = discover_version(&reader);\n \tswitch (version) {\n \tcase protocol_v2:\n-\t\tget_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL,\n+\t\tget_remote_refs(fd[1], &reader, &remote_refs, 0, NULL, NULL,\n \t\t\t\targs.stateless_rpc);\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n-\t\tget_remote_heads(&reader, &ref, 0, NULL, &shallow);\n+\t\tget_remote_heads(&reader, &remote_refs, 0, NULL, &shallow);\n \t\tbreak;\n \tcase protocol_unknown_version:\n \t\tBUG(\"unknown protocol version\");\n \t}\n \n-\tref = fetch_pack(&args, fd, ref, sought, nr_sought,\n+\tfetched_refs = fetch_pack(&args, fd, remote_refs, sought, nr_sought,\n \t\t\t &shallow, pack_lockfiles_ptr, version);\n+\n \tif (pack_lockfiles.nr) {\n \t\tint i;\n \n@@ -260,7 +261,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n \tif (finish_connect(conn))\n \t\treturn 1;\n \n-\tret = !ref;\n+\tret = !fetched_refs;\n \n \t/*\n \t * If the heads to pull were given, we should have consumed\n@@ -270,11 +271,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)\n \t */\n \tret |= report_unmatched_refs(sought, nr_sought);\n \n-\twhile (ref) {\n+\tfor (struct ref *ref = fetched_refs; ref; ref = ref->next)\n \t\tprintf(\"%s %s\\n\",\n \t\t oid_to_hex(&ref->old_oid), ref->name);\n-\t\tref = ref->next;\n-\t}\n \n+\tfor (size_t i = 0; i < nr_sought; i++)\n+\t\tfree_one_ref(sought[i]);\n+\tfree(sought);\n+\tfree_refs(fetched_refs);\n+\tfree_refs(remote_refs);\n \treturn ret;\n }\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501520","messageId":"532328b78140620549385c7e2dff3ecc98036593.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 15/20] remote: fix leaking config strings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:55Z","receivedAt":"2024-08-22T09:17:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We're leaking several config strings when assembling remotes, either\nbecause we do not free preceding values in case a config was set\nmultiple times, or because we do not free them when releasing the remote\nstate. This includes config strings for \"branch\" sections, \"insteadOf\",\n\"pushInsteadOf\", and \"pushDefault\".\n\nPlug those leaks.\n\nSigned-off-by: Patrick Steinhardt \n---\n remote.c | 40 ++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 7d5b8f750d8..2c52119bbb2 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -243,6 +243,17 @@ static struct branch *make_branch(struct remote_state *remote_state,\n \treturn ret;\n }\n \n+static void branch_release(struct branch *branch)\n+{\n+\tfree((char *)branch->name);\n+\tfree((char *)branch->refname);\n+\tfree(branch->remote_name);\n+\tfree(branch->pushremote_name);\n+\tfor (int i = 0; i < branch->merge_nr; i++)\n+\t\trefspec_item_clear(branch->merge[i]);\n+\tfree(branch->merge);\n+}\n+\n static struct rewrite *make_rewrite(struct rewrites *r,\n \t\t\t\t const char *base, size_t len)\n {\n@@ -263,6 +274,14 @@ static struct rewrite *make_rewrite(struct rewrites *r,\n \treturn ret;\n }\n \n+static void rewrites_release(struct rewrites *r)\n+{\n+\tfor (int i = 0; i < r->rewrite_nr; i++)\n+\t\tfree((char *)r->rewrite[i]->base);\n+\tfree(r->rewrite);\n+\tmemset(r, 0, sizeof(*r));\n+}\n+\n static void add_instead_of(struct rewrite *rewrite, const char *instead_of)\n {\n \tALLOC_GROW(rewrite->instead_of, rewrite->instead_of_nr + 1, rewrite->instead_of_alloc);\n@@ -373,8 +392,10 @@ static int handle_config(const char *key, const char *value,\n \t\t\treturn -1;\n \t\tbranch = make_branch(remote_state, name, namelen);\n \t\tif (!strcmp(subkey, \"remote\")) {\n+\t\t\tFREE_AND_NULL(branch->remote_name);\n \t\t\treturn git_config_string(&branch->remote_name, key, value);\n \t\t} else if (!strcmp(subkey, \"pushremote\")) {\n+\t\t\tFREE_AND_NULL(branch->pushremote_name);\n \t\t\treturn git_config_string(&branch->pushremote_name, key, value);\n \t\t} else if (!strcmp(subkey, \"merge\")) {\n \t\t\tif (!value)\n@@ -406,9 +427,11 @@ static int handle_config(const char *key, const char *value,\n \t\treturn 0;\n \n \t/* Handle remote.* variables */\n-\tif (!name && !strcmp(subkey, \"pushdefault\"))\n+\tif (!name && !strcmp(subkey, \"pushdefault\")) {\n+\t\tFREE_AND_NULL(remote_state->pushremote_name);\n \t\treturn git_config_string(&remote_state->pushremote_name, key,\n \t\t\t\t\t value);\n+\t}\n \n \tif (!name)\n \t\treturn 0;\n@@ -475,12 +498,15 @@ static int handle_config(const char *key, const char *value,\n \t\telse if (!strcmp(value, \"--tags\"))\n \t\t\tremote->fetch_tags = 2;\n \t} else if (!strcmp(subkey, \"proxy\")) {\n+\t\tFREE_AND_NULL(remote->http_proxy);\n \t\treturn git_config_string(&remote->http_proxy,\n \t\t\t\t\t key, value);\n \t} else if (!strcmp(subkey, \"proxyauthmethod\")) {\n+\t\tFREE_AND_NULL(remote->http_proxy_authmethod);\n \t\treturn git_config_string(&remote->http_proxy_authmethod,\n \t\t\t\t\t key, value);\n \t} else if (!strcmp(subkey, \"vcs\")) {\n+\t\tFREE_AND_NULL(remote->foreign_vcs);\n \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n \t}\n \treturn 0;\n@@ -2797,16 +2823,26 @@ struct remote_state *remote_state_new(void)\n \n void remote_state_clear(struct remote_state *remote_state)\n {\n+\tstruct hashmap_iter iter;\n+\tstruct branch *b;\n \tint i;\n \n \tfor (i = 0; i < remote_state->remotes_nr; i++)\n \t\tremote_clear(remote_state->remotes[i]);\n \tFREE_AND_NULL(remote_state->remotes);\n+\tFREE_AND_NULL(remote_state->pushremote_name);\n \tremote_state->remotes_alloc = 0;\n \tremote_state->remotes_nr = 0;\n \n+\trewrites_release(&remote_state->rewrites);\n+\trewrites_release(&remote_state->rewrites_push);\n+\n \thashmap_clear_and_free(&remote_state->remotes_hash, struct remote, ent);\n-\thashmap_clear_and_free(&remote_state->branches_hash, struct remote, ent);\n+\thashmap_for_each_entry(&remote_state->branches_hash, &iter, b, ent) {\n+\t\tbranch_release(b);\n+\t\tfree(b);\n+\t}\n+\thashmap_clear(&remote_state->branches_hash);\n }\n \n /*\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501521","messageId":"440b3d993724c465971628e9548c4937a7cd0833.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 16/20] remote: fix leaks when matching refspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:17:58Z","receivedAt":"2024-08-22T09:18:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `match_explicit()`, we try to match a source ref with a destination\nref according to a refspec item. This matching sometimes requires us to\nallocate a new source spec so that it looks like we expect. And while we\nin some end up assigning this allocated ref as `peer_ref`, which hands\nover ownership of it to the caller, in other cases we don't. We neither\nfree it though, causing a memory leak.\n\nFix the leak by creating a common exit path where we can easily free the\nsource ref in case it is allocated and hasn't been handed over to the\ncaller.\n\nSigned-off-by: Patrick Steinhardt \n---\n remote.c | 43 +++++++++++++++++++++++++++++--------------\n t/t5505-remote.sh | 1 +\n 2 files changed, 30 insertions(+), 14 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 2c52119bbb2..6ea81f9665b 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1344,18 +1344,21 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\t struct ref ***dst_tail,\n \t\t\t struct refspec_item *rs)\n {\n-\tstruct ref *matched_src, *matched_dst;\n-\tint allocated_src;\n+\tstruct ref *matched_src = NULL, *matched_dst = NULL;\n+\tint allocated_src = 0, ret;\n \n \tconst char *dst_value = rs->dst;\n \tchar *dst_guess;\n \n-\tif (rs->pattern || rs->matching || rs->negative)\n-\t\treturn 0;\n+\tif (rs->pattern || rs->matching || rs->negative) {\n+\t\tret = 0;\n+\t\tgoto out;\n+\t}\n \n-\tmatched_src = matched_dst = NULL;\n-\tif (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0)\n-\t\treturn -1;\n+\tif (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n \n \tif (!dst_value) {\n \t\tint flag;\n@@ -1394,18 +1397,30 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t dst_value);\n \t\tbreak;\n \t}\n-\tif (!matched_dst)\n-\t\treturn -1;\n-\tif (matched_dst->peer_ref)\n-\t\treturn error(_(\"dst ref %s receives from more than one src\"),\n-\t\t\t matched_dst->name);\n-\telse {\n+\n+\tif (!matched_dst) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n+\n+\tif (matched_dst->peer_ref) {\n+\t\tret = error(_(\"dst ref %s receives from more than one src\"),\n+\t\t\t matched_dst->name);\n+\t\tgoto out;\n+\t} else {\n \t\tmatched_dst->peer_ref = allocated_src ?\n \t\t\t\t\tmatched_src :\n \t\t\t\t\tcopy_ref(matched_src);\n \t\tmatched_dst->force = rs->force;\n+\t\tmatched_src = NULL;\n \t}\n-\treturn 0;\n+\n+\tret = 0;\n+\n+out:\n+\tif (allocated_src)\n+\t\tfree_one_ref(matched_src);\n+\treturn ret;\n }\n \n static int match_explicit_refs(struct ref *src, struct ref *dst,\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 08424e878e1..532035933f3 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -2,6 +2,7 @@\n \n test_description='git remote porcelain-ish'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n setup_repository () {\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501522","messageId":"662ec4e6484aa7fc495d528a0b58ef18e3be1953.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 17/20] remote: fix leaking peer ref when expanding refmap","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:18:00Z","receivedAt":"2024-08-22T09:18:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When expanding remote refs via the refspec in `get_expanded_map()`, we\nfirst copy the remote ref and then override its peer ref with the\nexpanded name. This may cause a memory leak though in case the peer ref\nis already set, as this field is being copied by `copy_ref()`, as well.\n\nFix the leak by freeing the peer ref before we re-assign the field.\n\nSigned-off-by: Patrick Steinhardt \n---\n remote.c | 2 ++\n t/t5536-fetch-conflicts.sh | 1 +\n t/t5553-set-upstream.sh | 1 +\n t/t5703-upload-pack-ref-in-want.sh | 1 +\n t/t6050-replace.sh | 1 +\n 5 files changed, 6 insertions(+)\n\ndiff --git a/remote.c b/remote.c\nindex 6ea81f9665b..8f3dee13186 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2081,6 +2081,8 @@ static struct ref *get_expanded_map(const struct ref *remote_refs,\n \t\t !ignore_symref_update(expn_name, &scratch)) {\n \t\t\tstruct ref *cpy = copy_ref(ref);\n \n+\t\t\tif (cpy->peer_ref)\n+\t\t\t\tfree_one_ref(cpy->peer_ref);\n \t\t\tcpy->peer_ref = alloc_ref(expn_name);\n \t\t\tif (refspec->force)\n \t\t\t\tcpy->peer_ref->force = 1;\ndiff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh\nindex 23bf6961700..2dcbe790523 100755\n--- a/t/t5536-fetch-conflicts.sh\n+++ b/t/t5536-fetch-conflicts.sh\n@@ -2,6 +2,7 @@\n \n test_description='fetch handles conflicting refspecs correctly'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n D=$(pwd)\ndiff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh\nindex 70e3376d31b..33e919a17e1 100755\n--- a/t/t5553-set-upstream.sh\n+++ b/t/t5553-set-upstream.sh\n@@ -4,6 +4,7 @@ test_description='\"git fetch/pull --set-upstream\" basic tests.'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n check_config () {\ndiff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh\nindex 191097171bc..f75fae52c83 100755\n--- a/t/t5703-upload-pack-ref-in-want.sh\n+++ b/t/t5703-upload-pack-ref-in-want.sh\n@@ -2,6 +2,7 @@\n \n test_description='upload-pack ref-in-want'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n get_actual_refs () {\ndiff --git a/t/t6050-replace.sh b/t/t6050-replace.sh\nindex c6e9b33e44e..d7702fc7562 100755\n--- a/t/t6050-replace.sh\n+++ b/t/t6050-replace.sh\n@@ -7,6 +7,7 @@ test_description='Tests replace refs functionality'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501523","messageId":"dbd8eaa2cb1413299d2511f5291e9b91b8828a53.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 18/20] builtin/fetch: fix leaking transaction with `--atomic`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:18:06Z","receivedAt":"2024-08-22T09:18:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"With the `--atomic` flag, we use a single ref transaction to commit all\nref updates in git-fetch(1). The lifetime of transactions is somewhat\nweird: while `ref_transaction_abort()` will free the transaction, a call\nto `ref_transaction_commit()` won't. We thus have to manually free the\ntransaction in the successful case.\n\nAdapt the code to free the transaction in the exit path to plug the\nresulting memory leak. As `ref_transaction_abort()` already freed the\ntransaction for us, we have to unset the transaction when we hit that\ncode path to not cause a double free.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/fetch.c | 8 ++++----\n t/t5574-fetch-output.sh | 1 +\n 2 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex c297569a473..0264483c0e5 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1731,11 +1731,8 @@ static int do_fetch(struct transport *transport,\n \t\t\tgoto cleanup;\n \n \t\tretcode = ref_transaction_commit(transaction, &err);\n-\t\tif (retcode) {\n-\t\t\tref_transaction_free(transaction);\n-\t\t\ttransaction = NULL;\n+\t\tif (retcode)\n \t\t\tgoto cleanup;\n-\t\t}\n \t}\n \n \tcommit_fetch_head(&fetch_head);\n@@ -1803,8 +1800,11 @@ static int do_fetch(struct transport *transport,\n \t\tif (transaction && ref_transaction_abort(transaction, &err) &&\n \t\t err.len)\n \t\t\terror(\"%s\", err.buf);\n+\t\ttransaction = NULL;\n \t}\n \n+\tif (transaction)\n+\t\tref_transaction_free(transaction);\n \tdisplay_state_release(&display_state);\n \tclose_fetch_head(&fetch_head);\n \tstrbuf_release(&err);\ndiff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh\nindex 5883839a04e..f7707326ea1 100755\n--- a/t/t5574-fetch-output.sh\n+++ b/t/t5574-fetch-output.sh\n@@ -5,6 +5,7 @@ test_description='git fetch output format'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'fetch with invalid output format configuration' '\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501524","messageId":"4c5740afe43c3bb619a9cba0c1634097c27ed33f.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 19/20] transport: fix leaking arguments when fetching from bundle","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:18:08Z","receivedAt":"2024-08-22T09:18:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass\nto `unbundle()`, but never free it. And in theory we wouldn't have to\nbecause `unbundle()` already knows to free the vector for us. But it\nfails to do so when it exits early due to `verify_bundle()` failing.\n\nThe calling convention that the arguments are freed by the callee and\nnot the caller feels somewhat weird. Refactor the code such that it is\ninstead the responsibility of the caller to free the vector, adapting\nthe only two callsites where we pass extra arguments. This also fixes\nthe memory leak.\n\nThis memory leak gets hit in t5510, but fixing it isn't sufficient to\nmake the whole test suite pass.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/bundle.c | 2 ++\n bundle.c | 4 +---\n transport.c | 2 ++\n 3 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/bundle.c b/builtin/bundle.c\nindex d5d41a8f672..df97f399019 100644\n--- a/builtin/bundle.c\n+++ b/builtin/bundle.c\n@@ -220,7 +220,9 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)\n \t\t\t &extra_index_pack_args, 0) ||\n \t\tlist_bundle_refs(&header, argc, argv);\n \tbundle_header_release(&header);\n+\n cleanup:\n+\tstrvec_clear(&extra_index_pack_args);\n \tfree(bundle_file);\n \treturn ret;\n }\ndiff --git a/bundle.c b/bundle.c\nindex ce164c37bc8..0f6c7a71ef1 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -639,10 +639,8 @@ int unbundle(struct repository *r, struct bundle_header *header,\n \tif (flags & VERIFY_BUNDLE_FSCK)\n \t\tstrvec_push(&ip.args, \"--fsck-objects\");\n \n-\tif (extra_index_pack_args) {\n+\tif (extra_index_pack_args)\n \t\tstrvec_pushv(&ip.args, extra_index_pack_args->v);\n-\t\tstrvec_clear(extra_index_pack_args);\n-\t}\n \n \tip.in = bundle_fd;\n \tip.no_stdout = 1;\ndiff --git a/transport.c b/transport.c\nindex f0672fdc505..da639d3bff0 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,\n \t\t &extra_index_pack_args,\n \t\t fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);\n \ttransport->hash_algo = data->header.hash_algo;\n+\n+\tstrvec_clear(&extra_index_pack_args);\n \treturn ret;\n }\n \n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501525","messageId":"c3d2b035761fa8a14df4d128375a24bf5821da9d.1724315484.git.ps@pks.im","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"[PATCH v2 20/20] transport: fix leaking negotiation tips","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T09:18:11Z","receivedAt":"2024-08-22T09:18:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We do not free negotiation tips in the transport's smart options. Fix\nthis by freeing them on disconnect.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/t5510-fetch.sh | 1 +\n transport.c | 4 ++++\n 2 files changed, 5 insertions(+)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 3b3991ab867..0890b9f61c5 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -5,6 +5,7 @@ test_description='Per branch config variables affects \"git fetch\".\n \n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-bundle.sh\n \ndiff --git a/transport.c b/transport.c\nindex da639d3bff0..0f20fc56e40 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -947,6 +947,10 @@ static int disconnect_git(struct transport *transport)\n \t\tfinish_connect(data->conn);\n \t}\n \n+\tif (data->options.negotiation_tips) {\n+\t\toid_array_clear(data->options.negotiation_tips);\n+\t\tfree(data->options.negotiation_tips);\n+\t}\n \tlist_objects_filter_release(&data->options.filter_options);\n \toid_array_clear(&data->extra_have);\n \toid_array_clear(&data->shallow);\n-- \n2.46.0.164.g477ce5ccd6.dirty\n\n"},{"id":"501544","messageId":"xmqqcym0tuhb.fsf@gitster.g","threadId":"61982","inReplyTo":"Zsb0nspisMs1Csl-@tanuki","subject":"Re: [PATCH 15/20] remote: fix leaking config strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-22T16:04:00Z","receivedAt":"2024-08-22T16:04:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Wed, Aug 21, 2024 at 10:58:55AM -0700, Junio C Hamano wrote:\n>> Patrick Steinhardt writes:\n>> \n>> > @@ -2802,6 +2809,7 @@ void remote_state_clear(struct remote_state *remote_state)\n>> > \tfor (i = 0; i < remote_state->remotes_nr; i++)\n>> > \t\tremote_clear(remote_state->remotes[i]);\n>> > \tFREE_AND_NULL(remote_state->remotes);\n>> > +\tFREE_AND_NULL(remote_state->pushremote_name);\n>> > \tremote_state->remotes_alloc = 0;\n>> > \tremote_state->remotes_nr = 0;\n>> \n>> As remote_state has two extra structures embedded in it, I wonder if\n>> we should be clearing them in this function, but possibly it is\n>> cleared elsewhere or perhaps in a later series?\n>\n> It is not yet part of any subsequent patch series, mostly because I\n> didn't happen to stumble over such leaks yet. Both of the rewrites very\n> much are leaky though, and would be hit when we use \"insteadOf\" or\n> \"pushInsteadOf\" configs.\n\nYes, I was wondering if our test coverage for the feature is\nlacking. In a sense, leaks from these insteadOf configuration\nvariables are tiny and uninteresting, compared to how much it\nbothers me to find us not testing these often used features.\n"},{"id":"501545","messageId":"xmqq8qwotucq.fsf@gitster.g","threadId":"61982","inReplyTo":"Zsb0qU-G4kdlxrWa@tanuki","subject":"Re: [PATCH 19/20] transport: fix leaking arguments when fetching from bundle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-22T16:06:45Z","receivedAt":"2024-08-22T16:06:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Yes we do, because there is an early return in case `verify_bundle()`\n> fails. I didn't notice that we have it in `unbundle()` though.\n\nAh, I missed that one.\n\n> I'd argue it's bad practice to have `unbundle()` clear the caller\n> provided args for us and somewhat surprising.\n\nI was surprised so it makes it at least two of us. In some call\npatterns, having the callee _consume_ what the caller fed it may\nbe natural, but not in this code path.\n\nThanks.\n"},{"id":"501554","messageId":"xmqqmsl4qv02.fsf@gitster.g","threadId":"61982","inReplyTo":"4c5740afe43c3bb619a9cba0c1634097c27ed33f.1724315484.git.ps@pks.im","subject":"Re: [PATCH v2 19/20] transport: fix leaking arguments when fetching from bundle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-22T18:21:01Z","receivedAt":"2024-08-22T18:21:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass\n> to `unbundle()`, but never free it. And in theory we wouldn't have to\n> because `unbundle()` already knows to free the vector for us. But it\n> fails to do so when it exits early due to `verify_bundle()` failing.\n>\n> The calling convention that the arguments are freed by the callee and\n> not the caller feels somewhat weird. Refactor the code such that it is\n> instead the responsibility of the caller to free the vector, adapting\n> the only two callsites where we pass extra arguments. This also fixes\n> the memory leak.\n>\n> This memory leak gets hit in t5510, but fixing it isn't sufficient to\n> make the whole test suite pass.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/bundle.c | 2 ++\n> bundle.c | 4 +---\n> transport.c | 2 ++\n> 3 files changed, 5 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/bundle.c b/builtin/bundle.c\n> index d5d41a8f672..df97f399019 100644\n> --- a/builtin/bundle.c\n> +++ b/builtin/bundle.c\n> @@ -220,7 +220,9 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)\n> \t\t\t &extra_index_pack_args, 0) ||\n> \t\tlist_bundle_refs(&header, argc, argv);\n> \tbundle_header_release(&header);\n> +\n> cleanup:\n> +\tstrvec_clear(&extra_index_pack_args);\n> \tfree(bundle_file);\n> \treturn ret;\n> }\n> diff --git a/bundle.c b/bundle.c\n> index ce164c37bc8..0f6c7a71ef1 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -639,10 +639,8 @@ int unbundle(struct repository *r, struct bundle_header *header,\n> \tif (flags & VERIFY_BUNDLE_FSCK)\n> \t\tstrvec_push(&ip.args, \"--fsck-objects\");\n> \n> -\tif (extra_index_pack_args) {\n> +\tif (extra_index_pack_args)\n> \t\tstrvec_pushv(&ip.args, extra_index_pack_args->v);\n> -\t\tstrvec_clear(extra_index_pack_args);\n> -\t}\n> \n> \tip.in = bundle_fd;\n> \tip.no_stdout = 1;\n> diff --git a/transport.c b/transport.c\n> index f0672fdc505..da639d3bff0 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,\n> \t\t &extra_index_pack_args,\n> \t\t fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);\n> \ttransport->hash_algo = data->header.hash_algo;\n> +\n> +\tstrvec_clear(&extra_index_pack_args);\n> \treturn ret;\n> }\n\nLooks much better. Will queue. Thanks.\n"},{"id":"501556","messageId":"xmqqh6bcqt5e.fsf@gitster.g","threadId":"61982","inReplyTo":"cover.1724315484.git.ps@pks.im","subject":"Re: [PATCH v2 00/20] Memory leak fixes (pt.5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-22T19:01:01Z","receivedAt":"2024-08-22T19:01:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> this is version 2 of this patch series that fixes another set of memory\n> leaks.\n>\n> Changes compared to v1:\n>\n> - Remove spaces between cast and variable.\n>\n> - Clarify why we move the `ctx.entries_nr` block.\n>\n> - Adapt sideband colors to use `git_config_get_string_tmp()` such that\n> we do not have to allocate the strings in the first place.\n>\n> - Fix more memory leaks for config values in the remote state.\n>\n> - Refactor `unbundle()` to not free extra args passed by the caller\n> anymore. Instead, this is now always done by the caler.\n\nAll these changes look sensible to me.\n\nLet me wait for a few days for others' comments, and then mark the\ntopic for 'next'.\n\nThanks.\n"}]}