config: clarify memory ownership in git_config_string()

pks-t · gitster · commit 1b261c20ed28 · 2024-05-27T11:20:00.000-07:00
The out parameter of `git_config_string()` 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/alias.c b/alias.c
@@ -22,7 +22,7 @@ static int config_alias_cb(const char *key, const char *value,
 
 	if (data->alias) {
 		if (!strcasecmp(p, data->alias))
-			return git_config_string((const char **)&data->v,
+			return git_config_string(&data->v,
 						 key, value);
 	} else if (data->list) {
 		string_list_append(data->list, p);
diff --git a/attr.c b/attr.c
@@ -25,7 +25,7 @@
 #include "tree-walk.h"
 #include "object-name.h"
 
-const char *git_attr_tree;
+char *git_attr_tree;
 
 const char git_attr__true[] = "(builtin)true";
 const char git_attr__false[] = "\0(builtin)false";
diff --git a/attr.h b/attr.h
@@ -236,6 +236,6 @@ const char *git_attr_global_file(void);
 /* Return whether the system gitattributes file is enabled and should be used. */
 int git_attr_system_is_enabled(void);
 
-extern const char *git_attr_tree;
+extern char *git_attr_tree;
 
 #endif /* ATTR_H */
diff --git a/builtin/commit.c b/builtin/commit.c
@@ -133,7 +133,7 @@ static struct strvec trailer_args = STRVEC_INIT;
  * is specified explicitly.
  */
 static enum commit_msg_cleanup_mode cleanup_mode;
-static const char *cleanup_arg;
+static char *cleanup_arg;
 
 static enum commit_whence whence;
 static int use_editor = 1, include_status = 1;
diff --git a/builtin/log.c b/builtin/log.c
@@ -582,11 +582,11 @@ static int git_log_config(const char *var, const char *value,
 
 	if (!strcmp(var, "format.pretty")) {
 		FREE_AND_NULL(cfg->fmt_pretty);
-		return git_config_string((const char **) &cfg->fmt_pretty, var, value);
+		return git_config_string(&cfg->fmt_pretty, var, value);
 	}
 	if (!strcmp(var, "format.subjectprefix")) {
 		FREE_AND_NULL(cfg->fmt_patch_subject_prefix);
-		return git_config_string((const char **) &cfg->fmt_patch_subject_prefix, var, value);
+		return git_config_string(&cfg->fmt_patch_subject_prefix, var, value);
 	}
 	if (!strcmp(var, "format.filenamemaxlength")) {
 		cfg->fmt_patch_name_max = git_config_int(var, value, ctx->kvi);
@@ -602,7 +602,7 @@ static int git_log_config(const char *var, const char *value,
 	}
 	if (!strcmp(var, "log.date")) {
 		FREE_AND_NULL(cfg->default_date_mode);
-		return git_config_string((const char **) &cfg->default_date_mode, var, value);
+		return git_config_string(&cfg->default_date_mode, var, value);
 	}
 	if (!strcmp(var, "log.decorate")) {
 		cfg->decoration_style = parse_decoration_style(value);
@@ -1076,7 +1076,7 @@ static int git_format_config(const char *var, const char *value,
 	}
 	if (!strcmp(var, "format.suffix")) {
 		FREE_AND_NULL(cfg->fmt_patch_suffix);
-		return git_config_string((const char **) &cfg->fmt_patch_suffix, var, value);
+		return git_config_string(&cfg->fmt_patch_suffix, var, value);
 	}
 	if (!strcmp(var, "format.to")) {
 		if (!value)
@@ -1133,7 +1133,7 @@ static int git_format_config(const char *var, const char *value,
 	}
 	if (!strcmp(var, "format.signature")) {
 		FREE_AND_NULL(cfg->signature);
-		return git_config_string((const char **) &cfg->signature, var, value);
+		return git_config_string(&cfg->signature, var, value);
 	}
 	if (!strcmp(var, "format.signaturefile")) {
 		FREE_AND_NULL(cfg->signature_file);
@@ -1149,7 +1149,7 @@ static int git_format_config(const char *var, const char *value,
 	}
 	if (!strcmp(var, "format.outputdirectory")) {
 		FREE_AND_NULL(cfg->config_output_directory);
-		return git_config_string((const char **) &cfg->config_output_directory, var, value);
+		return git_config_string(&cfg->config_output_directory, var, value);
 	}
 	if (!strcmp(var, "format.useautobase")) {
 		if (value && !strcasecmp(value, "whenAble")) {
diff --git a/builtin/merge.c b/builtin/merge.c
@@ -100,7 +100,7 @@ static struct strategy all_strategy[] = {
 	{ "subtree",    NO_FAST_FORWARD | NO_TRIVIAL },
 };
 
-static const char *pull_twohead, *pull_octopus;
+static char *pull_twohead, *pull_octopus;
 
 enum ff_type {
 	FF_NO,
@@ -110,7 +110,7 @@ enum ff_type {
 
 static enum ff_type fast_forward = FF_ALLOW;
 
-static const char *cleanup_arg;
+static char *cleanup_arg;
 static enum commit_msg_cleanup_mode cleanup_mode;
 
 static int option_parse_message(const struct option *opt,
diff --git a/builtin/rebase.c b/builtin/rebase.c
@@ -83,7 +83,7 @@ static const char *action_names[] = {
 struct rebase_options {
 	enum rebase_type type;
 	enum empty_type empty;
-	const char *default_backend;
+	char *default_backend;
 	const char *state_dir;
 	struct commit *upstream;
 	const char *upstream_name;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
@@ -88,7 +88,7 @@ static struct strbuf push_cert = STRBUF_INIT;
 static struct object_id push_cert_oid;
 static struct signature_check sigcheck;
 static const char *push_cert_nonce;
-static const char *cert_nonce_seed;
+static char *cert_nonce_seed;
 static struct strvec hidden_refs = STRVEC_INIT;
 
 static const char *NONCE_UNSOLICITED = "UNSOLICITED";
diff --git a/builtin/repack.c b/builtin/repack.c
@@ -48,10 +48,10 @@ static const char incremental_bitmap_conflict_error[] = N_(
 );
 
 struct pack_objects_args {
-	const char *window;
-	const char *window_memory;
-	const char *depth;
-	const char *threads;
+	char *window;
+	char *window_memory;
+	char *depth;
+	char *threads;
 	unsigned long max_pack_size;
 	int no_reuse_delta;
 	int no_reuse_object;
diff --git a/config.c b/config.c
@@ -1338,7 +1338,7 @@ int git_config_bool(const char *name, const char *value)
 	return v;
 }
 
-int git_config_string(const char **dest, const char *var, const char *value)
+int git_config_string(char **dest, const char *var, const char *value)
 {
 	if (!value)
 		return config_error_nonbool(var);
@@ -1566,7 +1566,7 @@ static int git_default_core_config(const char *var, const char *value,
 
 	if (!strcmp(var, "core.checkroundtripencoding")) {
 		FREE_AND_NULL(check_roundtrip_encoding);
-		return git_config_string((const char **) &check_roundtrip_encoding, var, value);
+		return git_config_string(&check_roundtrip_encoding, var, value);
 	}
 
 	if (!strcmp(var, "core.notesref")) {
@@ -2418,7 +2418,7 @@ int git_configset_get_string(struct config_set *set, const char *key, char **des
 {
 	const char *value;
 	if (!git_configset_get_value(set, key, &value, NULL))
-		return git_config_string((const char **)dest, key, value);
+		return git_config_string(dest, key, value);
 	else
 		return 1;
 }
diff --git a/config.h b/config.h
@@ -280,7 +280,7 @@ int git_config_bool(const char *, const char *);
  * Allocates and copies the value string into the `dest` parameter; if no
  * string is given, prints an error message and returns -1.
  */
-int git_config_string(const char **, const char *, const char *);
+int git_config_string(char **, const char *, const char *);
 
 /**
  * Similar to `git_config_string`, but expands `~` or `~user` into the
diff --git a/convert.c b/convert.c
@@ -981,9 +981,9 @@ int async_query_available_blobs(const char *cmd, struct string_list *available_p
 static struct convert_driver {
 	const char *name;
 	struct convert_driver *next;
-	const char *smudge;
-	const char *clean;
-	const char *process;
+	char *smudge;
+	char *clean;
+	char *process;
 	int required;
 } *user_convert, **user_convert_tail;
 
diff --git a/delta-islands.c b/delta-islands.c
@@ -313,7 +313,7 @@ struct island_load_data {
 	size_t nr;
 	size_t alloc;
 };
-static const char *core_island_name;
+static char *core_island_name;
 
 static void free_config_regexes(struct island_load_data *ild)
 {
diff --git a/diff.c b/diff.c
@@ -56,8 +56,8 @@ static int diff_color_moved_default;
 static int diff_color_moved_ws_default;
 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 char *diff_word_regex_cfg;
+static char *external_diff_cmd_cfg;
 static char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
@@ -412,11 +412,11 @@ int git_diff_ui_config(const char *var, const char *value,
 	}
 	if (!strcmp(var, "diff.srcprefix")) {
 		FREE_AND_NULL(diff_src_prefix);
-		return git_config_string((const char **) &diff_src_prefix, var, value);
+		return git_config_string(&diff_src_prefix, var, value);
 	}
 	if (!strcmp(var, "diff.dstprefix")) {
 		FREE_AND_NULL(diff_dst_prefix);
-		return git_config_string((const char **) &diff_dst_prefix, var, value);
+		return git_config_string(&diff_dst_prefix, var, value);
 	}
 	if (!strcmp(var, "diff.relative")) {
 		diff_relative = git_config_bool(var, value);
diff --git a/environment.c b/environment.c
@@ -42,8 +42,8 @@ int is_bare_repository_cfg = -1; /* unspecified */
 int warn_ambiguous_refs = 1;
 int warn_on_object_refname_ambiguity = 1;
 int repository_format_precious_objects;
-const char *git_commit_encoding;
-const char *git_log_output_encoding;
+char *git_commit_encoding;
+char *git_log_output_encoding;
 char *apply_default_whitespace;
 char *apply_default_ignorewhitespace;
 char *git_attributes_file;
@@ -58,8 +58,8 @@ size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
 size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
 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;
+char *editor_program;
+char *askpass_program;
 char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 enum eol core_eol = EOL_UNSET;
diff --git a/environment.h b/environment.h
@@ -224,11 +224,11 @@ int odb_pack_keep(const char *name);
 const char *get_log_output_encoding(void);
 const char *get_commit_output_encoding(void);
 
-extern const char *git_commit_encoding;
-extern const char *git_log_output_encoding;
+extern char *git_commit_encoding;
+extern char *git_log_output_encoding;
 
-extern const char *editor_program;
-extern const char *askpass_program;
+extern char *editor_program;
+extern char *askpass_program;
 extern char *excludes_file;
 
 /*
diff --git a/gpg-interface.c b/gpg-interface.c
@@ -27,14 +27,14 @@ static void gpg_interface_lazy_init(void)
 }
 
 static char *configured_signing_key;
-static const char *ssh_default_key_command;
+static 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 {
 	const char *name;
-	const char *program;
+	char *program;
 	const char **verify_args;
 	const char **sigs;
 	int (*verify_signed_buffer)(struct signature_check *sigc,
diff --git a/http.c b/http.c
@@ -38,11 +38,11 @@ char curl_errorstr[CURL_ERROR_SIZE];
 
 static int curl_ssl_verify = -1;
 static int curl_ssl_try;
-static const char *curl_http_version = NULL;
+static char *curl_http_version;
 static char *ssl_cert;
 static char *ssl_cert_type;
-static const char *ssl_cipherlist;
-static const char *ssl_version;
+static char *ssl_cipherlist;
+static char *ssl_version;
 static struct {
 	const char *name;
 	long ssl_version;
@@ -95,7 +95,7 @@ static struct {
 	 */
 };
 #ifdef CURLGSSAPI_DELEGATION_FLAG
-static const char *curl_deleg;
+static char *curl_deleg;
 static struct {
 	const char *name;
 	long curl_deleg_param;
@@ -383,11 +383,11 @@ static int http_options(const char *var, const char *value,
 	if (!strcmp("http.sslcert", var))
 		return git_config_pathname(&ssl_cert, var, value);
 	if (!strcmp("http.sslcerttype", var))
-		return git_config_string((const char **)&ssl_cert_type, var, value);
+		return git_config_string(&ssl_cert_type, var, value);
 	if (!strcmp("http.sslkey", var))
 		return git_config_pathname(&ssl_key, var, value);
 	if (!strcmp("http.sslkeytype", var))
-		return git_config_string((const char **)&ssl_key_type, var, value);
+		return git_config_string(&ssl_key_type, var, value);
 	if (!strcmp("http.sslcapath", var))
 		return git_config_pathname(&ssl_capath, var, value);
 	if (!strcmp("http.sslcainfo", var))
@@ -440,19 +440,19 @@ static int http_options(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp("http.proxy", var))
-		return git_config_string((const char **)&curl_http_proxy, var, value);
+		return git_config_string(&curl_http_proxy, var, value);
 
 	if (!strcmp("http.proxyauthmethod", var))
-		return git_config_string((const char **)&http_proxy_authmethod, var, value);
+		return git_config_string(&http_proxy_authmethod, var, value);
 
 	if (!strcmp("http.proxysslcert", var))
-		return git_config_string((const char **)&http_proxy_ssl_cert, var, value);
+		return git_config_string(&http_proxy_ssl_cert, var, value);
 
 	if (!strcmp("http.proxysslkey", var))
-		return git_config_string((const char **)&http_proxy_ssl_key, var, value);
+		return git_config_string(&http_proxy_ssl_key, var, value);
 
 	if (!strcmp("http.proxysslcainfo", var))
-		return git_config_string((const char **)&http_proxy_ssl_ca_info, var, value);
+		return git_config_string(&http_proxy_ssl_ca_info, var, value);
 
 	if (!strcmp("http.proxysslcertpasswordprotected", var)) {
 		proxy_ssl_cert_password_required = git_config_bool(var, value);
@@ -476,7 +476,7 @@ static int http_options(const char *var, const char *value,
 	}
 
 	if (!strcmp("http.useragent", var))
-		return git_config_string((const char **)&user_agent, var, value);
+		return git_config_string(&user_agent, var, value);
 
 	if (!strcmp("http.emptyauth", var)) {
 		if (value && !strcmp("auto", value))
diff --git a/imap-send.c b/imap-send.c
@@ -70,16 +70,16 @@ static char *next_arg(char **);
 
 struct imap_server_conf {
 	const char *name;
-	const char *tunnel;
-	const char *host;
+	char *tunnel;
+	char *host;
 	int port;
-	const char *folder;
-	const char *user;
-	const char *pass;
+	char *folder;
+	char *user;
+	char *pass;
 	int use_ssl;
 	int ssl_verify;
 	int use_html;
-	const char *auth_method;
+	char *auth_method;
 };
 
 static struct imap_server_conf server = {
diff --git a/mailmap.c b/mailmap.c
@@ -7,7 +7,7 @@
 #include "setup.h"
 
 char *git_mailmap_file;
-const char *git_mailmap_blob;
+char *git_mailmap_blob;
 
 struct mailmap_info {
 	char *name;
diff --git a/mailmap.h b/mailmap.h
@@ -4,7 +4,7 @@
 struct string_list;
 
 extern char *git_mailmap_file;
-extern const char *git_mailmap_blob;
+extern char *git_mailmap_blob;
 
 int read_mailmap(struct string_list *map);
 void clear_mailmap(struct string_list *map);
diff --git a/merge-ll.c b/merge-ll.c
diff --git a/pager.c b/pager.c
diff --git a/pretty.c b/pretty.c
diff --git a/promisor-remote.h b/promisor-remote.h
diff --git a/remote.c b/remote.c
diff --git a/remote.h b/remote.h
diff --git a/sequencer.c b/sequencer.c
diff --git a/upload-pack.c b/upload-pack.c
diff --git a/userdiff.h b/userdiff.h

Original file line number	Diff line number	Diff line change
`@@ -582,11 +582,11 @@ static int git_log_config(const char var, const char value,`
`582`	`582`
`583`	`583`	`if (!strcmp(var, "format.pretty")) {`
`584`	`584`	`FREE_AND_NULL(cfg->fmt_pretty);`
`585`		`- return git_config_string((const char **) &cfg->fmt_pretty, var, value);`
	`585`	`+ return git_config_string(&cfg->fmt_pretty, var, value);`
`586`	`586`	`}`
`587`	`587`	`if (!strcmp(var, "format.subjectprefix")) {`
`588`	`588`	`FREE_AND_NULL(cfg->fmt_patch_subject_prefix);`
`589`		`- return git_config_string((const char **) &cfg->fmt_patch_subject_prefix, var, value);`
	`589`	`+ return git_config_string(&cfg->fmt_patch_subject_prefix, var, value);`
`590`	`590`	`}`
`591`	`591`	`if (!strcmp(var, "format.filenamemaxlength")) {`
`592`	`592`	`cfg->fmt_patch_name_max = git_config_int(var, value, ctx->kvi);`
`@@ -602,7 +602,7 @@ static int git_log_config(const char var, const char value,`
`602`	`602`	`}`
`603`	`603`	`if (!strcmp(var, "log.date")) {`
`604`	`604`	`FREE_AND_NULL(cfg->default_date_mode);`
`605`		`- return git_config_string((const char **) &cfg->default_date_mode, var, value);`
	`605`	`+ return git_config_string(&cfg->default_date_mode, var, value);`
`606`	`606`	`}`
`607`	`607`	`if (!strcmp(var, "log.decorate")) {`
`608`	`608`	`cfg->decoration_style = parse_decoration_style(value);`
`@@ -1076,7 +1076,7 @@ static int git_format_config(const char var, const char value,`
`1076`	`1076`	`}`
`1077`	`1077`	`if (!strcmp(var, "format.suffix")) {`
`1078`	`1078`	`FREE_AND_NULL(cfg->fmt_patch_suffix);`
`1079`		`- return git_config_string((const char **) &cfg->fmt_patch_suffix, var, value);`
	`1079`	`+ return git_config_string(&cfg->fmt_patch_suffix, var, value);`
`1080`	`1080`	`}`
`1081`	`1081`	`if (!strcmp(var, "format.to")) {`
`1082`	`1082`	`if (!value)`
`@@ -1133,7 +1133,7 @@ static int git_format_config(const char var, const char value,`
`1133`	`1133`	`}`
`1134`	`1134`	`if (!strcmp(var, "format.signature")) {`
`1135`	`1135`	`FREE_AND_NULL(cfg->signature);`
`1136`		`- return git_config_string((const char **) &cfg->signature, var, value);`
	`1136`	`+ return git_config_string(&cfg->signature, var, value);`
`1137`	`1137`	`}`
`1138`	`1138`	`if (!strcmp(var, "format.signaturefile")) {`
`1139`	`1139`	`FREE_AND_NULL(cfg->signature_file);`
`@@ -1149,7 +1149,7 @@ static int git_format_config(const char var, const char value,`
`1149`	`1149`	`}`
`1150`	`1150`	`if (!strcmp(var, "format.outputdirectory")) {`
`1151`	`1151`	`FREE_AND_NULL(cfg->config_output_directory);`
`1152`		`- return git_config_string((const char **) &cfg->config_output_directory, var, value);`
	`1152`	`+ return git_config_string(&cfg->config_output_directory, var, value);`
`1153`	`1153`	`}`
`1154`	`1154`	`if (!strcmp(var, "format.useautobase")) {`
`1155`	`1155`	`if (value && !strcasecmp(value, "whenAble")) {`
Original file line number	Diff line number	Diff line change
`@@ -1338,7 +1338,7 @@ int git_config_bool(const char name, const char value)`
`1338`	`1338`	`return v;`
`1339`	`1339`	`}`
`1340`	`1340`
`1341`		`-int git_config_string(const char *dest, const char var, const char *value)`
	`1341`	`+int git_config_string(char *dest, const char var, const char *value)`
`1342`	`1342`	`{`
`1343`	`1343`	`if (!value)`
`1344`	`1344`	`return config_error_nonbool(var);`
`@@ -1566,7 +1566,7 @@ static int git_default_core_config(const char var, const char value,`
`1566`	`1566`
`1567`	`1567`	`if (!strcmp(var, "core.checkroundtripencoding")) {`
`1568`	`1568`	`FREE_AND_NULL(check_roundtrip_encoding);`
`1569`		`- return git_config_string((const char **) &check_roundtrip_encoding, var, value);`
	`1569`	`+ return git_config_string(&check_roundtrip_encoding, var, value);`
`1570`	`1570`	`}`
`1571`	`1571`
`1572`	`1572`	`if (!strcmp(var, "core.notesref")) {`
`@@ -2418,7 +2418,7 @@ int git_configset_get_string(struct config_set set, const char key, char **des`
`2418`	`2418`	`{`
`2419`	`2419`	`const char *value;`
`2420`	`2420`	`if (!git_configset_get_value(set, key, &value, NULL))`
`2421`		`- return git_config_string((const char **)dest, key, value);`
	`2421`	`+ return git_config_string(dest, key, value);`
`2422`	`2422`	`else`
`2423`	`2423`	`return 1;`
`2424`	`2424`	`}`