Re: [PATCH 0/4] fix a leak with excludes_file

[Junio C Hamano <gitster@pobox.com>]

Rubén Justo <rjusto@gmail.com> writes:

> We call twice to git_config(): first to get the main git-branch(1) options,
> and second to get the ones related to the sequencer.

The diagnosis of the control flow that leads to overwriting the
value assigned earlier to "excludes_file" is quite correct, but
I find that your fix is somehow not addressing the real issue.

We used to assume that excludes_file *always* points at a piece of
memory that somebody else owns.  But seeing the two places that
assign to the variable, both actually give the variable a piece of
memory allocated for the caller.

 - The one in config.c that reads core.excludesfile via
   git_config_pathname(), which calls interpolate_path(), whose
   return value is always allocated.

 - The one in dir.c that sets up standard excludes uses
   xdg_config_home() that returns a value obtained from mkpathdup().

The core of the fix ought to be just like this, but to lose the cast
that discards the constness away, the fallouts will be large (if you
remove "const" from the first parameter to git_config_pathname(),
the compiler will tell you where the fallouts are).

We may want to eventually fix the type of the first parameter of the
git_config_pathmame() function, but I somehow doubt that your
approach gets us any closer to that future.  Adding a parallel API
next to the existing git_config_pathname() and interpolate_path()
and convert only these callers that touch excludes_file would not
help other callers that hold the pointer git_config_pathname()
returned.

 config.c         | 4 +++-
 t/t7300-clean.sh | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git c/config.c w/config.c
index eebce8c7e0..ae3652b08f 100644
--- c/config.c
+++ w/config.c
@@ -1584,8 +1584,10 @@ static int git_default_core_config(const char *var, const char *value,
 	if (!strcmp(var, "core.askpass"))
 		return git_config_string(&askpass_program, var, value);
 
-	if (!strcmp(var, "core.excludesfile"))
+	if (!strcmp(var, "core.excludesfile")) {
+		free((char *)excludes_file);
 		return git_config_pathname(&excludes_file, var, value);
+	}
 
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
diff --git c/t/t7300-clean.sh w/t/t7300-clean.sh
index 1f7201eb60..0aae0dee67 100755
--- c/t/t7300-clean.sh
+++ w/t/t7300-clean.sh
@@ -5,6 +5,7 @@
 
 test_description='git clean basic tests'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 git config clean.requireForce no