system_path(): always return free'able memory to the callerjc/exec-cmd-system-path-leak-fix

The function sometimes returns a newly allocated string and
sometimes returns a borrowed string, the latter of which the callers
must not free().  The existing callers all assume that the return
value belongs to the callee and most of them copy it with strdup()
when they want to keep it around.  They end up leaking the returned
copy when the callee returned a new string because they cannot tell
if they should free it.

Change the contract between the callers and system_path() to make
the returned string owned by the callers; they are responsible for
freeing it when done, but they do not have to make their own copy to
store it away.

Adjust the callers to make sure they do not leak the returned string
once they are done, but do not bother freeing it just before dying,
exiting or exec'ing other program to avoid unnecessary churn.

Reported-by: Alexander Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 10 insertions, 5 deletions

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 56f85e239a..86c8a30a31 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -119,15 +119,18 @@ static void copy_templates(const char *template_dir)
 	DIR *dir;
 	const char *git_dir = get_git_dir();
 	int len = strlen(git_dir);
+	char *to_free = NULL;
 
 	if (!template_dir)
 		template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
 	if (!template_dir)
 		template_dir = init_db_template_dir;
 	if (!template_dir)
-		template_dir = system_path(DEFAULT_GIT_TEMPLATE_DIR);
-	if (!template_dir[0])
+		template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
+	if (!template_dir[0]) {
+		free(to_free);
 		return;
+	}
 	template_len = strlen(template_dir);
 	if (PATH_MAX <= (template_len+strlen("/config")))
 		die(_("insanely long template path %s"), template_dir);
@@ -139,7 +142,7 @@ static void copy_templates(const char *template_dir)
 	dir = opendir(template_path);
 	if (!dir) {
 		warning(_("templates not found %s"), template_dir);
-		return;
+		goto free_return;
 	}
 
 	/* Make sure that template is from the correct vintage */
@@ -155,8 +158,7 @@ static void copy_templates(const char *template_dir)
 			"a wrong format version %d from '%s'"),
 			repository_format_version,
 			template_dir);
-		closedir(dir);
-		return;
+		goto close_free_return;
 	}
 
 	memcpy(path, git_dir, len);
@@ -166,7 +168,10 @@ static void copy_templates(const char *template_dir)
 	copy_templates_1(path, len,
 			 template_path, template_len,
 			 dir);
+close_free_return:
 	closedir(dir);
+free_return:
+	free(to_free);
 }
 
 static int git_init_db_config(const char *k, const char *v, void *cb)