From 6073b3b5c37716c50244d635e7c358f41f43e286 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 27 May 2024 13:46:15 +0200
Subject: config: clarify memory ownership in `git_config_pathname()`

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 <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'builtin/log.c')

diff --git a/builtin/log.c b/builtin/log.c
index b17dd8b40a..a2f5845556 100644
--- 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;
-- 
cgit v1.3-5-g9baa


From 106a54aecb21fb17956290aee4e9204aa29174e7 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 27 May 2024 13:46:30 +0200
Subject: builtin/log: stop using globals for log config

We're using global variables to store the log configuration. Many of
these can be set both via the command line and via the config, and
depending on how they are being set, they may contain allocated strings.
This leads to hard-to-track memory ownership and memory leaks.

Refactor the code to instead use a `struct log_config` that is being
allocated on the stack. This allows us to more clearly scope the
variables, track memory ownership and ultimately release the memory.

This also prepares us for a change to `git_config_string()`, which will
be adapted to have a `char **` out parameter instead of `const char **`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c | 259 +++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 156 insertions(+), 103 deletions(-)

(limited to 'builtin/log.c')

diff --git a/builtin/log.c b/builtin/log.c
index a2f5845556..f5da29ee2a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -48,22 +48,8 @@
 #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
 #define FORMAT_PATCH_NAME_MAX_DEFAULT 64
 
-/* Set a default date-time format for git log ("log.date" config variable) */
-static const char *default_date_mode = NULL;
-
-static int default_abbrev_commit;
-static int default_show_root = 1;
-static int default_follow;
-static int default_show_signature;
-static int default_encode_email_headers = 1;
-static int decoration_style;
-static int decoration_given;
-static int use_mailmap_config = 1;
 static unsigned int force_in_body_from;
 static int stdout_mboxrd;
-static const char *fmt_patch_subject_prefix = "PATCH";
-static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;
-static const char *fmt_pretty;
 static int format_no_prefix;
 
 static const char * const builtin_log_usage[] = {
@@ -111,6 +97,39 @@ static int parse_decoration_style(const char *value)
 	return -1;
 }
 
+struct log_config {
+	int default_abbrev_commit;
+	int default_show_root;
+	int default_follow;
+	int default_show_signature;
+	int default_encode_email_headers;
+	int decoration_style;
+	int decoration_given;
+	int use_mailmap_config;
+	char *fmt_patch_subject_prefix;
+	int fmt_patch_name_max;
+	char *fmt_pretty;
+	char *default_date_mode;
+};
+
+static void log_config_init(struct log_config *cfg)
+{
+	memset(cfg, 0, sizeof(*cfg));
+	cfg->default_show_root = 1;
+	cfg->default_encode_email_headers = 1;
+	cfg->use_mailmap_config = 1;
+	cfg->fmt_patch_subject_prefix = xstrdup("PATCH");
+	cfg->fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;
+	cfg->decoration_style = auto_decoration_style();
+}
+
+static void log_config_release(struct log_config *cfg)
+{
+	free(cfg->default_date_mode);
+	free(cfg->fmt_pretty);
+	free(cfg->fmt_patch_subject_prefix);
+}
+
 static int use_default_decoration_filter = 1;
 static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
 static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
@@ -127,20 +146,22 @@ static int clear_decorations_callback(const struct option *opt UNUSED,
 	return 0;
 }
 
-static int decorate_callback(const struct option *opt UNUSED, const char *arg,
+static int decorate_callback(const struct option *opt, const char *arg,
 			     int unset)
 {
+	struct log_config *cfg = opt->value;
+
 	if (unset)
-		decoration_style = 0;
+		cfg->decoration_style = 0;
 	else if (arg)
-		decoration_style = parse_decoration_style(arg);
+		cfg->decoration_style = parse_decoration_style(arg);
 	else
-		decoration_style = DECORATE_SHORT_REFS;
+		cfg->decoration_style = DECORATE_SHORT_REFS;
 
-	if (decoration_style < 0)
+	if (cfg->decoration_style < 0)
 		die(_("invalid --decorate option: %s"), arg);
 
-	decoration_given = 1;
+	cfg->decoration_given = 1;
 
 	return 0;
 }
@@ -160,32 +181,26 @@ static int log_line_range_callback(const struct option *option, const char *arg,
 	return 0;
 }
 
-static void init_log_defaults(void)
+static void cmd_log_init_defaults(struct rev_info *rev,
+				  struct log_config *cfg)
 {
-	init_diff_ui_defaults();
-
-	decoration_style = auto_decoration_style();
-}
-
-static void cmd_log_init_defaults(struct rev_info *rev)
-{
-	if (fmt_pretty)
-		get_commit_format(fmt_pretty, rev);
-	if (default_follow)
+	if (cfg->fmt_pretty)
+		get_commit_format(cfg->fmt_pretty, rev);
+	if (cfg->default_follow)
 		rev->diffopt.flags.default_follow_renames = 1;
 	rev->verbose_header = 1;
 	init_diffstat_widths(&rev->diffopt);
 	rev->diffopt.flags.recursive = 1;
 	rev->diffopt.flags.allow_textconv = 1;
-	rev->abbrev_commit = default_abbrev_commit;
-	rev->show_root_diff = default_show_root;
-	rev->subject_prefix = fmt_patch_subject_prefix;
-	rev->patch_name_max = fmt_patch_name_max;
-	rev->show_signature = default_show_signature;
-	rev->encode_email_headers = default_encode_email_headers;
+	rev->abbrev_commit = cfg->default_abbrev_commit;
+	rev->show_root_diff = cfg->default_show_root;
+	rev->subject_prefix = cfg->fmt_patch_subject_prefix;
+	rev->patch_name_max = cfg->fmt_patch_name_max;
+	rev->show_signature = cfg->default_show_signature;
+	rev->encode_email_headers = cfg->default_encode_email_headers;
 
-	if (default_date_mode)
-		parse_date_format(default_date_mode, &rev->date_mode);
+	if (cfg->default_date_mode)
+		parse_date_format(cfg->default_date_mode, &rev->date_mode);
 }
 
 static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
@@ -233,7 +248,8 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
 }
 
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
-			 struct rev_info *rev, struct setup_revision_opt *opt)
+			 struct rev_info *rev, struct setup_revision_opt *opt,
+			 struct log_config *cfg)
 {
 	struct userformat_want w;
 	int quiet = 0, source = 0, mailmap;
@@ -258,7 +274,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 				N_("pattern"), N_("only decorate refs that match <pattern>")),
 		OPT_STRING_LIST(0, "decorate-refs-exclude", &decorate_refs_exclude,
 				N_("pattern"), N_("do not decorate refs that match <pattern>")),
-		OPT_CALLBACK_F(0, "decorate", NULL, NULL, N_("decorate options"),
+		OPT_CALLBACK_F(0, "decorate", cfg, NULL, N_("decorate options"),
 			       PARSE_OPT_OPTARG, decorate_callback),
 		OPT_CALLBACK('L', NULL, &line_cb, "range:file",
 			     N_("trace the evolution of line range <start>,<end> or function :<funcname> in <file>"),
@@ -269,7 +285,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	line_cb.rev = rev;
 	line_cb.prefix = prefix;
 
-	mailmap = use_mailmap_config;
+	mailmap = cfg->use_mailmap_config;
 	argc = parse_options(argc, argv, prefix,
 			     builtin_log_options, builtin_log_usage,
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT |
@@ -314,8 +330,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		 * "log --pretty=raw" is special; ignore UI oriented
 		 * configuration variables such as decoration.
 		 */
-		if (!decoration_given)
-			decoration_style = 0;
+		if (!cfg->decoration_given)
+			cfg->decoration_style = 0;
 		if (!rev->abbrev_commit_given)
 			rev->abbrev_commit = 0;
 	}
@@ -326,24 +342,24 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			 * Disable decoration loading if the format will not
 			 * show them anyway.
 			 */
-			decoration_style = 0;
-		} else if (!decoration_style) {
+			cfg->decoration_style = 0;
+		} else if (!cfg->decoration_style) {
 			/*
 			 * If we are going to show them, make sure we do load
 			 * them here, but taking care not to override a
 			 * specific style set by config or --decorate.
 			 */
-			decoration_style = DECORATE_SHORT_REFS;
+			cfg->decoration_style = DECORATE_SHORT_REFS;
 		}
 	}
 
-	if (decoration_style || rev->simplify_by_decoration) {
+	if (cfg->decoration_style || rev->simplify_by_decoration) {
 		set_default_decoration_filter(&decoration_filter);
 
-		if (decoration_style)
+		if (cfg->decoration_style)
 			rev->show_decorations = 1;
 
-		load_ref_decorations(&decoration_filter, decoration_style);
+		load_ref_decorations(&decoration_filter, cfg->decoration_style);
 	}
 
 	if (rev->line_level_traverse)
@@ -353,16 +369,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 }
 
 static void cmd_log_init(int argc, const char **argv, const char *prefix,
-			 struct rev_info *rev, struct setup_revision_opt *opt)
+			 struct rev_info *rev, struct setup_revision_opt *opt,
+			 struct log_config *cfg)
 {
-	cmd_log_init_defaults(rev);
-	cmd_log_init_finish(argc, argv, prefix, rev, opt);
-}
-
-static int cmd_log_deinit(int ret, struct rev_info *rev)
-{
-	release_revisions(rev);
-	return ret;
+	cmd_log_init_defaults(rev, cfg);
+	cmd_log_init_finish(argc, argv, prefix, rev, opt, cfg);
 }
 
 /*
@@ -566,30 +577,37 @@ static int cmd_log_walk(struct rev_info *rev)
 static int git_log_config(const char *var, const char *value,
 			  const struct config_context *ctx, void *cb)
 {
+	struct log_config *cfg = cb;
 	const char *slot_name;
 
-	if (!strcmp(var, "format.pretty"))
-		return git_config_string(&fmt_pretty, var, value);
-	if (!strcmp(var, "format.subjectprefix"))
-		return git_config_string(&fmt_patch_subject_prefix, var, value);
+	if (!strcmp(var, "format.pretty")) {
+		FREE_AND_NULL(cfg->fmt_pretty);
+		return git_config_string((const char **) &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);
+	}
 	if (!strcmp(var, "format.filenamemaxlength")) {
-		fmt_patch_name_max = git_config_int(var, value, ctx->kvi);
+		cfg->fmt_patch_name_max = git_config_int(var, value, ctx->kvi);
 		return 0;
 	}
 	if (!strcmp(var, "format.encodeemailheaders")) {
-		default_encode_email_headers = git_config_bool(var, value);
+		cfg->default_encode_email_headers = git_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "log.abbrevcommit")) {
-		default_abbrev_commit = git_config_bool(var, value);
+		cfg->default_abbrev_commit = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "log.date"))
-		return git_config_string(&default_date_mode, var, 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);
+	}
 	if (!strcmp(var, "log.decorate")) {
-		decoration_style = parse_decoration_style(value);
-		if (decoration_style < 0)
-			decoration_style = 0; /* maybe warn? */
+		cfg->decoration_style = parse_decoration_style(value);
+		if (cfg->decoration_style < 0)
+			cfg->decoration_style = 0; /* maybe warn? */
 		return 0;
 	}
 	if (!strcmp(var, "log.diffmerges")) {
@@ -598,21 +616,21 @@ static int git_log_config(const char *var, const char *value,
 		return diff_merges_config(value);
 	}
 	if (!strcmp(var, "log.showroot")) {
-		default_show_root = git_config_bool(var, value);
+		cfg->default_show_root = git_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "log.follow")) {
-		default_follow = git_config_bool(var, value);
+		cfg->default_follow = git_config_bool(var, value);
 		return 0;
 	}
 	if (skip_prefix(var, "color.decorate.", &slot_name))
 		return parse_decorate_color_config(var, slot_name, value);
 	if (!strcmp(var, "log.mailmap")) {
-		use_mailmap_config = git_config_bool(var, value);
+		cfg->use_mailmap_config = git_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "log.showsignature")) {
-		default_show_signature = git_config_bool(var, value);
+		cfg->default_show_signature = git_config_bool(var, value);
 		return 0;
 	}
 
@@ -621,11 +639,14 @@ static int git_log_config(const char *var, const char *value,
 
 int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 {
+	struct log_config cfg;
 	struct rev_info rev;
 	struct setup_revision_opt opt;
+	int ret;
 
-	init_log_defaults();
-	git_config(git_log_config, NULL);
+	log_config_init(&cfg);
+	init_diff_ui_defaults();
+	git_config(git_log_config, &cfg);
 
 	repo_init_revisions(the_repository, &rev, prefix);
 	git_config(grep_config, &rev.grep_filter);
@@ -635,10 +656,15 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 	memset(&opt, 0, sizeof(opt));
 	opt.def = "HEAD";
 	opt.revarg_opt = REVARG_COMMITTISH;
-	cmd_log_init(argc, argv, prefix, &rev, &opt);
+	cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg);
 	if (!rev.diffopt.output_format)
 		rev.diffopt.output_format = DIFF_FORMAT_RAW;
-	return cmd_log_deinit(cmd_log_walk(&rev), &rev);
+
+	ret = cmd_log_walk(&rev);
+
+	release_revisions(&rev);
+	log_config_release(&cfg);
+	return ret;
 }
 
 static void show_tagger(const char *buf, struct rev_info *rev)
@@ -733,14 +759,16 @@ static void show_setup_revisions_tweak(struct rev_info *rev)
 
 int cmd_show(int argc, const char **argv, const char *prefix)
 {
+	struct log_config cfg;
 	struct rev_info rev;
 	unsigned int i;
 	struct setup_revision_opt opt;
 	struct pathspec match_all;
 	int ret = 0;
 
-	init_log_defaults();
-	git_config(git_log_config, NULL);
+	log_config_init(&cfg);
+	init_diff_ui_defaults();
+	git_config(git_log_config, &cfg);
 
 	if (the_repository->gitdir) {
 		prepare_repo_settings(the_repository);
@@ -759,10 +787,14 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 	memset(&opt, 0, sizeof(opt));
 	opt.def = "HEAD";
 	opt.tweak = show_setup_revisions_tweak;
-	cmd_log_init(argc, argv, prefix, &rev, &opt);
+	cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg);
 
-	if (!rev.no_walk)
-		return cmd_log_deinit(cmd_log_walk(&rev), &rev);
+	if (!rev.no_walk) {
+		ret = cmd_log_walk(&rev);
+		release_revisions(&rev);
+		log_config_release(&cfg);
+		return ret;
+	}
 
 	rev.diffopt.no_free = 1;
 	for (i = 0; i < rev.pending.nr && !ret; i++) {
@@ -832,8 +864,10 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 
 	rev.diffopt.no_free = 0;
 	diff_free(&rev.diffopt);
+	release_revisions(&rev);
+	log_config_release(&cfg);
 
-	return cmd_log_deinit(ret, &rev);
+	return ret;
 }
 
 /*
@@ -841,11 +875,14 @@ int cmd_show(int argc, const char **argv, const char *prefix)
  */
 int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 {
+	struct log_config cfg;
 	struct rev_info rev;
 	struct setup_revision_opt opt;
+	int ret;
 
-	init_log_defaults();
-	git_config(git_log_config, NULL);
+	log_config_init(&cfg);
+	init_diff_ui_defaults();
+	git_config(git_log_config, &cfg);
 
 	repo_init_revisions(the_repository, &rev, prefix);
 	init_reflog_walk(&rev.reflog_info);
@@ -854,14 +891,18 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
 	rev.verbose_header = 1;
 	memset(&opt, 0, sizeof(opt));
 	opt.def = "HEAD";
-	cmd_log_init_defaults(&rev);
+	cmd_log_init_defaults(&rev, &cfg);
 	rev.abbrev_commit = 1;
 	rev.commit_format = CMIT_FMT_ONELINE;
 	rev.use_terminator = 1;
 	rev.always_show_header = 1;
-	cmd_log_init_finish(argc, argv, prefix, &rev, &opt);
+	cmd_log_init_finish(argc, argv, prefix, &rev, &opt, &cfg);
+
+	ret = cmd_log_walk(&rev);
 
-	return cmd_log_deinit(cmd_log_walk(&rev), &rev);
+	release_revisions(&rev);
+	log_config_release(&cfg);
+	return ret;
 }
 
 static void log_setup_revisions_tweak(struct rev_info *rev)
@@ -876,11 +917,14 @@ static void log_setup_revisions_tweak(struct rev_info *rev)
 
 int cmd_log(int argc, const char **argv, const char *prefix)
 {
+	struct log_config cfg;
 	struct rev_info rev;
 	struct setup_revision_opt opt;
+	int ret;
 
-	init_log_defaults();
-	git_config(git_log_config, NULL);
+	log_config_init(&cfg);
+	init_diff_ui_defaults();
+	git_config(git_log_config, &cfg);
 
 	repo_init_revisions(the_repository, &rev, prefix);
 	git_config(grep_config, &rev.grep_filter);
@@ -890,8 +934,13 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 	opt.def = "HEAD";
 	opt.revarg_opt = REVARG_COMMITTISH;
 	opt.tweak = log_setup_revisions_tweak;
-	cmd_log_init(argc, argv, prefix, &rev, &opt);
-	return cmd_log_deinit(cmd_log_walk(&rev), &rev);
+	cmd_log_init(argc, argv, prefix, &rev, &opt, &cfg);
+
+	ret = cmd_log_walk(&rev);
+
+	release_revisions(&rev);
+	log_config_release(&cfg);
+	return ret;
 }
 
 /* format-patch */
@@ -1884,6 +1933,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,
 
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
+	struct log_config cfg;
 	struct commit *commit;
 	struct commit **list = NULL;
 	struct rev_info rev;
@@ -1943,7 +1993,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    N_("start numbering patches at <n> instead of 1")),
 		OPT_STRING('v', "reroll-count", &reroll_count, N_("reroll-count"),
 			    N_("mark the series as Nth re-roll")),
-		OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max,
+		OPT_INTEGER(0, "filename-max-length", &cfg.fmt_patch_name_max,
 			    N_("max length of output filename")),
 		OPT_CALLBACK_F(0, "rfc", &rfc, N_("rfc"),
 			       N_("add <rfc> (default 'RFC') before 'PATCH'"),
@@ -2017,16 +2067,17 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	extra_to.strdup_strings = 1;
 	extra_cc.strdup_strings = 1;
 
-	init_log_defaults();
+	log_config_init(&cfg);
+	init_diff_ui_defaults();
 	init_display_notes(&notes_opt);
-	git_config(git_format_config, NULL);
+	git_config(git_format_config, &cfg);
 	repo_init_revisions(the_repository, &rev, prefix);
 	git_config(grep_config, &rev.grep_filter);
 
 	rev.show_notes = show_notes;
 	memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
 	rev.commit_format = CMIT_FMT_EMAIL;
-	rev.encode_email_headers = default_encode_email_headers;
+	rev.encode_email_headers = cfg.default_encode_email_headers;
 	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
 	rev.diff = 1;
@@ -2037,7 +2088,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	s_r_opt.def = "HEAD";
 	s_r_opt.revarg_opt = REVARG_COMMITTISH;
 
-	strbuf_addstr(&sprefix, fmt_patch_subject_prefix);
+	strbuf_addstr(&sprefix, cfg.fmt_patch_subject_prefix);
 	if (format_no_prefix)
 		diff_set_noprefix(&rev.diffopt);
 
@@ -2059,8 +2110,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.force_in_body_from = force_in_body_from;
 
 	/* Make sure "0000-$sub.patch" gives non-negative length for $sub */
-	if (fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
-		fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
+	if (cfg.fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
+		cfg.fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
 
 	if (cover_from_description_arg)
 		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
@@ -2156,7 +2207,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.always_show_header = 1;
 
 	rev.zero_commit = zero_commit;
-	rev.patch_name_max = fmt_patch_name_max;
+	rev.patch_name_max = cfg.fmt_patch_name_max;
 
 	if (!rev.diffopt.flags.text && !no_binary_diff)
 		rev.diffopt.flags.binary = 1;
@@ -2450,7 +2501,9 @@ done:
 	if (rev.ref_message_ids)
 		string_list_clear(rev.ref_message_ids, 0);
 	free(rev.ref_message_ids);
-	return cmd_log_deinit(0, &rev);
+	release_revisions(&rev);
+	log_config_release(&cfg);
+	return 0;
 }
 
 static int add_pending_commit(const char *arg, struct rev_info *revs, int flags)
-- 
cgit v1.3-5-g9baa


From 83024d98f78684fce3d4d0598e6b2b147a4b2ffa Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 27 May 2024 13:46:34 +0200
Subject: builtin/log: stop using globals for format config

This commit does the exact same as the preceding commit, only for the
format configuration instead of the log configuration.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c | 467 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 265 insertions(+), 202 deletions(-)

(limited to 'builtin/log.c')

diff --git a/builtin/log.c b/builtin/log.c
index f5da29ee2a..890bf0c425 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -945,36 +945,6 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 
 /* format-patch */
 
-static const char *fmt_patch_suffix = ".patch";
-static int numbered = 0;
-static int auto_number = 1;
-
-static char *default_attach = NULL;
-
-static struct string_list extra_hdr = STRING_LIST_INIT_NODUP;
-static struct string_list extra_to = STRING_LIST_INIT_NODUP;
-static struct string_list extra_cc = STRING_LIST_INIT_NODUP;
-
-static void add_header(const char *value)
-{
-	struct string_list_item *item;
-	int len = strlen(value);
-	while (len && value[len - 1] == '\n')
-		len--;
-
-	if (!strncasecmp(value, "to: ", 4)) {
-		item = string_list_append(&extra_to, value + 4);
-		len -= 4;
-	} else if (!strncasecmp(value, "cc: ", 4)) {
-		item = string_list_append(&extra_cc, value + 4);
-		len -= 4;
-	} else {
-		item = string_list_append(&extra_hdr, value);
-	}
-
-	item->string[len] = '\0';
-}
-
 enum cover_setting {
 	COVER_UNSET,
 	COVER_OFF,
@@ -1001,17 +971,61 @@ enum auto_base_setting {
 	AUTO_BASE_WHEN_ABLE
 };
 
-static enum thread_level thread;
-static int do_signoff;
-static enum auto_base_setting auto_base;
-static char *from;
-static const char *signature = git_version_string;
-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;
-static int show_notes;
-static struct display_notes_opt notes_opt;
+struct format_config {
+	struct log_config log;
+	enum thread_level thread;
+	int do_signoff;
+	enum auto_base_setting auto_base;
+	char *base_commit;
+	char *from;
+	char *signature;
+	char *signature_file;
+	enum cover_setting config_cover_letter;
+	char *config_output_directory;
+	enum cover_from_description cover_from_description_mode;
+	int show_notes;
+	struct display_notes_opt notes_opt;
+	int numbered_cmdline_opt;
+	int numbered;
+	int auto_number;
+	char *default_attach;
+	struct string_list extra_hdr;
+	struct string_list extra_to;
+	struct string_list extra_cc;
+	int keep_subject;
+	int subject_prefix;
+	struct strbuf sprefix;
+	char *fmt_patch_suffix;
+};
+
+static void format_config_init(struct format_config *cfg)
+{
+	memset(cfg, 0, sizeof(*cfg));
+	log_config_init(&cfg->log);
+	cfg->cover_from_description_mode = COVER_FROM_MESSAGE;
+	cfg->auto_number = 1;
+	string_list_init_dup(&cfg->extra_hdr);
+	string_list_init_dup(&cfg->extra_to);
+	string_list_init_dup(&cfg->extra_cc);
+	strbuf_init(&cfg->sprefix, 0);
+	cfg->fmt_patch_suffix = xstrdup(".patch");
+}
+
+static void format_config_release(struct format_config *cfg)
+{
+	log_config_release(&cfg->log);
+	free(cfg->base_commit);
+	free(cfg->from);
+	free(cfg->signature);
+	free(cfg->signature_file);
+	free(cfg->config_output_directory);
+	free(cfg->default_attach);
+	string_list_clear(&cfg->extra_hdr, 0);
+	string_list_clear(&cfg->extra_to, 0);
+	string_list_clear(&cfg->extra_cc, 0);
+	strbuf_release(&cfg->sprefix);
+	free(cfg->fmt_patch_suffix);
+}
 
 static enum cover_from_description parse_cover_from_description(const char *arg)
 {
@@ -1029,27 +1043,51 @@ static enum cover_from_description parse_cover_from_description(const char *arg)
 		die(_("%s: invalid cover from description mode"), arg);
 }
 
+static void add_header(struct format_config *cfg, const char *value)
+{
+	struct string_list_item *item;
+	int len = strlen(value);
+	while (len && value[len - 1] == '\n')
+		len--;
+
+	if (!strncasecmp(value, "to: ", 4)) {
+		item = string_list_append(&cfg->extra_to, value + 4);
+		len -= 4;
+	} else if (!strncasecmp(value, "cc: ", 4)) {
+		item = string_list_append(&cfg->extra_cc, value + 4);
+		len -= 4;
+	} else {
+		item = string_list_append(&cfg->extra_hdr, value);
+	}
+
+	item->string[len] = '\0';
+}
+
 static int git_format_config(const char *var, const char *value,
 			     const struct config_context *ctx, void *cb)
 {
+	struct format_config *cfg = cb;
+
 	if (!strcmp(var, "format.headers")) {
 		if (!value)
 			die(_("format.headers without value"));
-		add_header(value);
+		add_header(cfg, value);
 		return 0;
 	}
-	if (!strcmp(var, "format.suffix"))
-		return git_config_string(&fmt_patch_suffix, var, 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);
+	}
 	if (!strcmp(var, "format.to")) {
 		if (!value)
 			return config_error_nonbool(var);
-		string_list_append(&extra_to, value);
+		string_list_append(&cfg->extra_to, value);
 		return 0;
 	}
 	if (!strcmp(var, "format.cc")) {
 		if (!value)
 			return config_error_nonbool(var);
-		string_list_append(&extra_cc, value);
+		string_list_append(&cfg->extra_cc, value);
 		return 0;
 	}
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff") ||
@@ -1058,69 +1096,76 @@ static int git_format_config(const char *var, const char *value,
 	}
 	if (!strcmp(var, "format.numbered")) {
 		if (value && !strcasecmp(value, "auto")) {
-			auto_number = 1;
+			cfg->auto_number = 1;
 			return 0;
 		}
-		numbered = git_config_bool(var, value);
-		auto_number = auto_number && numbered;
+		cfg->numbered = git_config_bool(var, value);
+		cfg->auto_number = cfg->auto_number && cfg->numbered;
 		return 0;
 	}
 	if (!strcmp(var, "format.attach")) {
-		if (value && *value)
-			default_attach = xstrdup(value);
-		else if (value && !*value)
-			FREE_AND_NULL(default_attach);
-		else
-			default_attach = xstrdup(git_version_string);
+		if (value && *value) {
+			FREE_AND_NULL(cfg->default_attach);
+			cfg->default_attach = xstrdup(value);
+		} else if (value && !*value) {
+			FREE_AND_NULL(cfg->default_attach);
+		} else {
+			FREE_AND_NULL(cfg->default_attach);
+			cfg->default_attach = xstrdup(git_version_string);
+		}
 		return 0;
 	}
 	if (!strcmp(var, "format.thread")) {
 		if (value && !strcasecmp(value, "deep")) {
-			thread = THREAD_DEEP;
+			cfg->thread = THREAD_DEEP;
 			return 0;
 		}
 		if (value && !strcasecmp(value, "shallow")) {
-			thread = THREAD_SHALLOW;
+			cfg->thread = THREAD_SHALLOW;
 			return 0;
 		}
-		thread = git_config_bool(var, value) ? THREAD_SHALLOW : THREAD_UNSET;
+		cfg->thread = git_config_bool(var, value) ? THREAD_SHALLOW : THREAD_UNSET;
 		return 0;
 	}
 	if (!strcmp(var, "format.signoff")) {
-		do_signoff = git_config_bool(var, value);
+		cfg->do_signoff = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "format.signature"))
-		return git_config_string(&signature, var, value);
-	if (!strcmp(var, "format.signaturefile"))
-		return git_config_pathname(&signature_file, var, value);
+	if (!strcmp(var, "format.signature")) {
+		FREE_AND_NULL(cfg->signature);
+		return git_config_string((const char **) &cfg->signature, var, value);
+	}
+	if (!strcmp(var, "format.signaturefile")) {
+		FREE_AND_NULL(cfg->signature_file);
+		return git_config_pathname(&cfg->signature_file, var, value);
+	}
 	if (!strcmp(var, "format.coverletter")) {
 		if (value && !strcasecmp(value, "auto")) {
-			config_cover_letter = COVER_AUTO;
+			cfg->config_cover_letter = COVER_AUTO;
 			return 0;
 		}
-		config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
+		cfg->config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
 		return 0;
 	}
-	if (!strcmp(var, "format.outputdirectory"))
-		return git_config_string(&config_output_directory, var, 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);
+	}
 	if (!strcmp(var, "format.useautobase")) {
 		if (value && !strcasecmp(value, "whenAble")) {
-			auto_base = AUTO_BASE_WHEN_ABLE;
+			cfg->auto_base = AUTO_BASE_WHEN_ABLE;
 			return 0;
 		}
-		auto_base = git_config_bool(var, value) ? AUTO_BASE_ALWAYS : AUTO_BASE_NEVER;
+		cfg->auto_base = git_config_bool(var, value) ? AUTO_BASE_ALWAYS : AUTO_BASE_NEVER;
 		return 0;
 	}
 	if (!strcmp(var, "format.from")) {
 		int b = git_parse_maybe_bool(value);
-		free(from);
+		FREE_AND_NULL(cfg->from);
 		if (b < 0)
-			from = xstrdup(value);
+			cfg->from = xstrdup(value);
 		else if (b)
-			from = xstrdup(git_committer_info(IDENT_NO_DATE));
-		else
-			from = NULL;
+			cfg->from = xstrdup(git_committer_info(IDENT_NO_DATE));
 		return 0;
 	}
 	if (!strcmp(var, "format.forceinbodyfrom")) {
@@ -1130,15 +1175,15 @@ static int git_format_config(const char *var, const char *value,
 	if (!strcmp(var, "format.notes")) {
 		int b = git_parse_maybe_bool(value);
 		if (b < 0)
-			enable_ref_display_notes(&notes_opt, &show_notes, value);
+			enable_ref_display_notes(&cfg->notes_opt, &cfg->show_notes, value);
 		else if (b)
-			enable_default_display_notes(&notes_opt, &show_notes);
+			enable_default_display_notes(&cfg->notes_opt, &cfg->show_notes);
 		else
-			disable_display_notes(&notes_opt, &show_notes);
+			disable_display_notes(&cfg->notes_opt, &cfg->show_notes);
 		return 0;
 	}
 	if (!strcmp(var, "format.coverfromdescription")) {
-		cover_from_description_mode = parse_cover_from_description(value);
+		cfg->cover_from_description_mode = parse_cover_from_description(value);
 		return 0;
 	}
 	if (!strcmp(var, "format.mboxrd")) {
@@ -1159,7 +1204,7 @@ static int git_format_config(const char *var, const char *value,
 	if (!strcmp(var, "diff.noprefix"))
 		return 0;
 
-	return git_log_config(var, value, ctx, cb);
+	return git_log_config(var, value, ctx, &cfg->log);
 }
 
 static const char *output_directory = NULL;
@@ -1247,7 +1292,7 @@ static void gen_message_id(struct rev_info *info, char *base)
 	info->message_id = strbuf_detach(&buf, NULL);
 }
 
-static void print_signature(FILE *file)
+static void print_signature(const char *signature, FILE *file)
 {
 	if (!signature || !*signature)
 		return;
@@ -1317,14 +1362,15 @@ static void prepare_cover_text(struct pretty_print_context *pp,
 			       const char *branch_name,
 			       struct strbuf *sb,
 			       const char *encoding,
-			       int need_8bit_cte)
+			       int need_8bit_cte,
+			       const struct format_config *cfg)
 {
 	const char *subject = "*** SUBJECT HERE ***";
 	const char *body = "*** BLURB HERE ***";
 	struct strbuf description_sb = STRBUF_INIT;
 	struct strbuf subject_sb = STRBUF_INIT;
 
-	if (cover_from_description_mode == COVER_FROM_NONE)
+	if (cfg->cover_from_description_mode == COVER_FROM_NONE)
 		goto do_pp;
 
 	if (description_file && *description_file)
@@ -1334,13 +1380,13 @@ static void prepare_cover_text(struct pretty_print_context *pp,
 	if (!description_sb.len)
 		goto do_pp;
 
-	if (cover_from_description_mode == COVER_FROM_SUBJECT ||
-			cover_from_description_mode == COVER_FROM_AUTO)
+	if (cfg->cover_from_description_mode == COVER_FROM_SUBJECT ||
+	    cfg->cover_from_description_mode == COVER_FROM_AUTO)
 		body = format_subject(&subject_sb, description_sb.buf, " ");
 
-	if (cover_from_description_mode == COVER_FROM_MESSAGE ||
-			(cover_from_description_mode == COVER_FROM_AUTO &&
-			 subject_sb.len > COVER_FROM_AUTO_MAX_SUBJECT_LEN))
+	if (cfg->cover_from_description_mode == COVER_FROM_MESSAGE ||
+	    (cfg->cover_from_description_mode == COVER_FROM_AUTO &&
+	     subject_sb.len > COVER_FROM_AUTO_MAX_SUBJECT_LEN))
 		body = description_sb.buf;
 	else
 		subject = subject_sb.buf;
@@ -1377,7 +1423,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      int nr, struct commit **list,
 			      const char *description_file,
 			      const char *branch_name,
-			      int quiet)
+			      int quiet,
+			      const struct format_config *cfg)
 {
 	const char *committer;
 	struct shortlog log;
@@ -1416,7 +1463,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	pp.encode_email_headers = rev->encode_email_headers;
 	pp_user_info(&pp, NULL, &sb, committer, encoding);
 	prepare_cover_text(&pp, description_file, branch_name, &sb,
-			   encoding, need_8bit_cte);
+			   encoding, need_8bit_cte, cfg);
 	fprintf(rev->diffopt.file, "%s\n", sb.buf);
 
 	free(pp.after_subject);
@@ -1517,29 +1564,30 @@ static const char * const builtin_format_patch_usage[] = {
 	NULL
 };
 
-static int keep_subject = 0;
+struct keep_callback_data {
+	struct format_config *cfg;
+	struct rev_info *revs;
+};
 
 static int keep_callback(const struct option *opt, const char *arg, int unset)
 {
+	struct keep_callback_data *data = opt->value;
 	BUG_ON_OPT_NEG(unset);
 	BUG_ON_OPT_ARG(arg);
-	((struct rev_info *)opt->value)->total = -1;
-	keep_subject = 1;
+	data->revs->total = -1;
+	data->cfg->keep_subject = 1;
 	return 0;
 }
 
-static int subject_prefix = 0;
-
 static int subject_prefix_callback(const struct option *opt, const char *arg,
 			    int unset)
 {
-	struct strbuf *sprefix;
+	struct format_config *cfg = opt->value;
 
 	BUG_ON_OPT_NEG(unset);
-	sprefix = opt->value;
-	subject_prefix = 1;
-	strbuf_reset(sprefix);
-	strbuf_addstr(sprefix, arg);
+	cfg->subject_prefix = 1;
+	strbuf_reset(&cfg->sprefix);
+	strbuf_addstr(&cfg->sprefix, arg);
 	return 0;
 }
 
@@ -1556,15 +1604,14 @@ static int rfc_callback(const struct option *opt, const char *arg,
 	return 0;
 }
 
-static int numbered_cmdline_opt = 0;
-
 static int numbered_callback(const struct option *opt, const char *arg,
 			     int unset)
 {
+	struct format_config *cfg = opt->value;
 	BUG_ON_OPT_ARG(arg);
-	*(int *)opt->value = numbered_cmdline_opt = unset ? 0 : 1;
+	cfg->numbered = cfg->numbered_cmdline_opt = unset ? 0 : 1;
 	if (unset)
-		auto_number =  0;
+		cfg->auto_number =  0;
 	return 0;
 }
 
@@ -1588,13 +1635,14 @@ static int output_directory_callback(const struct option *opt, const char *arg,
 
 static int thread_callback(const struct option *opt, const char *arg, int unset)
 {
-	enum thread_level *thread = (enum thread_level *)opt->value;
+	struct format_config *cfg = opt->value;
+
 	if (unset)
-		*thread = THREAD_UNSET;
+		cfg->thread = THREAD_UNSET;
 	else if (!arg || !strcmp(arg, "shallow"))
-		*thread = THREAD_SHALLOW;
+		cfg->thread = THREAD_SHALLOW;
 	else if (!strcmp(arg, "deep"))
-		*thread = THREAD_DEEP;
+		cfg->thread = THREAD_DEEP;
 	/*
 	 * Please update _git_formatpatch() in git-completion.bash
 	 * when you add new options.
@@ -1630,15 +1678,17 @@ static int inline_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-static int header_callback(const struct option *opt UNUSED, const char *arg,
+static int header_callback(const struct option *opt, const char *arg,
 			   int unset)
 {
+	struct format_config *cfg = opt->value;
+
 	if (unset) {
-		string_list_clear(&extra_hdr, 0);
-		string_list_clear(&extra_to, 0);
-		string_list_clear(&extra_cc, 0);
+		string_list_clear(&cfg->extra_hdr, 0);
+		string_list_clear(&cfg->extra_to, 0);
+		string_list_clear(&cfg->extra_cc, 0);
 	} else {
-		add_header(arg);
+		add_header(cfg, arg);
 	}
 	return 0;
 }
@@ -1660,17 +1710,17 @@ static int from_callback(const struct option *opt, const char *arg, int unset)
 
 static int base_callback(const struct option *opt, const char *arg, int unset)
 {
-	const char **base_commit = opt->value;
+	struct format_config *cfg = opt->value;
 
 	if (unset) {
-		auto_base = AUTO_BASE_NEVER;
-		*base_commit = NULL;
+		cfg->auto_base = AUTO_BASE_NEVER;
+		FREE_AND_NULL(cfg->base_commit);
 	} else if (!strcmp(arg, "auto")) {
-		auto_base = AUTO_BASE_ALWAYS;
-		*base_commit = NULL;
+		cfg->auto_base = AUTO_BASE_ALWAYS;
+		FREE_AND_NULL(cfg->base_commit);
 	} else {
-		auto_base = AUTO_BASE_NEVER;
-		*base_commit = arg;
+		cfg->auto_base = AUTO_BASE_NEVER;
+		cfg->base_commit = xstrdup(arg);
 	}
 	return 0;
 }
@@ -1681,7 +1731,7 @@ struct base_tree_info {
 	struct object_id *patch_id;
 };
 
-static struct commit *get_base_commit(const char *base_commit,
+static struct commit *get_base_commit(const struct format_config *cfg,
 				      struct commit **list,
 				      int total)
 {
@@ -1689,9 +1739,9 @@ static struct commit *get_base_commit(const char *base_commit,
 	struct commit **rev;
 	int i = 0, rev_nr = 0, auto_select, die_on_failure, ret;
 
-	switch (auto_base) {
+	switch (cfg->auto_base) {
 	case AUTO_BASE_NEVER:
-		if (base_commit) {
+		if (cfg->base_commit) {
 			auto_select = 0;
 			die_on_failure = 1;
 		} else {
@@ -1701,11 +1751,11 @@ static struct commit *get_base_commit(const char *base_commit,
 		break;
 	case AUTO_BASE_ALWAYS:
 	case AUTO_BASE_WHEN_ABLE:
-		if (base_commit) {
+		if (cfg->base_commit) {
 			BUG("requested automatic base selection but a commit was provided");
 		} else {
 			auto_select = 1;
-			die_on_failure = auto_base == AUTO_BASE_ALWAYS;
+			die_on_failure = cfg->auto_base == AUTO_BASE_ALWAYS;
 		}
 		break;
 	default:
@@ -1713,9 +1763,9 @@ static struct commit *get_base_commit(const char *base_commit,
 	}
 
 	if (!auto_select) {
-		base = lookup_commit_reference_by_name(base_commit);
+		base = lookup_commit_reference_by_name(cfg->base_commit);
 		if (!base)
-			die(_("unknown commit %s"), base_commit);
+			die(_("unknown commit %s"), cfg->base_commit);
 	} else {
 		struct branch *curr_branch = branch_get(NULL);
 		const char *upstream = branch_get_upstream(curr_branch, NULL);
@@ -1933,7 +1983,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,
 
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
-	struct log_config cfg;
+	struct format_config cfg;
 	struct commit *commit;
 	struct commit **list = NULL;
 	struct rev_info rev;
@@ -1958,7 +2008,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	char *cover_from_description_arg = NULL;
 	char *description_file = NULL;
 	char *branch_name = NULL;
-	char *base_commit = NULL;
 	struct base_tree_info bases;
 	struct commit *base;
 	int show_progress = 0;
@@ -1969,18 +2018,24 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff1 = STRBUF_INIT;
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
-	struct strbuf sprefix = STRBUF_INIT;
 	const char *rfc = NULL;
 	int creation_factor = -1;
+	const char *signature = git_version_string;
+	const char *signature_file_arg = NULL;
+	struct keep_callback_data keep_callback_data = {
+		.cfg = &cfg,
+		.revs = &rev,
+	};
+	const char *fmt_patch_suffix = NULL;
 
 	const struct option builtin_format_patch_options[] = {
-		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
+		OPT_CALLBACK_F('n', "numbered", &cfg, NULL,
 			    N_("use [PATCH n/m] even with a single patch"),
 			    PARSE_OPT_NOARG, numbered_callback),
-		OPT_CALLBACK_F('N', "no-numbered", &numbered, NULL,
+		OPT_CALLBACK_F('N', "no-numbered", &cfg, NULL,
 			    N_("use [PATCH] even with multiple patches"),
 			    PARSE_OPT_NOARG | PARSE_OPT_NONEG, no_numbered_callback),
-		OPT_BOOL('s', "signoff", &do_signoff, N_("add a Signed-off-by trailer")),
+		OPT_BOOL('s', "signoff", &cfg.do_signoff, N_("add a Signed-off-by trailer")),
 		OPT_BOOL(0, "stdout", &use_stdout,
 			    N_("print patches to standard out")),
 		OPT_BOOL(0, "cover-letter", &cover_letter,
@@ -1993,7 +2048,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    N_("start numbering patches at <n> instead of 1")),
 		OPT_STRING('v', "reroll-count", &reroll_count, N_("reroll-count"),
 			    N_("mark the series as Nth re-roll")),
-		OPT_INTEGER(0, "filename-max-length", &cfg.fmt_patch_name_max,
+		OPT_INTEGER(0, "filename-max-length", &cfg.log.fmt_patch_name_max,
 			    N_("max length of output filename")),
 		OPT_CALLBACK_F(0, "rfc", &rfc, N_("rfc"),
 			       N_("add <rfc> (default 'RFC') before 'PATCH'"),
@@ -2003,13 +2058,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    N_("generate parts of a cover letter based on a branch's description")),
 		OPT_FILENAME(0, "description-file", &description_file,
 			     N_("use branch description from file")),
-		OPT_CALLBACK_F(0, "subject-prefix", &sprefix, N_("prefix"),
+		OPT_CALLBACK_F(0, "subject-prefix", &cfg, N_("prefix"),
 			    N_("use [<prefix>] instead of [PATCH]"),
 			    PARSE_OPT_NONEG, subject_prefix_callback),
 		OPT_CALLBACK_F('o', "output-directory", &output_directory,
 			    N_("dir"), N_("store resulting files in <dir>"),
 			    PARSE_OPT_NONEG, output_directory_callback),
-		OPT_CALLBACK_F('k', "keep-subject", &rev, NULL,
+		OPT_CALLBACK_F('k', "keep-subject", &keep_callback_data, NULL,
 			    N_("don't strip/add [PATCH]"),
 			    PARSE_OPT_NOARG | PARSE_OPT_NONEG, keep_callback),
 		OPT_BOOL(0, "no-binary", &no_binary_diff,
@@ -2022,11 +2077,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			      N_("show patch format instead of default (patch + stat)"),
 			      1, PARSE_OPT_NONEG),
 		OPT_GROUP(N_("Messaging")),
-		OPT_CALLBACK(0, "add-header", NULL, N_("header"),
+		OPT_CALLBACK(0, "add-header", &cfg, N_("header"),
 			    N_("add email header"), header_callback),
-		OPT_STRING_LIST(0, "to", &extra_to, N_("email"), N_("add To: header")),
-		OPT_STRING_LIST(0, "cc", &extra_cc, N_("email"), N_("add Cc: header")),
-		OPT_CALLBACK_F(0, "from", &from, N_("ident"),
+		OPT_STRING_LIST(0, "to", &cfg.extra_to, N_("email"), N_("add To: header")),
+		OPT_STRING_LIST(0, "cc", &cfg.extra_cc, N_("email"), N_("add Cc: header")),
+		OPT_CALLBACK_F(0, "from", &cfg.from, N_("ident"),
 			    N_("set From address to <ident> (or committer ident if absent)"),
 			    PARSE_OPT_OPTARG, from_callback),
 		OPT_STRING(0, "in-reply-to", &in_reply_to, N_("message-id"),
@@ -2038,15 +2093,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    N_("inline the patch"),
 			    PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
 			    inline_callback),
-		OPT_CALLBACK_F(0, "thread", &thread, N_("style"),
+		OPT_CALLBACK_F(0, "thread", &cfg, N_("style"),
 			    N_("enable message threading, styles: shallow, deep"),
 			    PARSE_OPT_OPTARG, thread_callback),
 		OPT_STRING(0, "signature", &signature, N_("signature"),
 			    N_("add a signature")),
-		OPT_CALLBACK_F(0, "base", &base_commit, N_("base-commit"),
+		OPT_CALLBACK_F(0, "base", &cfg, N_("base-commit"),
 			       N_("add prerequisite tree info to the patch series"),
 			       0, base_callback),
-		OPT_FILENAME(0, "signature-file", &signature_file,
+		OPT_FILENAME(0, "signature-file", &signature_file_arg,
 				N_("add a signature from a file")),
 		OPT__QUIET(&quiet, N_("don't print the patch filenames")),
 		OPT_BOOL(0, "progress", &show_progress,
@@ -2063,21 +2118,17 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	extra_hdr.strdup_strings = 1;
-	extra_to.strdup_strings = 1;
-	extra_cc.strdup_strings = 1;
-
-	log_config_init(&cfg);
+	format_config_init(&cfg);
 	init_diff_ui_defaults();
-	init_display_notes(&notes_opt);
+	init_display_notes(&cfg.notes_opt);
 	git_config(git_format_config, &cfg);
 	repo_init_revisions(the_repository, &rev, prefix);
 	git_config(grep_config, &rev.grep_filter);
 
-	rev.show_notes = show_notes;
-	memcpy(&rev.notes_opt, &notes_opt, sizeof(notes_opt));
+	rev.show_notes = cfg.show_notes;
+	memcpy(&rev.notes_opt, &cfg.notes_opt, sizeof(cfg.notes_opt));
 	rev.commit_format = CMIT_FMT_EMAIL;
-	rev.encode_email_headers = cfg.default_encode_email_headers;
+	rev.encode_email_headers = cfg.log.default_encode_email_headers;
 	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
 	rev.diff = 1;
@@ -2088,12 +2139,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	s_r_opt.def = "HEAD";
 	s_r_opt.revarg_opt = REVARG_COMMITTISH;
 
-	strbuf_addstr(&sprefix, cfg.fmt_patch_subject_prefix);
+	strbuf_addstr(&cfg.sprefix, cfg.log.fmt_patch_subject_prefix);
 	if (format_no_prefix)
 		diff_set_noprefix(&rev.diffopt);
 
-	if (default_attach) {
-		rev.mime_boundary = default_attach;
+	if (cfg.default_attach) {
+		rev.mime_boundary = cfg.default_attach;
 		rev.no_inline = 1;
 	}
 
@@ -2109,60 +2160,63 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	rev.force_in_body_from = force_in_body_from;
 
+	if (!fmt_patch_suffix)
+		fmt_patch_suffix = cfg.fmt_patch_suffix;
+
 	/* Make sure "0000-$sub.patch" gives non-negative length for $sub */
-	if (cfg.fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
-		cfg.fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
+	if (cfg.log.fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
+		cfg.log.fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
 
 	if (cover_from_description_arg)
-		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
+		cfg.cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
 
 	if (rfc && rfc[0]) {
-		subject_prefix = 1;
+		cfg.subject_prefix = 1;
 		if (rfc[0] == '-')
-			strbuf_addf(&sprefix, " %s", rfc + 1);
+			strbuf_addf(&cfg.sprefix, " %s", rfc + 1);
 		else
-			strbuf_insertf(&sprefix, 0, "%s ", rfc);
+			strbuf_insertf(&cfg.sprefix, 0, "%s ", rfc);
 	}
 
 	if (reroll_count) {
-		strbuf_addf(&sprefix, " v%s", reroll_count);
+		strbuf_addf(&cfg.sprefix, " v%s", reroll_count);
 		rev.reroll_count = reroll_count;
 	}
 
-	rev.subject_prefix = sprefix.buf;
+	rev.subject_prefix = cfg.sprefix.buf;
 
-	for (i = 0; i < extra_hdr.nr; i++) {
-		strbuf_addstr(&buf, extra_hdr.items[i].string);
+	for (i = 0; i < cfg.extra_hdr.nr; i++) {
+		strbuf_addstr(&buf, cfg.extra_hdr.items[i].string);
 		strbuf_addch(&buf, '\n');
 	}
 
-	if (extra_to.nr)
+	if (cfg.extra_to.nr)
 		strbuf_addstr(&buf, "To: ");
-	for (i = 0; i < extra_to.nr; i++) {
+	for (i = 0; i < cfg.extra_to.nr; i++) {
 		if (i)
 			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_to.items[i].string);
-		if (i + 1 < extra_to.nr)
+		strbuf_addstr(&buf, cfg.extra_to.items[i].string);
+		if (i + 1 < cfg.extra_to.nr)
 			strbuf_addch(&buf, ',');
 		strbuf_addch(&buf, '\n');
 	}
 
-	if (extra_cc.nr)
+	if (cfg.extra_cc.nr)
 		strbuf_addstr(&buf, "Cc: ");
-	for (i = 0; i < extra_cc.nr; i++) {
+	for (i = 0; i < cfg.extra_cc.nr; i++) {
 		if (i)
 			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_cc.items[i].string);
-		if (i + 1 < extra_cc.nr)
+		strbuf_addstr(&buf, cfg.extra_cc.items[i].string);
+		if (i + 1 < cfg.extra_cc.nr)
 			strbuf_addch(&buf, ',');
 		strbuf_addch(&buf, '\n');
 	}
 
 	rev.extra_headers = to_free = strbuf_detach(&buf, NULL);
 
-	if (from) {
-		if (split_ident_line(&rev.from_ident, from, strlen(from)))
-			die(_("invalid ident line: %s"), from);
+	if (cfg.from) {
+		if (split_ident_line(&rev.from_ident, cfg.from, strlen(cfg.from)))
+			die(_("invalid ident line: %s"), cfg.from);
 	}
 
 	if (start_number < 0)
@@ -2173,14 +2227,14 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	 * and it would conflict with --keep-subject (-k) from the
 	 * command line, reset "numbered".
 	 */
-	if (numbered && keep_subject && !numbered_cmdline_opt)
-		numbered = 0;
+	if (cfg.numbered && cfg.keep_subject && !cfg.numbered_cmdline_opt)
+		cfg.numbered = 0;
 
-	if (numbered && keep_subject)
+	if (cfg.numbered && cfg.keep_subject)
 		die(_("options '%s' and '%s' cannot be used together"), "-n", "-k");
-	if (keep_subject && subject_prefix)
+	if (cfg.keep_subject && cfg.subject_prefix)
 		die(_("options '%s' and '%s' cannot be used together"), "--subject-prefix/--rfc", "-k");
-	rev.preserve_subject = keep_subject;
+	rev.preserve_subject = cfg.keep_subject;
 
 	argc = setup_revisions(argc, argv, &rev, &s_r_opt);
 	if (argc > 1)
@@ -2207,7 +2261,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.always_show_header = 1;
 
 	rev.zero_commit = zero_commit;
-	rev.patch_name_max = cfg.fmt_patch_name_max;
+	rev.patch_name_max = cfg.log.fmt_patch_name_max;
 
 	if (!rev.diffopt.flags.text && !no_binary_diff)
 		rev.diffopt.flags.binary = 1;
@@ -2228,7 +2282,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		int saved;
 
 		if (!output_directory)
-			output_directory = config_output_directory;
+			output_directory = cfg.config_output_directory;
 		output_directory = set_outdir(prefix, output_directory);
 
 		if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
@@ -2326,14 +2380,14 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		goto done;
 	total = nr;
 	if (cover_letter == -1) {
-		if (config_cover_letter == COVER_AUTO)
+		if (cfg.config_cover_letter == COVER_AUTO)
 			cover_letter = (total > 1);
 		else
-			cover_letter = (config_cover_letter == COVER_ON);
+			cover_letter = (cfg.config_cover_letter == COVER_ON);
 	}
-	if (!keep_subject && auto_number && (total > 1 || cover_letter))
-		numbered = 1;
-	if (numbered)
+	if (!cfg.keep_subject && cfg.auto_number && (total > 1 || cover_letter))
+		cfg.numbered = 1;
+	if (cfg.numbered)
 		rev.total = total + start_number - 1;
 
 	if (idiff_prev.nr) {
@@ -2365,27 +2419,40 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 					     _("Range-diff against v%d:"));
 	}
 
+	/*
+	 * The order of precedence is:
+	 *
+	 *   1. The `--signature` and `--no-signature` options.
+	 *   2. The `--signature-file` option.
+	 *   3. The `format.signature` config.
+	 *   4. The `format.signatureFile` config.
+	 *   5. Default `git_version_string`.
+	 */
 	if (!signature) {
 		; /* --no-signature inhibits all signatures */
 	} else if (signature && signature != git_version_string) {
 		; /* non-default signature already set */
-	} else if (signature_file) {
+	} else if (signature_file_arg || (cfg.signature_file && !cfg.signature)) {
 		struct strbuf buf = STRBUF_INIT;
+		const char *signature_file = signature_file_arg ?
+			signature_file_arg : cfg.signature_file;
 
 		if (strbuf_read_file(&buf, signature_file, 128) < 0)
 			die_errno(_("unable to read signature file '%s'"), signature_file);
 		signature = strbuf_detach(&buf, NULL);
+	} else if (cfg.signature) {
+		signature = cfg.signature;
 	}
 
 	memset(&bases, 0, sizeof(bases));
-	base = get_base_commit(base_commit, list, nr);
+	base = get_base_commit(&cfg, list, nr);
 	if (base) {
 		reset_revision_walk();
 		clear_object_flags(UNINTERESTING);
 		prepare_bases(&bases, base, list, nr);
 	}
 
-	if (in_reply_to || thread || cover_letter) {
+	if (in_reply_to || cfg.thread || cover_letter) {
 		rev.ref_message_ids = xmalloc(sizeof(*rev.ref_message_ids));
 		string_list_init_dup(rev.ref_message_ids);
 	}
@@ -2396,19 +2463,19 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.numbered_files = just_numbers;
 	rev.patch_suffix = fmt_patch_suffix;
 	if (cover_letter) {
-		if (thread)
+		if (cfg.thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, !!output_directory,
-				  origin, nr, list, description_file, branch_name, quiet);
+				  origin, nr, list, description_file, branch_name, quiet, &cfg);
 		print_bases(&bases, rev.diffopt.file);
-		print_signature(rev.diffopt.file);
+		print_signature(signature, rev.diffopt.file);
 		total++;
 		start_number--;
 		/* interdiff/range-diff in cover-letter; omit from patches */
 		rev.idiff_oid1 = NULL;
 		rev.rdiff1 = NULL;
 	}
-	rev.add_signoff = do_signoff;
+	rev.add_signoff = cfg.do_signoff;
 
 	if (show_progress)
 		progress = start_delayed_progress(_("Generating patches"), total);
@@ -2418,7 +2485,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		commit = list[nr];
 		rev.nr = total - nr + (start_number - 1);
 		/* Make the second and subsequent mails replies to the first */
-		if (thread) {
+		if (cfg.thread) {
 			/* Have we already had a message ID? */
 			if (rev.message_id) {
 				/*
@@ -2442,7 +2509,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				 * letter is a reply to the
 				 * --in-reply-to, if specified.
 				 */
-				if (thread == THREAD_SHALLOW
+				if (cfg.thread == THREAD_SHALLOW
 				    && rev.ref_message_ids->nr > 0
 				    && (!cover_letter || rev.nr > 1))
 					free(rev.message_id);
@@ -2475,7 +2542,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				       mime_boundary_leader,
 				       rev.mime_boundary);
 			else
-				print_signature(rev.diffopt.file);
+				print_signature(signature, rev.diffopt.file);
 		}
 		if (output_directory)
 			fclose(rev.diffopt.file);
@@ -2483,9 +2550,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	stop_progress(&progress);
 	free(list);
 	free(branch_name);
-	string_list_clear(&extra_to, 0);
-	string_list_clear(&extra_cc, 0);
-	string_list_clear(&extra_hdr, 0);
 	if (ignore_if_in_upstream)
 		free_patch_ids(&ids);
 
@@ -2495,14 +2559,13 @@ done:
 	strbuf_release(&rdiff1);
 	strbuf_release(&rdiff2);
 	strbuf_release(&rdiff_title);
-	strbuf_release(&sprefix);
 	free(to_free);
 	free(rev.message_id);
 	if (rev.ref_message_ids)
 		string_list_clear(rev.ref_message_ids, 0);
 	free(rev.ref_message_ids);
 	release_revisions(&rev);
-	log_config_release(&cfg);
+	format_config_release(&cfg);
 	return 0;
 }
 
-- 
cgit v1.3-5-g9baa


From 1b261c20ed28ad26ddbcd3dff94a248ac6866ac8 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 27 May 2024 13:46:39 +0200
Subject: config: clarify memory ownership in `git_config_string()`

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 <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 alias.c                |  2 +-
 attr.c                 |  2 +-
 attr.h                 |  2 +-
 builtin/commit.c       |  2 +-
 builtin/log.c          | 12 ++++++------
 builtin/merge.c        |  4 ++--
 builtin/rebase.c       |  2 +-
 builtin/receive-pack.c |  2 +-
 builtin/repack.c       |  8 ++++----
 config.c               |  6 +++---
 config.h               |  2 +-
 convert.c              |  6 +++---
 delta-islands.c        |  2 +-
 diff.c                 |  8 ++++----
 environment.c          |  8 ++++----
 environment.h          |  8 ++++----
 gpg-interface.c        |  4 ++--
 http.c                 | 24 ++++++++++++------------
 imap-send.c            | 12 ++++++------
 mailmap.c              |  2 +-
 mailmap.h              |  2 +-
 merge-ll.c             |  6 +++---
 pager.c                |  2 +-
 pretty.c               | 14 +++++++++-----
 promisor-remote.h      |  2 +-
 remote.c               | 20 ++++++++++----------
 remote.h               |  8 ++++----
 sequencer.c            |  2 +-
 upload-pack.c          |  2 +-
 userdiff.h             | 12 ++++++------
 30 files changed, 96 insertions(+), 92 deletions(-)

(limited to 'builtin/log.c')

diff --git a/alias.c b/alias.c
index 5a238f2e30..269892c356 100644
--- 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
index 33473bdce0..5607db2bbe 100644
--- 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
index 127998ae01..e7cc318b0c 100644
--- 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
index 1cc88e92bf..f53e7e86ff 100644
--- 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
index 890bf0c425..517d7982f1 100644
--- 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
index e4bd65eeba..daed2d4e1e 100644
--- 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
index 0466d9414a..14d4f0a5e6 100644
--- 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
index 56228ad314..01c1f04ece 100644
--- 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
index 43491a4cbf..e40dceaada 100644
--- 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
index f9101045ee..da52a7f8a1 100644
--- 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
index b3103bba94..2d2e22ed64 100644
--- 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
index 03c3c528f9..f2b9f01354 100644
--- a/convert.c
+++ b/convert.c
@@ -981,9 +981,9 @@ done:
 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
index 4ac3c10551..89d51b72e3 100644
--- 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
index 679ef472f4..e70301df76 100644
--- 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
index ab6956559e..701d515135 100644
--- 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
index be1b88ad6f..e9f01d4d11 100644
--- 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
index 2b50ed0fa0..5193223714 100644
--- 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
index fa3ea87451..67cc47d28f 100644
--- 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
index c0130d0e02..a5d1510180 100644
--- 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
index 044466b043..b2efe29b3d 100644
--- 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
index 429a760945..cbda9bc5e0 100644
--- 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
index bf1077ae09..e29b15fa4a 100644
--- a/merge-ll.c
+++ b/merge-ll.c
@@ -27,9 +27,9 @@ typedef enum ll_merge_result (*ll_merge_fn)(const struct ll_merge_driver *,
 
 struct ll_merge_driver {
 	const char *name;
-	const char *description;
+	char *description;
 	ll_merge_fn fn;
-	const char *recursive;
+	char *recursive;
 	struct ll_merge_driver *next;
 	char *cmdline;
 };
@@ -268,7 +268,7 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
  * merge.default and merge.driver configuration items
  */
 static struct ll_merge_driver *ll_user_merge, **ll_user_merge_tail;
-static const char *default_ll_merge;
+static char *default_ll_merge;
 
 static int read_merge_config(const char *var, const char *value,
 			     const struct config_context *ctx UNUSED,
diff --git a/pager.c b/pager.c
index b8822a9381..e9e121db69 100644
--- a/pager.c
+++ b/pager.c
@@ -13,7 +13,7 @@ int pager_use_color = 1;
 #endif
 
 static struct child_process pager_process;
-static const char *pager_program;
+static char *pager_program;
 
 /* Is the value coming back from term_columns() just a guess? */
 static int term_columns_guessed;
diff --git a/pretty.c b/pretty.c
index 7ead078998..22a81506b7 100644
--- a/pretty.c
+++ b/pretty.c
@@ -62,7 +62,7 @@ static int git_pretty_formats_config(const char *var, const char *value,
 {
 	struct cmt_fmt_map *commit_format = NULL;
 	const char *name;
-	const char *fmt;
+	char *fmt;
 	int i;
 
 	if (!skip_prefix(var, "pretty.", &name))
@@ -93,13 +93,17 @@ static int git_pretty_formats_config(const char *var, const char *value,
 	if (git_config_string(&fmt, var, value))
 		return -1;
 
-	if (skip_prefix(fmt, "format:", &fmt))
+	if (skip_prefix(fmt, "format:", &commit_format->user_format)) {
 		commit_format->is_tformat = 0;
-	else if (skip_prefix(fmt, "tformat:", &fmt) || strchr(fmt, '%'))
+	} else if (skip_prefix(fmt, "tformat:", &commit_format->user_format)) {
 		commit_format->is_tformat = 1;
-	else
+	} else if (strchr(fmt, '%')) {
+		commit_format->is_tformat = 1;
+		commit_format->user_format = fmt;
+	} else {
 		commit_format->is_alias = 1;
-	commit_format->user_format = fmt;
+		commit_format->user_format = fmt;
+	}
 
 	return 0;
 }
diff --git a/promisor-remote.h b/promisor-remote.h
index 2cb9eda9ea..88cb599c39 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -13,7 +13,7 @@ struct object_id;
  */
 struct promisor_remote {
 	struct promisor_remote *next;
-	const char *partial_clone_filter;
+	char *partial_clone_filter;
 	const char name[FLEX_ARRAY];
 };
 
diff --git a/remote.c b/remote.c
index ec8c158e60..d319f28757 100644
--- a/remote.c
+++ b/remote.c
@@ -428,29 +428,29 @@ static int handle_config(const char *key, const char *value,
 	else if (!strcmp(subkey, "prunetags"))
 		remote->prune_tags = git_config_bool(key, value);
 	else if (!strcmp(subkey, "url")) {
-		const char *v;
+		char *v;
 		if (git_config_string(&v, key, value))
 			return -1;
 		add_url(remote, v);
 	} else if (!strcmp(subkey, "pushurl")) {
-		const char *v;
+		char *v;
 		if (git_config_string(&v, key, value))
 			return -1;
 		add_pushurl(remote, v);
 	} else if (!strcmp(subkey, "push")) {
-		const char *v;
+		char *v;
 		if (git_config_string(&v, key, value))
 			return -1;
 		refspec_append(&remote->push, v);
-		free((char *)v);
+		free(v);
 	} else if (!strcmp(subkey, "fetch")) {
-		const char *v;
+		char *v;
 		if (git_config_string(&v, key, value))
 			return -1;
 		refspec_append(&remote->fetch, v);
-		free((char *)v);
+		free(v);
 	} else if (!strcmp(subkey, "receivepack")) {
-		const char *v;
+		char *v;
 		if (git_config_string(&v, key, value))
 			return -1;
 		if (!remote->receivepack)
@@ -458,7 +458,7 @@ static int handle_config(const char *key, const char *value,
 		else
 			error(_("more than one receivepack given, using the first"));
 	} else if (!strcmp(subkey, "uploadpack")) {
-		const char *v;
+		char *v;
 		if (git_config_string(&v, key, value))
 			return -1;
 		if (!remote->uploadpack)
@@ -471,10 +471,10 @@ static int handle_config(const char *key, const char *value,
 		else if (!strcmp(value, "--tags"))
 			remote->fetch_tags = 2;
 	} else if (!strcmp(subkey, "proxy")) {
-		return git_config_string((const char **)&remote->http_proxy,
+		return git_config_string(&remote->http_proxy,
 					 key, value);
 	} else if (!strcmp(subkey, "proxyauthmethod")) {
-		return git_config_string((const char **)&remote->http_proxy_authmethod,
+		return git_config_string(&remote->http_proxy_authmethod,
 					 key, value);
 	} else if (!strcmp(subkey, "vcs")) {
 		return git_config_string(&remote->foreign_vcs, key, value);
diff --git a/remote.h b/remote.h
index dfd4837e60..e8c6655e42 100644
--- a/remote.h
+++ b/remote.h
@@ -46,7 +46,7 @@ struct remote_state {
 	struct hashmap branches_hash;
 
 	struct branch *current_branch;
-	const char *pushremote_name;
+	char *pushremote_name;
 
 	struct rewrites rewrites;
 	struct rewrites rewrites_push;
@@ -65,7 +65,7 @@ struct remote {
 
 	int origin, configured_in_repo;
 
-	const char *foreign_vcs;
+	char *foreign_vcs;
 
 	/* An array of all of the url_nr URLs configured for the remote */
 	const char **url;
@@ -309,9 +309,9 @@ struct branch {
 	const char *refname;
 
 	/* The name of the remote listed in the configuration. */
-	const char *remote_name;
+	char *remote_name;
 
-	const char *pushremote_name;
+	char *pushremote_name;
 
 	/* An array of the "merge" lines in the configuration. */
 	const char **merge_name;
diff --git a/sequencer.c b/sequencer.c
index dbd60d79b9..3c81ef9ca5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -306,7 +306,7 @@ static int git_sequencer_config(const char *k, const char *v,
 	}
 
 	if (!opts->default_strategy && !strcmp(k, "pull.twohead")) {
-		int ret = git_config_string((const char**)&opts->default_strategy, k, v);
+		int ret = git_config_string(&opts->default_strategy, k, v);
 		if (ret == 0) {
 			/*
 			 * pull.twohead is allowed to be multi-valued; we only
diff --git a/upload-pack.c b/upload-pack.c
index 8fbd138515..5801eb2639 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -94,7 +94,7 @@ struct upload_pack_data {
 
 	struct packet_writer writer;
 
-	const char *pack_objects_hook;
+	char *pack_objects_hook;
 
 	unsigned stateless_rpc : 1;				/* v0 only */
 	unsigned no_done : 1;					/* v0 only */
diff --git a/userdiff.h b/userdiff.h
index d726804c3e..cc8e5abfef 100644
--- a/userdiff.h
+++ b/userdiff.h
@@ -7,19 +7,19 @@ struct index_state;
 struct repository;
 
 struct userdiff_funcname {
-	const char *pattern;
+	char *pattern;
 	int cflags;
 };
 
 struct userdiff_driver {
 	const char *name;
-	const char *external;
-	const char *algorithm;
+	char *external;
+	char *algorithm;
 	int binary;
 	struct userdiff_funcname funcname;
-	const char *word_regex;
-	const char *word_regex_multi_byte;
-	const char *textconv;
+	char *word_regex;
+	char *word_regex_multi_byte;
+	char *textconv;
 	struct notes_cache *textconv_cache;
 	int textconv_want_cache;
 };
-- 
cgit v1.3-5-g9baa