config: clarify memory ownership in git_config_pathname()

pks-t · gitster · commit 6073b3b5c377 · 2024-05-27T11:19:59.000-07:00
The out parameter of `git_config_pathname()` is a `const char **` even
though we transfer ownership of memory to the caller. This is quite
misleading and has led to many memory leaks all over the place. Adapt
the parameter to instead be `char **`.

Signed-off-by: Patrick Steinhardt &lt;ps@pks.im&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/builtin/blame.c b/builtin/blame.c
@@ -718,7 +718,7 @@ static int git_blame_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "blame.ignorerevsfile")) {
-		const char *str;
+		char *str;
 		int ret;
 
 		ret = git_config_pathname(&str, var, value);
diff --git a/builtin/commit.c b/builtin/commit.c
@@ -107,7 +107,7 @@ static enum {
 } commit_style;
 
 static const char *logfile, *force_author;
-static const char *template_file;
+static char *template_file;
 /*
  * The _message variables are commit names from which to take
  * the commit message and/or authorship.
diff --git a/builtin/config.c b/builtin/config.c
@@ -277,7 +277,7 @@ static int format_config(struct strbuf *buf, const char *key_,
 			else
 				strbuf_addstr(buf, v ? "true" : "false");
 		} else if (type == TYPE_PATH) {
-			const char *v;
+			char *v;
 			if (git_config_pathname(&v, key_, value_) < 0)
 				return -1;
 			strbuf_addstr(buf, v);
diff --git a/builtin/log.c b/builtin/log.c
@@ -957,7 +957,7 @@ static int do_signoff;
 static enum auto_base_setting auto_base;
 static char *from;
 static const char *signature = git_version_string;
-static const char *signature_file;
+static char *signature_file;
 static enum cover_setting config_cover_letter;
 static const char *config_output_directory;
 static enum cover_from_description cover_from_description_mode = COVER_FROM_MESSAGE;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
@@ -168,13 +168,13 @@ static int receive_pack_config(const char *var, const char *value,
 	}
 
 	if (strcmp(var, "receive.fsck.skiplist") == 0) {
-		const char *path;
+		char *path;
 
 		if (git_config_pathname(&path, var, value))
 			return 1;
 		strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
 			fsck_msg_types.len ? ',' : '=', path);
-		free((char *)path);
+		free(path);
 		return 0;
 	}
 
diff --git a/config.c b/config.c
@@ -1346,7 +1346,7 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
-int git_config_pathname(const char **dest, const char *var, const char *value)
+int git_config_pathname(char **dest, const char *var, const char *value)
 {
 	if (!value)
 		return config_error_nonbool(var);
@@ -1597,7 +1597,7 @@ static int git_default_core_config(const char *var, const char *value,
 		return git_config_string(&askpass_program, var, value);
 
 	if (!strcmp(var, "core.excludesfile")) {
-		free((char *)excludes_file);
+		free(excludes_file);
 		return git_config_pathname(&excludes_file, var, value);
 	}
 
@@ -2494,7 +2494,7 @@ int git_configset_get_maybe_bool(struct config_set *set, const char *key, int *d
 		return 1;
 }
 
-int git_configset_get_pathname(struct config_set *set, const char *key, const char **dest)
+int git_configset_get_pathname(struct config_set *set, const char *key, char **dest)
 {
 	const char *value;
 	if (!git_configset_get_value(set, key, &value, NULL))
@@ -2639,7 +2639,7 @@ int repo_config_get_maybe_bool(struct repository *repo,
 }
 
 int repo_config_get_pathname(struct repository *repo,
-			     const char *key, const char **dest)
+			     const char *key, char **dest)
 {
 	int ret;
 	git_config_check_init(repo);
@@ -2738,7 +2738,7 @@ int git_config_get_maybe_bool(const char *key, int *dest)
 	return repo_config_get_maybe_bool(the_repository, key, dest);
 }
 
-int git_config_get_pathname(const char *key, const char **dest)
+int git_config_get_pathname(const char *key, char **dest)
 {
 	return repo_config_get_pathname(the_repository, key, dest);
 }
diff --git a/config.h b/config.h
@@ -286,7 +286,7 @@ int git_config_string(const char **, const char *, const char *);
  * Similar to `git_config_string`, but expands `~` or `~user` into the
  * user's home directory when found at the beginning of the path.
  */
-int git_config_pathname(const char **, const char *, const char *);
+int git_config_pathname(char **, const char *, const char *);
 
 int git_config_expiry_date(timestamp_t *, const char *, const char *);
 int git_config_color(char *, const char *, const char *);
@@ -541,7 +541,7 @@ int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned lon
 int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);
 int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);
 int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);
-int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest);
+int git_configset_get_pathname(struct config_set *cs, const char *key, char **dest);
 
 /* Functions for reading a repository's config */
 struct repository;
@@ -577,7 +577,7 @@ int repo_config_get_bool_or_int(struct repository *repo,
 int repo_config_get_maybe_bool(struct repository *repo,
 			       const char *key, int *dest);
 int repo_config_get_pathname(struct repository *repo,
-			     const char *key, const char **dest);
+			     const char *key, char **dest);
 
 /*
  * Functions for reading protected config. By definition, protected
@@ -687,7 +687,7 @@ int git_config_get_maybe_bool(const char *key, int *dest);
  * Similar to `git_config_get_string`, but expands `~` or `~user` into
  * the user's home directory when found at the beginning of the path.
  */
-int git_config_get_pathname(const char *key, const char **dest);
+int git_config_get_pathname(const char *key, char **dest);
 
 int git_config_get_index_threads(int *dest);
 int git_config_get_split_index(void);
diff --git a/diff.c b/diff.c
@@ -58,7 +58,7 @@ static int diff_context_default = 3;
 static int diff_interhunk_context_default;
 static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
-static const char *diff_order_file_cfg;
+static char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
diff --git a/environment.c b/environment.c
@@ -46,8 +46,8 @@ const char *git_commit_encoding;
 const char *git_log_output_encoding;
 char *apply_default_whitespace;
 char *apply_default_ignorewhitespace;
-const char *git_attributes_file;
-const char *git_hooks_path;
+char *git_attributes_file;
+char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
 int pack_compression_level = Z_DEFAULT_COMPRESSION;
 int fsync_object_files = -1;
@@ -60,7 +60,7 @@ size_t delta_base_cache_limit = 96 * 1024 * 1024;
 unsigned long big_file_threshold = 512 * 1024 * 1024;
 const char *editor_program;
 const char *askpass_program;
-const char *excludes_file;
+char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
diff --git a/environment.h b/environment.h
@@ -131,8 +131,8 @@ extern int warn_ambiguous_refs;
 extern int warn_on_object_refname_ambiguity;
 extern char *apply_default_whitespace;
 extern char *apply_default_ignorewhitespace;
-extern const char *git_attributes_file;
-extern const char *git_hooks_path;
+extern char *git_attributes_file;
+extern char *git_hooks_path;
 extern int zlib_compression_level;
 extern int pack_compression_level;
 extern size_t packed_git_window_size;
@@ -229,7 +229,7 @@ extern const char *git_log_output_encoding;
 
 extern const char *editor_program;
 extern const char *askpass_program;
-extern const char *excludes_file;
+extern char *excludes_file;
 
 /*
  * Should we print an ellipsis after an abbreviated SHA-1 value
diff --git a/fetch-pack.c b/fetch-pack.c
@@ -1865,13 +1865,13 @@ static int fetch_pack_config_cb(const char *var, const char *value,
 	const char *msg_id;
 
 	if (strcmp(var, "fetch.fsck.skiplist") == 0) {
-		const char *path;
+		char *path ;
 
 		if (git_config_pathname(&path, var, value))
 			return 1;
 		strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
 			fsck_msg_types.len ? ',' : '=', path);
-		free((char *)path);
+		free(path);
 		return 0;
 	}
 
diff --git a/fsck.c b/fsck.c
@@ -1330,13 +1330,13 @@ int git_fsck_config(const char *var, const char *value,
 	const char *msg_id;
 
 	if (strcmp(var, "fsck.skiplist") == 0) {
-		const char *path;
+		char *path;
 		struct strbuf sb = STRBUF_INIT;
 
 		if (git_config_pathname(&path, var, value))
 			return 1;
 		strbuf_addf(&sb, "skiplist=%s", path);
-		free((char *)path);
+		free(path);
 		fsck_set_msg_types(options, sb.buf);
 		strbuf_release(&sb);
 		return 0;
diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c
@@ -103,6 +103,7 @@ static struct fsmonitor_settings *alloc_settings(void)
 static void lookup_fsmonitor_settings(struct repository *r)
 {
 	const char *const_str;
+	char *to_free = NULL;
 	int bool_value;
 
 	if (r->settings.fsmonitor)
@@ -129,8 +130,9 @@ static void lookup_fsmonitor_settings(struct repository *r)
 		break;
 
 	case -1: /* config value set to an arbitrary string */
-		if (repo_config_get_pathname(r, "core.fsmonitor", &const_str))
+		if (repo_config_get_pathname(r, "core.fsmonitor", &to_free))
 			return; /* should not happen */
+		const_str = to_free;
 		break;
 
 	default: /* should not happen */
@@ -141,6 +143,7 @@ static void lookup_fsmonitor_settings(struct repository *r)
 		fsm_settings__set_hook(r, const_str);
 	else
 		fsm_settings__set_disabled(r);
+	free(to_free);
 }
 
 enum fsmonitor_mode fsm_settings__get_mode(struct repository *r)
diff --git a/gpg-interface.c b/gpg-interface.c
@@ -27,7 +27,9 @@ static void gpg_interface_lazy_init(void)
 }
 
 static char *configured_signing_key;
-static const char *ssh_default_key_command, *ssh_allowed_signers, *ssh_revocation_file;
+static const char *ssh_default_key_command;
+static char *ssh_allowed_signers;
+static char *ssh_revocation_file;
 static enum signature_trust_level configured_min_trust_level = TRUST_UNDEFINED;
 
 struct gpg_format {
diff --git a/http.c b/http.c
@@ -64,7 +64,7 @@ static char *ssl_key_type;
 static char *ssl_capath;
 static char *curl_no_proxy;
 #ifdef GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY
-static const char *ssl_pinnedkey;
+static char *ssl_pinnedkey;
 #endif
 static char *ssl_cainfo;
 static long curl_low_speed_limit = -1;
@@ -108,7 +108,7 @@ static struct {
 
 static struct credential proxy_auth = CREDENTIAL_INIT;
 static const char *curl_proxyuserpwd;
-static const char *curl_cookie_file;
+static char *curl_cookie_file;
 static int curl_save_cookies;
 struct credential http_auth = CREDENTIAL_INIT;
 static int http_proactive_auth;
@@ -381,17 +381,17 @@ static int http_options(const char *var, const char *value,
 	if (!strcmp("http.sslversion", var))
 		return git_config_string(&ssl_version, var, value);
 	if (!strcmp("http.sslcert", var))
-		return git_config_pathname((const char **)&ssl_cert, var, value);
+		return git_config_pathname(&ssl_cert, var, value);
 	if (!strcmp("http.sslcerttype", var))
 		return git_config_string((const char **)&ssl_cert_type, var, value);
 	if (!strcmp("http.sslkey", var))
-		return git_config_pathname((const char **)&ssl_key, var, value);
+		return git_config_pathname(&ssl_key, var, value);
 	if (!strcmp("http.sslkeytype", var))
 		return git_config_string((const char **)&ssl_key_type, var, value);
 	if (!strcmp("http.sslcapath", var))
-		return git_config_pathname((const char **)&ssl_capath, var, value);
+		return git_config_pathname(&ssl_capath, var, value);
 	if (!strcmp("http.sslcainfo", var))
-		return git_config_pathname((const char **)&ssl_cainfo, var, value);
+		return git_config_pathname(&ssl_cainfo, var, value);
 	if (!strcmp("http.sslcertpasswordprotected", var)) {
 		ssl_cert_password_required = git_config_bool(var, value);
 		return 0;
diff --git a/mailmap.c b/mailmap.c
@@ -6,7 +6,7 @@
 #include "object-store-ll.h"
 #include "setup.h"
 
-const char *git_mailmap_file;
+char *git_mailmap_file;
 const char *git_mailmap_blob;
 
 struct mailmap_info {
diff --git a/mailmap.h b/mailmap.h
@@ -3,7 +3,7 @@
 
 struct string_list;
 
-extern const char *git_mailmap_file;
+extern char *git_mailmap_file;
 extern const char *git_mailmap_blob;
 
 int read_mailmap(struct string_list *map);
diff --git a/setup.c b/setup.c
@@ -1177,13 +1177,13 @@ static int safe_directory_cb(const char *key, const char *value,
 	} else if (!strcmp(value, "*")) {
 		data->is_safe = 1;
 	} else {
-		const char *interpolated = NULL;
+		char *interpolated = NULL;
 
 		if (!git_config_pathname(&interpolated, key, value) &&
 		    !fspathcmp(data->path, interpolated ? interpolated : value))
 			data->is_safe = 1;
 
-		free((char *)interpolated);
+		free(interpolated);
 	}
 
 	return 0;
@@ -1822,7 +1822,7 @@ static int template_dir_cb(const char *key, const char *value,
 		char *path = NULL;
 
 		FREE_AND_NULL(data->path);
-		if (!git_config_pathname((const char **)&path, key, value))
+		if (!git_config_pathname(&path, key, value))
 			data->path = path ? path : xstrdup(value);
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -718,7 +718,7 @@ static int git_blame_config(const char var, const char value,`
`718`	`718`	`return 0;`
`719`	`719`	`}`
`720`	`720`	`if (!strcmp(var, "blame.ignorerevsfile")) {`
`721`		`- const char *str;`
	`721`	`+ char *str;`
`722`	`722`	`int ret;`
`723`	`723`
`724`	`724`	`ret = git_config_pathname(&str, var, value);`
Original file line number	Diff line number	Diff line change
`@@ -168,13 +168,13 @@ static int receive_pack_config(const char var, const char value,`
`168`	`168`	`}`
`169`	`169`
`170`	`170`	`if (strcmp(var, "receive.fsck.skiplist") == 0) {`
`171`		`- const char *path;`
	`171`	`+ char *path;`
`172`	`172`
`173`	`173`	`if (git_config_pathname(&path, var, value))`
`174`	`174`	`return 1;`
`175`	`175`	`strbuf_addf(&fsck_msg_types, "%cskiplist=%s",`
`176`	`176`	`fsck_msg_types.len ? ',' : '=', path);`
`177`		`- free((char *)path);`
	`177`	`+ free(path);`
`178`	`178`	`return 0;`
`179`	`179`	`}`
`180`	`180`
Original file line number	Diff line number	Diff line change
`@@ -1346,7 +1346,7 @@ int git_config_string(const char *dest, const char var, const char *value)`
`1346`	`1346`	`return 0;`
`1347`	`1347`	`}`
`1348`	`1348`
`1349`		`-int git_config_pathname(const char *dest, const char var, const char *value)`
	`1349`	`+int git_config_pathname(char *dest, const char var, const char *value)`
`1350`	`1350`	`{`
`1351`	`1351`	`if (!value)`
`1352`	`1352`	`return config_error_nonbool(var);`
`@@ -1597,7 +1597,7 @@ static int git_default_core_config(const char var, const char value,`
`1597`	`1597`	`return git_config_string(&askpass_program, var, value);`
`1598`	`1598`
`1599`	`1599`	`if (!strcmp(var, "core.excludesfile")) {`
`1600`		`- free((char *)excludes_file);`
	`1600`	`+ free(excludes_file);`
`1601`	`1601`	`return git_config_pathname(&excludes_file, var, value);`
`1602`	`1602`	`}`
`1603`	`1603`
`@@ -2494,7 +2494,7 @@ int git_configset_get_maybe_bool(struct config_set set, const char key, int *d`
`2494`	`2494`	`return 1;`
`2495`	`2495`	`}`
`2496`	`2496`
`2497`		`-int git_configset_get_pathname(struct config_set set, const char key, const char **dest)`
	`2497`	`+int git_configset_get_pathname(struct config_set set, const char key, char **dest)`
`2498`	`2498`	`{`
`2499`	`2499`	`const char *value;`
`2500`	`2500`	`if (!git_configset_get_value(set, key, &value, NULL))`
`@@ -2639,7 +2639,7 @@ int repo_config_get_maybe_bool(struct repository *repo,`
`2639`	`2639`	`}`
`2640`	`2640`
`2641`	`2641`	`int repo_config_get_pathname(struct repository *repo,`
`2642`		`- const char key, const char *dest)`
	`2642`	`+ const char key, char *dest)`
`2643`	`2643`	`{`
`2644`	`2644`	`int ret;`
`2645`	`2645`	`git_config_check_init(repo);`
`@@ -2738,7 +2738,7 @@ int git_config_get_maybe_bool(const char key, int dest)`
`2738`	`2738`	`return repo_config_get_maybe_bool(the_repository, key, dest);`
`2739`	`2739`	`}`
`2740`	`2740`
`2741`		`-int git_config_get_pathname(const char key, const char *dest)`
	`2741`	`+int git_config_get_pathname(const char key, char *dest)`
`2742`	`2742`	`{`
`2743`	`2743`	`return repo_config_get_pathname(the_repository, key, dest);`
`2744`	`2744`	`}`