{"thread":{"id":"56748","subject":"[PATCH 0/6] leaks: miscellaneous small leak fixes","startedAt":"2021-10-21T15:57:44Z","lastAt":"2021-10-22T08:56:03Z","messageCount":20,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"439241","messageId":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","threadId":"56748","inReplyTo":null,"subject":"[PATCH 0/6] leaks: miscellaneous small leak fixes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T15:57:31Z","receivedAt":"2021-10-21T15:57:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A few small miscellaneous memory leak fixes. I'm currently waiting on\nthe other in-flight leak fixes I've got to mark more tests as passing\nunder SANITIZE=leak, these fixes don't make any tests pass by\nthemselves, but in combination with later fixes I've got planned will\nmake some tests scripts pass completely.\n\nÆvar Arnfjörð Bjarmason (6):\n grep: prefer \"struct grep_opt\" over its \"void *\"\n grep: use object_array_clear() in cmd_grep()\n clone: fix a memory leak of the \"git_dir\" variable\n submodule--helper: fix small memory leaks\n reflog: free() ref given to us by dwim_log()\n repack: stop leaking a \"struct child_process\"\n\n builtin/clone.c | 4 +++-\n builtin/grep.c | 5 +++--\n builtin/reflog.c | 1 +\n builtin/repack.c | 4 +++-\n builtin/submodule--helper.c | 2 ++\n 5 files changed, 12 insertions(+), 4 deletions(-)\n\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439242","messageId":"patch-1.6-2bdd21e4e59-20211021T155529Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","subject":"[PATCH 1/6] grep: prefer \"struct grep_opt\" over its \"void *\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T15:57:32Z","receivedAt":"2021-10-21T15:57:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Stylistically fix up code added in bfac23d9534 (grep: Fix two memory\nleaks, 2010-01-30). We usually don't use the \"arg\" at all once we've\nunpacked it into the struct we want, let's not do that here when we're\nfreeing it. Perhaps it was thought that a cast to \"void *\" would\notherwise be needed?\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 8af5249a7bb..fd184c182a3 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -199,8 +199,8 @@ static void *run(void *arg)\n \t\tgrep_source_clear_data(&w->source);\n \t\twork_done(w);\n \t}\n-\tfree_grep_patterns(arg);\n-\tfree(arg);\n+\tfree_grep_patterns(opt);\n+\tfree(opt);\n \n \treturn (void*) (intptr_t) hit;\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439243","messageId":"patch-2.6-727fdb27a2a-20211021T155529Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","subject":"[PATCH 2/6] grep: use object_array_clear() in cmd_grep()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T15:57:33Z","receivedAt":"2021-10-21T15:57:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Free the \"struct object_array\" before exiting. This makes grep tests\n(e.g. \"t7815-grep-binary.sh\") a bit happer under SANITIZE=leak.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex fd184c182a3..555b2ab6008 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\trun_pager(&opt, prefix);\n \tclear_pathspec(&pathspec);\n \tfree_grep_patterns(&opt);\n+\tobject_array_clear(&list);\n \tfree_repos();\n \treturn !hit;\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439244","messageId":"patch-3.6-86d928ae2f9-20211021T155529Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","subject":"[PATCH 3/6] clone: fix a memory leak of the \"git_dir\" variable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T15:57:34Z","receivedAt":"2021-10-21T15:57:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"At this point in cmd_clone the \"git_dir\" is always either an\nxstrdup()'d string, or something we got from mkpathdup(). Let's free()\nit before we clobber it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/clone.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 559acf9e036..fb377b27657 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1040,8 +1040,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tinit_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,\n \t\tINIT_DB_QUIET);\n \n-\tif (real_git_dir)\n+\tif (real_git_dir) {\n+\t\tfree((char *)git_dir);\n \t\tgit_dir = real_git_dir;\n+\t}\n \n \t/*\n \t * additional config can be injected with -c, make sure it's included\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439245","messageId":"patch-4.6-9c3c0529ad0-20211021T155529Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","subject":"[PATCH 4/6] submodule--helper: fix small memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T15:57:35Z","receivedAt":"2021-10-21T15:57:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a missing strbuf_release() and a clear_pathspec() to the\nsubmodule--helper.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/submodule--helper.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6298cbdd4e5..a157656a48a 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3220,6 +3220,7 @@ static void die_on_index_match(const char *path, int force)\n \t\t}\n \t\tfree(ps_matched);\n \t}\n+\tclear_pathspec(&ps);\n }\n \n static void die_on_repo_without_commits(const char *path)\n@@ -3231,6 +3232,7 @@ static void die_on_repo_without_commits(const char *path)\n \t\tif (resolve_gitlink_ref(path, \"HEAD\", &oid) < 0)\n \t\t\tdie(_(\"'%s' does not have a commit checked out\"), path);\n \t}\n+\tstrbuf_release(&sb);\n }\n \n static int module_add(int argc, const char **argv, const char *prefix)\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439246","messageId":"patch-5.6-85b7b7aef37-20211021T155529Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","subject":"[PATCH 5/6] reflog: free() ref given to us by dwim_log()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T15:57:36Z","receivedAt":"2021-10-21T15:57:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When dwim_log() returns the \"ref\" is always ether NULL or an\nxstrdup()'d string.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/reflog.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex bd4c669918d..175c83e7cc2 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -653,6 +653,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tshould_expire_reflog_ent,\n \t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t&cb);\n+\t\tfree(ref);\n \t}\n \treturn status;\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439247","messageId":"patch-6.6-526d5649156-20211021T155529Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","subject":"[PATCH 6/6] repack: stop leaking a \"struct child_process\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-21T15:57:37Z","receivedAt":"2021-10-21T15:57:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Signed-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/repack.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 0b2d1e5d82b..50730517c7b 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -258,9 +258,11 @@ static void repack_promisor_objects(const struct pack_objects_args *args,\n \tfor_each_packed_object(write_oid, &cmd,\n \t\t\t FOR_EACH_OBJECT_PROMISOR_ONLY);\n \n-\tif (cmd.in == -1)\n+\tif (cmd.in == -1) {\n+\t\tchild_process_clear(&cmd);\n \t\t/* No packed objects; cmd was never started */\n \t\treturn;\n+\t}\n \n \tclose(cmd.in);\n \n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439312","messageId":"xmqqv91qey1d.fsf@gitster.g","threadId":"56748","inReplyTo":"patch-1.6-2bdd21e4e59-20211021T155529Z-avarab@gmail.com","subject":"Re: [PATCH 1/6] grep: prefer \"struct grep_opt\" over its \"void *\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-21T23:52:14Z","receivedAt":"2021-10-21T23:52:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Subject: Re: [PATCH 1/6] grep: prefer \"struct grep_opt\" over its \"void *\"\n\nI sense an incomplete sentence. \"counterpart\" or \"equivalent\" is\nmissing at the tail, perhaps?\n\n> Stylistically fix up code added in bfac23d9534 (grep: Fix two memory\n> leaks, 2010-01-30). We usually don't use the \"arg\" at all once we've\n> unpacked it into the struct we want, let's not do that here when we're\n\nAs we do not unpack and instead take it as a whole and cast, \"we've\nunpacked it into\" -> \"we've casted it to\".\n\n\n\n\n> freeing it. Perhaps it was thought that a cast to \"void *\" would\n> otherwise be needed?\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/grep.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 8af5249a7bb..fd184c182a3 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -199,8 +199,8 @@ static void *run(void *arg)\n> \t\tgrep_source_clear_data(&w->source);\n> \t\twork_done(w);\n> \t}\n> -\tfree_grep_patterns(arg);\n> -\tfree(arg);\n> +\tfree_grep_patterns(opt);\n> +\tfree(opt);\n> \n> \treturn (void*) (intptr_t) hit;\n> }\n"},{"id":"439313","messageId":"xmqqr1ceexug.fsf@gitster.g","threadId":"56748","inReplyTo":"patch-2.6-727fdb27a2a-20211021T155529Z-avarab@gmail.com","subject":"Re: [PATCH 2/6] grep: use object_array_clear() in cmd_grep()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-21T23:56:23Z","receivedAt":"2021-10-21T23:56:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Free the \"struct object_array\" before exiting. This makes grep tests\n> (e.g. \"t7815-grep-binary.sh\") a bit happer under SANITIZE=leak.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/grep.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index fd184c182a3..555b2ab6008 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\trun_pager(&opt, prefix);\n> \tclear_pathspec(&pathspec);\n> \tfree_grep_patterns(&opt);\n> +\tobject_array_clear(&list);\n> \tfree_repos();\n> \treturn !hit;\n> }\n\nOK.\n"},{"id":"439314","messageId":"xmqqmtn2exqo.fsf@gitster.g","threadId":"56748","inReplyTo":"patch-2.6-727fdb27a2a-20211021T155529Z-avarab@gmail.com","subject":"Re: [PATCH 2/6] grep: use object_array_clear() in cmd_grep()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-21T23:58:39Z","receivedAt":"2021-10-21T23:58:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Free the \"struct object_array\" before exiting. This makes grep tests\n> (e.g. \"t7815-grep-binary.sh\") a bit happer under SANITIZE=leak.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/grep.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index fd184c182a3..555b2ab6008 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\trun_pager(&opt, prefix);\n> \tclear_pathspec(&pathspec);\n> \tfree_grep_patterns(&opt);\n> +\tobject_array_clear(&list);\n> \tfree_repos();\n> \treturn !hit;\n> }\n\nNot a new issue introduced by this patch, but after run_pager(), it\nseems that opt.output_priv aka path_list is never cleared. Should\nthis step do that, too, as the series never revisits this file?\n"},{"id":"439316","messageId":"xmqqilxpgbvv.fsf@gitster.g","threadId":"56748","inReplyTo":"patch-3.6-86d928ae2f9-20211021T155529Z-avarab@gmail.com","subject":"Re: [PATCH 3/6] clone: fix a memory leak of the \"git_dir\" variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-22T00:07:48Z","receivedAt":"2021-10-22T00:07:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> At this point in cmd_clone the \"git_dir\" is always either an\n> xstrdup()'d string, or something we got from mkpathdup(). Let's free()\n> it before we clobber it.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/clone.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 559acf9e036..fb377b27657 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -1040,8 +1040,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n> \tinit_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,\n> \t\tINIT_DB_QUIET);\n> \n> -\tif (real_git_dir)\n> +\tif (real_git_dir) {\n> +\t\tfree((char *)git_dir);\n> \t\tgit_dir = real_git_dir;\n> +\t}\n> \n> \t/*\n> \t * additional config can be injected with -c, make sure it's included\n\nI had to wonder if the old git_dir can still be pointed at by\njunk_git_dir. Much earlier than this point there is this:\n\n\tif (real_git_dir) {\n\t\tif (real_dest_exists)\n\t\t\tjunk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;\n\t\tjunk_git_dir = real_git_dir;\n\t} else {\n\t\tif (dest_exists)\n\t\t\tjunk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;\n\t\tjunk_git_dir = git_dir;\n\t}\n\tif (safe_create_leading_directories_const(git_dir) < 0)\n\t\tdie(_(\"could not create leading directories of '%s'\"), git_dir);\n\nLuckily, junk_git_dir gets git_dir only when !real_git_dir, so it is\nsafe. real_git_dir can only be set via the --separate-git-dir\ncommand line option, so we are safe here.\n\nThanks.\n"},{"id":"439317","messageId":"xmqqee8dgbhq.fsf@gitster.g","threadId":"56748","inReplyTo":"patch-5.6-85b7b7aef37-20211021T155529Z-avarab@gmail.com","subject":"Re: [PATCH 5/6] reflog: free() ref given to us by dwim_log()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-22T00:16:17Z","receivedAt":"2021-10-22T00:16:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> When dwim_log() returns the \"ref\" is always ether NULL or an\n> xstrdup()'d string.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/reflog.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index bd4c669918d..175c83e7cc2 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -653,6 +653,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\tshould_expire_reflog_ent,\n> \t\t\t\t\treflog_expiry_cleanup,\n> \t\t\t\t\t&cb);\n> +\t\tfree(ref);\n\nOK. When dwim_log() returns false, it would not have touched &ref\nat all, so the \"continue\" (not shown in the context above) is not\nleaking, and this loop looks good.\n\nThanks.\n\n> \t}\n> \treturn status;\n> }\n"},{"id":"439318","messageId":"xmqqa6j1gb7f.fsf@gitster.g","threadId":"56748","inReplyTo":"patch-6.6-526d5649156-20211021T155529Z-avarab@gmail.com","subject":"Re: [PATCH 6/6] repack: stop leaking a \"struct child_process\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-22T00:22:28Z","receivedAt":"2021-10-22T00:22:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> builtin/repack.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> index 0b2d1e5d82b..50730517c7b 100644\n> --- a/builtin/repack.c\n> +++ b/builtin/repack.c\n> @@ -258,9 +258,11 @@ static void repack_promisor_objects(const struct pack_objects_args *args,\n> \tfor_each_packed_object(write_oid, &cmd,\n> \t\t\t FOR_EACH_OBJECT_PROMISOR_ONLY);\n> \n> -\tif (cmd.in == -1)\n> +\tif (cmd.in == -1) {\n> +\t\tchild_process_clear(&cmd);\n> \t\t/* No packed objects; cmd was never started */\n> \t\treturn;\n> +\t}\n> \n> \tclose(cmd.in);\n\nNot wrong per-se, but let's take the one that is part of Taylor's\n\"plug pack bitmap leaks\" series that plugs the same leak.\n"},{"id":"439351","messageId":"cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-0.6-00000000000-20211021T155529Z-avarab@gmail.com","subject":"[PATCH v2 0/6] leaks: miscellaneous small leak fixes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T08:55:38Z","receivedAt":"2021-10-22T08:55:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A re-roll of these miscellaneous small leak fixes to a address\ncomments on v1. I ejected the patch that conflicted with Taylor's\nversion (sorry, didn't notice it), and fixed an additional leak in\ngrep.c pointed out by Junio. Doing that allowed us to mark a test as\npassing under SANITIZE=leak, with only the minor change of replacing a\n\"git checkout\" with \"git reset --hard\" (as \"checkout\" happens to leak\ncurrently, but I've also got a pending patch for that).\n\nÆvar Arnfjörð Bjarmason (6):\n grep: prefer \"struct grep_opt\" over its \"void *\" equivalent\n grep: use object_array_clear() in cmd_grep()\n grep: fix a \"path_list\" memory leak\n clone: fix a memory leak of the \"git_dir\" variable\n submodule--helper: fix small memory leaks\n reflog: free() ref given to us by dwim_log()\n\n builtin/clone.c | 4 +++-\n builtin/grep.c | 14 ++++++++------\n builtin/reflog.c | 1 +\n builtin/submodule--helper.c | 2 ++\n t/t7811-grep-open.sh | 3 ++-\n 5 files changed, 16 insertions(+), 8 deletions(-)\n\nRange-diff against v1:\n1: 2bdd21e4e59 ! 1: 66c838fd800 grep: prefer \"struct grep_opt\" over its \"void *\"\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - grep: prefer \"struct grep_opt\" over its \"void *\"\n + grep: prefer \"struct grep_opt\" over its \"void *\" equivalent\n \n Stylistically fix up code added in bfac23d9534 (grep: Fix two memory\n leaks, 2010-01-30). We usually don't use the \"arg\" at all once we've\n - unpacked it into the struct we want, let's not do that here when we're\n + casted it to the struct we want, let's not do that here when we're\n freeing it. Perhaps it was thought that a cast to \"void *\" would\n otherwise be needed?\n \n2: 727fdb27a2a = 2: 033ca3f7b4f grep: use object_array_clear() in cmd_grep()\n-: ----------- > 3: 8e941e40711 grep: fix a \"path_list\" memory leak\n3: 86d928ae2f9 = 4: 0d0e6359cf4 clone: fix a memory leak of the \"git_dir\" variable\n4: 9c3c0529ad0 = 5: a529c04a29a submodule--helper: fix small memory leaks\n5: 85b7b7aef37 = 6: 6ea5e611ae0 reflog: free() ref given to us by dwim_log()\n6: 526d5649156 < -: ----------- repack: stop leaking a \"struct child_process\"\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439350","messageId":"patch-v2-1.6-66c838fd800-20211022T085306Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com","subject":"[PATCH v2 1/6] grep: prefer \"struct grep_opt\" over its \"void *\" equivalent","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T08:55:39Z","receivedAt":"2021-10-22T08:55:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Stylistically fix up code added in bfac23d9534 (grep: Fix two memory\nleaks, 2010-01-30). We usually don't use the \"arg\" at all once we've\ncasted it to the struct we want, let's not do that here when we're\nfreeing it. Perhaps it was thought that a cast to \"void *\" would\notherwise be needed?\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 8af5249a7bb..fd184c182a3 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -199,8 +199,8 @@ static void *run(void *arg)\n \t\tgrep_source_clear_data(&w->source);\n \t\twork_done(w);\n \t}\n-\tfree_grep_patterns(arg);\n-\tfree(arg);\n+\tfree_grep_patterns(opt);\n+\tfree(opt);\n \n \treturn (void*) (intptr_t) hit;\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439352","messageId":"patch-v2-2.6-033ca3f7b4f-20211022T085306Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com","subject":"[PATCH v2 2/6] grep: use object_array_clear() in cmd_grep()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T08:55:40Z","receivedAt":"2021-10-22T08:55:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Free the \"struct object_array\" before exiting. This makes grep tests\n(e.g. \"t7815-grep-binary.sh\") a bit happer under SANITIZE=leak.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex fd184c182a3..555b2ab6008 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\trun_pager(&opt, prefix);\n \tclear_pathspec(&pathspec);\n \tfree_grep_patterns(&opt);\n+\tobject_array_clear(&list);\n \tfree_repos();\n \treturn !hit;\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439353","messageId":"patch-v2-3.6-8e941e40711-20211022T085306Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com","subject":"[PATCH v2 3/6] grep: fix a \"path_list\" memory leak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T08:55:41Z","receivedAt":"2021-10-22T08:56:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Free the \"path_list\" used in builtin/grep.c, it was declared as\nSTRING_LIST_INIT_NODUP, let's change it to a STRING_LIST_INIT_DUP\nsince an early user in cmd_grep() appends a string passed via\nparse-options.c to it, which needs to be duplicated.\n\nLet's then convert the remaining callers to use\nstring_list_append_nodup() instead, allowing us to free the list.\n\nThis makes all the tests in t7811-grep-open.sh pass, 6/10 would fail\nbefore this change. The only remaining failure would have been due to\na stray \"git checkout\" (which still leaks memory). In this case we can\nuse a \"git reset --hard\" instead, so let's do that, and move the\ntest_when_finished() above the code that would modify the relevant\nfile.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/grep.c | 9 +++++----\n t/t7811-grep-open.sh | 3 ++-\n 2 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 555b2ab6008..9e34a820ad4 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -401,7 +401,7 @@ static void append_path(struct grep_opt *opt, const void *data, size_t len)\n \n \tif (len == 1 && *(const char *)data == '\\0')\n \t\treturn;\n-\tstring_list_append(path_list, xstrndup(data, len));\n+\tstring_list_append_nodup(path_list, xstrndup(data, len));\n }\n \n static void run_pager(struct grep_opt *opt, const char *prefix)\n@@ -839,7 +839,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tstruct grep_opt opt;\n \tstruct object_array list = OBJECT_ARRAY_INIT;\n \tstruct pathspec pathspec;\n-\tstruct string_list path_list = STRING_LIST_INIT_NODUP;\n+\tstruct string_list path_list = STRING_LIST_INIT_DUP;\n \tint i;\n \tint dummy;\n \tint use_index = 1;\n@@ -1159,8 +1159,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tstrbuf_addf(&buf, \"+/%s%s\",\n \t\t\t\t\tstrcmp(\"less\", pager) ? \"\" : \"*\",\n \t\t\t\t\topt.pattern_list->pattern);\n-\t\t\tstring_list_append(&path_list,\n-\t\t\t\t\t strbuf_detach(&buf, NULL));\n+\t\t\tstring_list_append_nodup(&path_list,\n+\t\t\t\t\t\t strbuf_detach(&buf, NULL));\n \t\t}\n \t}\n \n@@ -1195,6 +1195,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\n \tclear_pathspec(&pathspec);\n+\tstring_list_clear(&path_list, 0);\n \tfree_grep_patterns(&opt);\n \tobject_array_clear(&list);\n \tfree_repos();\ndiff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh\nindex a98785da795..1dd07141a7d 100755\n--- a/t/t7811-grep-open.sh\n+++ b/t/t7811-grep-open.sh\n@@ -3,6 +3,7 @@\n test_description='git grep --open-files-in-pager\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-pager.sh\n unset PAGER GIT_PAGER\n@@ -114,8 +115,8 @@ test_expect_success 'modified file' '\n \tunrelated\n \tEOF\n \n+\ttest_when_finished \"git reset --hard\" &&\n \techo \"enum grep_pat_token\" >unrelated &&\n-\ttest_when_finished \"git checkout HEAD unrelated\" &&\n \tGIT_PAGER=./less git grep -F -O \"enum grep_pat_token\" >out &&\n \ttest_cmp expect actual &&\n \ttest_must_be_empty out\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439354","messageId":"patch-v2-4.6-0d0e6359cf4-20211022T085306Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com","subject":"[PATCH v2 4/6] clone: fix a memory leak of the \"git_dir\" variable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T08:55:42Z","receivedAt":"2021-10-22T08:56:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"At this point in cmd_clone the \"git_dir\" is always either an\nxstrdup()'d string, or something we got from mkpathdup(). Let's free()\nit before we clobber it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/clone.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 559acf9e036..fb377b27657 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1040,8 +1040,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tinit_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,\n \t\tINIT_DB_QUIET);\n \n-\tif (real_git_dir)\n+\tif (real_git_dir) {\n+\t\tfree((char *)git_dir);\n \t\tgit_dir = real_git_dir;\n+\t}\n \n \t/*\n \t * additional config can be injected with -c, make sure it's included\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439355","messageId":"patch-v2-5.6-a529c04a29a-20211022T085306Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com","subject":"[PATCH v2 5/6] submodule--helper: fix small memory leaks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T08:55:43Z","receivedAt":"2021-10-22T08:56:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a missing strbuf_release() and a clear_pathspec() to the\nsubmodule--helper.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/submodule--helper.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6298cbdd4e5..a157656a48a 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3220,6 +3220,7 @@ static void die_on_index_match(const char *path, int force)\n \t\t}\n \t\tfree(ps_matched);\n \t}\n+\tclear_pathspec(&ps);\n }\n \n static void die_on_repo_without_commits(const char *path)\n@@ -3231,6 +3232,7 @@ static void die_on_repo_without_commits(const char *path)\n \t\tif (resolve_gitlink_ref(path, \"HEAD\", &oid) < 0)\n \t\t\tdie(_(\"'%s' does not have a commit checked out\"), path);\n \t}\n+\tstrbuf_release(&sb);\n }\n \n static int module_add(int argc, const char **argv, const char *prefix)\n-- \n2.33.1.1494.g88b39a443e1\n\n"},{"id":"439356","messageId":"patch-v2-6.6-6ea5e611ae0-20211022T085306Z-avarab@gmail.com","threadId":"56748","inReplyTo":"cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com","subject":"[PATCH v2 6/6] reflog: free() ref given to us by dwim_log()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-22T08:55:44Z","receivedAt":"2021-10-22T08:56:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When dwim_log() returns the \"ref\" is always ether NULL or an\nxstrdup()'d string.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/reflog.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex bd4c669918d..175c83e7cc2 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -653,6 +653,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tshould_expire_reflog_ent,\n \t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t&cb);\n+\t\tfree(ref);\n \t}\n \treturn status;\n }\n-- \n2.33.1.1494.g88b39a443e1\n\n"}]}