[PATCH 0/6] leaks: miscellaneous small leak fixes

[PATCH 0/6] leaks: miscellaneous small leak fixes

[Ævar Arnfjörð Bjarmason]

A few small miscellaneous memory leak fixes. I'm currently waiting on
the other in-flight leak fixes I've got to mark more tests as passing
under SANITIZE=leak, these fixes don't make any tests pass by
themselves, but in combination with later fixes I've got planned will
make some tests scripts pass completely.

Ævar Arnfjörð Bjarmason (6):
  grep: prefer "struct grep_opt" over its "void *"
  grep: use object_array_clear() in cmd_grep()
  clone: fix a memory leak of the "git_dir" variable
  submodule--helper: fix small memory leaks
  reflog: free() ref given to us by dwim_log()
  repack: stop leaking a "struct child_process"

 builtin/clone.c             | 4 +++-
 builtin/grep.c              | 5 +++--
 builtin/reflog.c            | 1 +
 builtin/repack.c            | 4 +++-
 builtin/submodule--helper.c | 2 ++
 5 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.33.1.1494.g88b39a443e1

[PATCH 1/6] grep: prefer "struct grep_opt" over its "void *"

[Ævar Arnfjörð Bjarmason]

Stylistically fix up code added in bfac23d9534 (grep: Fix two memory
leaks, 2010-01-30). We usually don't use the "arg" at all once we've
unpacked it into the struct we want, let's not do that here when we're
freeing it. Perhaps it was thought that a cast to "void *" would
otherwise be needed?

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/grep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 8af5249a7bb..fd184c182a3 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -199,8 +199,8 @@ static void *run(void *arg)
 		grep_source_clear_data(&w->source);
 		work_done(w);
 	}
-	free_grep_patterns(arg);
-	free(arg);
+	free_grep_patterns(opt);
+	free(opt);
 
 	return (void*) (intptr_t) hit;
 }
-- 
2.33.1.1494.g88b39a443e1

Re: [PATCH 1/6] grep: prefer "struct grep_opt" over its "void *"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Subject: Re: [PATCH 1/6] grep: prefer "struct grep_opt" over its "void *"

I sense an incomplete sentence.  "counterpart" or "equivalent" is
missing at the tail, perhaps?

> Stylistically fix up code added in bfac23d9534 (grep: Fix two memory
> leaks, 2010-01-30). We usually don't use the "arg" at all once we've
> unpacked it into the struct we want, let's not do that here when we're

As we do not unpack and instead take it as a whole and cast, "we've
unpacked it into" -> "we've casted it to".




> freeing it. Perhaps it was thought that a cast to "void *" would
> otherwise be needed?
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/grep.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 8af5249a7bb..fd184c182a3 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -199,8 +199,8 @@ static void *run(void *arg)
>  		grep_source_clear_data(&w->source);
>  		work_done(w);
>  	}
> -	free_grep_patterns(arg);
> -	free(arg);
> +	free_grep_patterns(opt);
> +	free(opt);
>  
>  	return (void*) (intptr_t) hit;
>  }

[PATCH 2/6] grep: use object_array_clear() in cmd_grep()

[Ævar Arnfjörð Bjarmason]

Free the "struct object_array" before exiting. This makes grep tests
(e.g.  "t7815-grep-binary.sh") a bit happer under SANITIZE=leak.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/grep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index fd184c182a3..555b2ab6008 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		run_pager(&opt, prefix);
 	clear_pathspec(&pathspec);
 	free_grep_patterns(&opt);
+	object_array_clear(&list);
 	free_repos();
 	return !hit;
 }
-- 
2.33.1.1494.g88b39a443e1

Re: [PATCH 2/6] grep: use object_array_clear() in cmd_grep()

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Free the "struct object_array" before exiting. This makes grep tests
> (e.g.  "t7815-grep-binary.sh") a bit happer under SANITIZE=leak.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/grep.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index fd184c182a3..555b2ab6008 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		run_pager(&opt, prefix);
>  	clear_pathspec(&pathspec);
>  	free_grep_patterns(&opt);
> +	object_array_clear(&list);
>  	free_repos();
>  	return !hit;
>  }

OK.

Re: [PATCH 2/6] grep: use object_array_clear() in cmd_grep()

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Free the "struct object_array" before exiting. This makes grep tests
> (e.g.  "t7815-grep-binary.sh") a bit happer under SANITIZE=leak.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/grep.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index fd184c182a3..555b2ab6008 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		run_pager(&opt, prefix);
>  	clear_pathspec(&pathspec);
>  	free_grep_patterns(&opt);
> +	object_array_clear(&list);
>  	free_repos();
>  	return !hit;
>  }

Not a new issue introduced by this patch, but after run_pager(), it
seems that opt.output_priv aka path_list is never cleared.  Should
this step do that, too, as the series never revisits this file?

[PATCH 3/6] clone: fix a memory leak of the "git_dir" variable

[Ævar Arnfjörð Bjarmason]

At this point in cmd_clone the "git_dir" is always either an
xstrdup()'d string, or something we got from mkpathdup(). Let's free()
it before we clobber it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/clone.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 559acf9e036..fb377b27657 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1040,8 +1040,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
 		INIT_DB_QUIET);
 
-	if (real_git_dir)
+	if (real_git_dir) {
+		free((char *)git_dir);
 		git_dir = real_git_dir;
+	}
 
 	/*
 	 * additional config can be injected with -c, make sure it's included
-- 
2.33.1.1494.g88b39a443e1

Re: [PATCH 3/6] clone: fix a memory leak of the "git_dir" variable

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> At this point in cmd_clone the "git_dir" is always either an
> xstrdup()'d string, or something we got from mkpathdup(). Let's free()
> it before we clobber it.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/clone.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 559acf9e036..fb377b27657 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1040,8 +1040,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
>  		INIT_DB_QUIET);
>  
> -	if (real_git_dir)
> +	if (real_git_dir) {
> +		free((char *)git_dir);
>  		git_dir = real_git_dir;
> +	}
>  
>  	/*
>  	 * additional config can be injected with -c, make sure it's included

I had to wonder if the old git_dir can still be pointed at by
junk_git_dir.  Much earlier than this point there is this:

	if (real_git_dir) {
		if (real_dest_exists)
			junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
		junk_git_dir = real_git_dir;
	} else {
		if (dest_exists)
			junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
		junk_git_dir = git_dir;
	}
	if (safe_create_leading_directories_const(git_dir) < 0)
		die(_("could not create leading directories of '%s'"), git_dir);

Luckily, junk_git_dir gets git_dir only when !real_git_dir, so it is
safe.  real_git_dir can only be set via the --separate-git-dir
command line option, so we are safe here.

Thanks.

[PATCH 4/6] submodule--helper: fix small memory leaks

[Ævar Arnfjörð Bjarmason]

Add a missing strbuf_release() and a clear_pathspec() to the
submodule--helper.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/submodule--helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6298cbdd4e5..a157656a48a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3220,6 +3220,7 @@ static void die_on_index_match(const char *path, int force)
 		}
 		free(ps_matched);
 	}
+	clear_pathspec(&ps);
 }
 
 static void die_on_repo_without_commits(const char *path)
@@ -3231,6 +3232,7 @@ static void die_on_repo_without_commits(const char *path)
 		if (resolve_gitlink_ref(path, "HEAD", &oid) < 0)
 			die(_("'%s' does not have a commit checked out"), path);
 	}
+	strbuf_release(&sb);
 }
 
 static int module_add(int argc, const char **argv, const char *prefix)
-- 
2.33.1.1494.g88b39a443e1

[PATCH 5/6] reflog: free() ref given to us by dwim_log()

[Ævar Arnfjörð Bjarmason]

When dwim_log() returns the "ref" is always ether NULL or an
xstrdup()'d string.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/reflog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index bd4c669918d..175c83e7cc2 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -653,6 +653,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 					should_expire_reflog_ent,
 					reflog_expiry_cleanup,
 					&cb);
+		free(ref);
 	}
 	return status;
 }
-- 
2.33.1.1494.g88b39a443e1

Re: [PATCH 5/6] reflog: free() ref given to us by dwim_log()

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> When dwim_log() returns the "ref" is always ether NULL or an
> xstrdup()'d string.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/reflog.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index bd4c669918d..175c83e7cc2 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -653,6 +653,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
>  					should_expire_reflog_ent,
>  					reflog_expiry_cleanup,
>  					&cb);
> +		free(ref);

OK.  When dwim_log() returns false, it would not have touched &ref
at all, so the "continue" (not shown in the context above) is not
leaking, and this loop looks good.

Thanks.

>  	}
>  	return status;
>  }

[PATCH 6/6] repack: stop leaking a "struct child_process"

[Ævar Arnfjörð Bjarmason]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/repack.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 0b2d1e5d82b..50730517c7b 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -258,9 +258,11 @@ static void repack_promisor_objects(const struct pack_objects_args *args,
 	for_each_packed_object(write_oid, &cmd,
 			       FOR_EACH_OBJECT_PROMISOR_ONLY);
 
-	if (cmd.in == -1)
+	if (cmd.in == -1) {
+		child_process_clear(&cmd);
 		/* No packed objects; cmd was never started */
 		return;
+	}
 
 	close(cmd.in);
 
-- 
2.33.1.1494.g88b39a443e1

Re: [PATCH 6/6] repack: stop leaking a "struct child_process"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/repack.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/repack.c b/builtin/repack.c
> index 0b2d1e5d82b..50730517c7b 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -258,9 +258,11 @@ static void repack_promisor_objects(const struct pack_objects_args *args,
>  	for_each_packed_object(write_oid, &cmd,
>  			       FOR_EACH_OBJECT_PROMISOR_ONLY);
>  
> -	if (cmd.in == -1)
> +	if (cmd.in == -1) {
> +		child_process_clear(&cmd);
>  		/* No packed objects; cmd was never started */
>  		return;
> +	}
>  
>  	close(cmd.in);

Not wrong per-se, but let's take the one that is part of Taylor's
"plug pack bitmap leaks" series that plugs the same leak.

[PATCH v2 0/6] leaks: miscellaneous small leak fixes

[Ævar Arnfjörð Bjarmason]

A re-roll of these miscellaneous small leak fixes to a address
comments on v1. I ejected the patch that conflicted with Taylor's
version (sorry, didn't notice it), and fixed an additional leak in
grep.c pointed out by Junio. Doing that allowed us to mark a test as
passing under SANITIZE=leak, with only the minor change of replacing a
"git checkout" with "git reset --hard" (as "checkout" happens to leak
currently, but I've also got a pending patch for that).

Ævar Arnfjörð Bjarmason (6):
  grep: prefer "struct grep_opt" over its "void *" equivalent
  grep: use object_array_clear() in cmd_grep()
  grep: fix a "path_list" memory leak
  clone: fix a memory leak of the "git_dir" variable
  submodule--helper: fix small memory leaks
  reflog: free() ref given to us by dwim_log()

 builtin/clone.c             |  4 +++-
 builtin/grep.c              | 14 ++++++++------
 builtin/reflog.c            |  1 +
 builtin/submodule--helper.c |  2 ++
 t/t7811-grep-open.sh        |  3 ++-
 5 files changed, 16 insertions(+), 8 deletions(-)

Range-diff against v1:
1:  2bdd21e4e59 ! 1:  66c838fd800 grep: prefer "struct grep_opt" over its "void *"
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    grep: prefer "struct grep_opt" over its "void *"
    +    grep: prefer "struct grep_opt" over its "void *" equivalent
     
         Stylistically fix up code added in bfac23d9534 (grep: Fix two memory
         leaks, 2010-01-30). We usually don't use the "arg" at all once we've
    -    unpacked it into the struct we want, let's not do that here when we're
    +    casted it to the struct we want, let's not do that here when we're
         freeing it. Perhaps it was thought that a cast to "void *" would
         otherwise be needed?
     
2:  727fdb27a2a = 2:  033ca3f7b4f grep: use object_array_clear() in cmd_grep()
-:  ----------- > 3:  8e941e40711 grep: fix a "path_list" memory leak
3:  86d928ae2f9 = 4:  0d0e6359cf4 clone: fix a memory leak of the "git_dir" variable
4:  9c3c0529ad0 = 5:  a529c04a29a submodule--helper: fix small memory leaks
5:  85b7b7aef37 = 6:  6ea5e611ae0 reflog: free() ref given to us by dwim_log()
6:  526d5649156 < -:  ----------- repack: stop leaking a "struct child_process"
-- 
2.33.1.1494.g88b39a443e1

[PATCH v2 1/6] grep: prefer "struct grep_opt" over its "void *" equivalent

[Ævar Arnfjörð Bjarmason]

Stylistically fix up code added in bfac23d9534 (grep: Fix two memory
leaks, 2010-01-30). We usually don't use the "arg" at all once we've
casted it to the struct we want, let's not do that here when we're
freeing it. Perhaps it was thought that a cast to "void *" would
otherwise be needed?

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/grep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 8af5249a7bb..fd184c182a3 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -199,8 +199,8 @@ static void *run(void *arg)
 		grep_source_clear_data(&w->source);
 		work_done(w);
 	}
-	free_grep_patterns(arg);
-	free(arg);
+	free_grep_patterns(opt);
+	free(opt);
 
 	return (void*) (intptr_t) hit;
 }
-- 
2.33.1.1494.g88b39a443e1

[PATCH v2 2/6] grep: use object_array_clear() in cmd_grep()

[Ævar Arnfjörð Bjarmason]

Free the "struct object_array" before exiting. This makes grep tests
(e.g.  "t7815-grep-binary.sh") a bit happer under SANITIZE=leak.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/grep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index fd184c182a3..555b2ab6008 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		run_pager(&opt, prefix);
 	clear_pathspec(&pathspec);
 	free_grep_patterns(&opt);
+	object_array_clear(&list);
 	free_repos();
 	return !hit;
 }
-- 
2.33.1.1494.g88b39a443e1

[PATCH v2 3/6] grep: fix a "path_list" memory leak

[Ævar Arnfjörð Bjarmason]

Free the "path_list" used in builtin/grep.c, it was declared as
STRING_LIST_INIT_NODUP, let's change it to a STRING_LIST_INIT_DUP
since an early user in cmd_grep() appends a string passed via
parse-options.c to it, which needs to be duplicated.

Let's then convert the remaining callers to use
string_list_append_nodup() instead, allowing us to free the list.

This makes all the tests in t7811-grep-open.sh pass, 6/10 would fail
before this change. The only remaining failure would have been due to
a stray "git checkout" (which still leaks memory). In this case we can
use a "git reset --hard" instead, so let's do that, and move the
test_when_finished() above the code that would modify the relevant
file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/grep.c       | 9 +++++----
 t/t7811-grep-open.sh | 3 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 555b2ab6008..9e34a820ad4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -401,7 +401,7 @@ static void append_path(struct grep_opt *opt, const void *data, size_t len)
 
 	if (len == 1 && *(const char *)data == '\0')
 		return;
-	string_list_append(path_list, xstrndup(data, len));
+	string_list_append_nodup(path_list, xstrndup(data, len));
 }
 
 static void run_pager(struct grep_opt *opt, const char *prefix)
@@ -839,7 +839,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct grep_opt opt;
 	struct object_array list = OBJECT_ARRAY_INIT;
 	struct pathspec pathspec;
-	struct string_list path_list = STRING_LIST_INIT_NODUP;
+	struct string_list path_list = STRING_LIST_INIT_DUP;
 	int i;
 	int dummy;
 	int use_index = 1;
@@ -1159,8 +1159,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			strbuf_addf(&buf, "+/%s%s",
 					strcmp("less", pager) ? "" : "*",
 					opt.pattern_list->pattern);
-			string_list_append(&path_list,
-					   strbuf_detach(&buf, NULL));
+			string_list_append_nodup(&path_list,
+						 strbuf_detach(&buf, NULL));
 		}
 	}
 
@@ -1195,6 +1195,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (hit && show_in_pager)
 		run_pager(&opt, prefix);
 	clear_pathspec(&pathspec);
+	string_list_clear(&path_list, 0);
 	free_grep_patterns(&opt);
 	object_array_clear(&list);
 	free_repos();
diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh
index a98785da795..1dd07141a7d 100755
--- a/t/t7811-grep-open.sh
+++ b/t/t7811-grep-open.sh
@@ -3,6 +3,7 @@
 test_description='git grep --open-files-in-pager
 '
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-pager.sh
 unset PAGER GIT_PAGER
@@ -114,8 +115,8 @@ test_expect_success 'modified file' '
 	unrelated
 	EOF
 
+	test_when_finished "git reset --hard" &&
 	echo "enum grep_pat_token" >unrelated &&
-	test_when_finished "git checkout HEAD unrelated" &&
 	GIT_PAGER=./less git grep -F -O "enum grep_pat_token" >out &&
 	test_cmp expect actual &&
 	test_must_be_empty out
-- 
2.33.1.1494.g88b39a443e1

[PATCH v2 4/6] clone: fix a memory leak of the "git_dir" variable

[Ævar Arnfjörð Bjarmason]

At this point in cmd_clone the "git_dir" is always either an
xstrdup()'d string, or something we got from mkpathdup(). Let's free()
it before we clobber it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/clone.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 559acf9e036..fb377b27657 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1040,8 +1040,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
 		INIT_DB_QUIET);
 
-	if (real_git_dir)
+	if (real_git_dir) {
+		free((char *)git_dir);
 		git_dir = real_git_dir;
+	}
 
 	/*
 	 * additional config can be injected with -c, make sure it's included
-- 
2.33.1.1494.g88b39a443e1

[PATCH v2 5/6] submodule--helper: fix small memory leaks

[Ævar Arnfjörð Bjarmason]

Add a missing strbuf_release() and a clear_pathspec() to the
submodule--helper.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/submodule--helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6298cbdd4e5..a157656a48a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3220,6 +3220,7 @@ static void die_on_index_match(const char *path, int force)
 		}
 		free(ps_matched);
 	}
+	clear_pathspec(&ps);
 }
 
 static void die_on_repo_without_commits(const char *path)
@@ -3231,6 +3232,7 @@ static void die_on_repo_without_commits(const char *path)
 		if (resolve_gitlink_ref(path, "HEAD", &oid) < 0)
 			die(_("'%s' does not have a commit checked out"), path);
 	}
+	strbuf_release(&sb);
 }
 
 static int module_add(int argc, const char **argv, const char *prefix)
-- 
2.33.1.1494.g88b39a443e1

[PATCH v2 6/6] reflog: free() ref given to us by dwim_log()

[Ævar Arnfjörð Bjarmason]

When dwim_log() returns the "ref" is always ether NULL or an
xstrdup()'d string.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/reflog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index bd4c669918d..175c83e7cc2 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -653,6 +653,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 					should_expire_reflog_ent,
 					reflog_expiry_cleanup,
 					&cb);
+		free(ref);
 	}
 	return status;
 }
-- 
2.33.1.1494.g88b39a443e1