From b567004b4b43f9b0d88aa1f0b15698eae8f15836 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:37:39 +0200
Subject: [PATCH 01/27] global: improve const correctness when assigning string
 constants

We're about to enable `-Wwrite-strings`, which changes the type of
string constants to `const char[]`. Fix various sites where we assign
such constants to non-const variables.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/bisect.c             |  3 ++-
 builtin/blame.c              |  2 +-
 builtin/bugreport.c          |  2 +-
 builtin/check-ignore.c       |  4 +--
 builtin/clone.c              |  6 ++---
 builtin/commit.c             |  6 ++---
 builtin/diagnose.c           |  2 +-
 builtin/log.c                |  2 +-
 builtin/mailsplit.c          |  4 +--
 builtin/pull.c               | 52 ++++++++++++++++++------------------
 builtin/receive-pack.c       |  4 +--
 builtin/revert.c             |  2 +-
 compat/regex/regcomp.c       |  2 +-
 diff.c                       |  4 +--
 diffcore-rename.c            |  6 ++---
 fmt-merge-msg.c              |  2 +-
 fsck.c                       |  2 +-
 fsck.h                       |  2 +-
 gpg-interface.c              |  2 +-
 http-backend.c               |  2 +-
 imap-send.c                  |  6 ++---
 pretty.c                     |  2 +-
 refs.c                       |  2 +-
 refs.h                       |  2 +-
 reftable/basics.c            | 15 +++++------
 reftable/basics.h            |  4 +--
 reftable/basics_test.c       |  4 +--
 reftable/record.c            |  6 ++---
 reftable/stack.c             | 10 ++++---
 reftable/stack_test.c        |  8 +++---
 run-command.c                |  2 +-
 t/helper/test-hashmap.c      |  3 ++-
 t/helper/test-json-writer.c  | 10 +++----
 t/helper/test-regex.c        |  4 +--
 t/helper/test-rot13-filter.c |  5 ++--
 t/unit-tests/t-strbuf.c      | 10 ++++---
 trailer.c                    |  2 +-
 wt-status.c                  |  2 +-
 38 files changed, 106 insertions(+), 102 deletions(-)

diff --git a/builtin/bisect.c b/builtin/bisect.c
index a58432b9d9..dabce9b542 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -262,7 +262,8 @@ static int bisect_reset(const char *commit)
 	return bisect_clean_state();
 }
 
-static void log_commit(FILE *fp, char *fmt, const char *state,
+static void log_commit(FILE *fp,
+		       const char *fmt, const char *state,
 		       struct commit *commit)
 {
 	struct pretty_print_context pp = {0};
diff --git a/builtin/blame.c b/builtin/blame.c
index 838cd476be..98c7629b6a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -134,7 +134,7 @@ static void get_ac_line(const char *inbuf, const char *what,
 {
 	struct ident_split ident;
 	size_t len, maillen, namelen;
-	char *tmp, *endp;
+	const char *tmp, *endp;
 	const char *namebuf, *mailbuf;
 
 	tmp = strstr(inbuf, what);
diff --git a/builtin/bugreport.c b/builtin/bugreport.c
index 25f860a0d9..b3cc77af53 100644
--- a/builtin/bugreport.c
+++ b/builtin/bugreport.c
@@ -107,7 +107,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
 	struct tm tm;
 	enum diagnose_mode diagnose = DIAGNOSE_NONE;
 	char *option_output = NULL;
-	char *option_suffix = "%Y-%m-%d-%H%M";
+	const char *option_suffix = "%Y-%m-%d-%H%M";
 	const char *user_relative_path = NULL;
 	char *prefixed_filename;
 	size_t output_path_len;
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 6c43430ec4..2bda6a1d46 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -35,8 +35,8 @@ static const struct option check_ignore_options[] = {
 
 static void output_pattern(const char *path, struct path_pattern *pattern)
 {
-	char *bang  = (pattern && pattern->flags & PATTERN_FLAG_NEGATIVE)  ? "!" : "";
-	char *slash = (pattern && pattern->flags & PATTERN_FLAG_MUSTBEDIR) ? "/" : "";
+	const char *bang  = (pattern && pattern->flags & PATTERN_FLAG_NEGATIVE)  ? "!" : "";
+	const char *slash = (pattern && pattern->flags & PATTERN_FLAG_MUSTBEDIR) ? "/" : "";
 	if (!nul_term_line) {
 		if (!verbose) {
 			write_name_quoted(path, stdout, '\n');
diff --git a/builtin/clone.c b/builtin/clone.c
index 23993b905b..92ab7d7165 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -71,7 +71,7 @@ static char *option_branch = NULL;
 static struct string_list option_not = STRING_LIST_INIT_NODUP;
 static const char *real_git_dir;
 static const char *ref_format;
-static char *option_upload_pack = "git-upload-pack";
+static const char *option_upload_pack = "git-upload-pack";
 static int option_verbosity;
 static int option_progress = -1;
 static int option_sparse_checkout;
@@ -177,8 +177,8 @@ static struct option builtin_clone_options[] = {
 
 static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
 {
-	static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
-	static char *bundle_suffix[] = { ".bundle", "" };
+	static const char *suffix[] = { "/.git", "", ".git/.git", ".git" };
+	static const char *bundle_suffix[] = { ".bundle", "" };
 	size_t baselen = path->len;
 	struct stat st;
 	int i;
diff --git a/builtin/commit.c b/builtin/commit.c
index f53e7e86ff..75c741173e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -113,7 +113,7 @@ static char *template_file;
  * the commit message and/or authorship.
  */
 static const char *author_message, *author_message_buffer;
-static char *edit_message, *use_message;
+static const char *edit_message, *use_message;
 static char *fixup_message, *fixup_commit, *squash_message;
 static const char *fixup_prefix;
 static int all, also, interactive, patch_interactive, only, amend, signoff;
@@ -121,8 +121,8 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
-static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
-static char *sign_commit, *pathspec_from_file;
+static const char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
+static const char *sign_commit, *pathspec_from_file;
 static struct strvec trailer_args = STRVEC_INIT;
 
 /*
diff --git a/builtin/diagnose.c b/builtin/diagnose.c
index 4f22eb2b55..4857a4395b 100644
--- a/builtin/diagnose.c
+++ b/builtin/diagnose.c
@@ -18,7 +18,7 @@ int cmd_diagnose(int argc, const char **argv, const char *prefix)
 	struct tm tm;
 	enum diagnose_mode mode = DIAGNOSE_STATS;
 	char *option_output = NULL;
-	char *option_suffix = "%Y-%m-%d-%H%M";
+	const char *option_suffix = "%Y-%m-%d-%H%M";
 	char *prefixed_filename;
 
 	const struct option diagnose_options[] = {
diff --git a/builtin/log.c b/builtin/log.c
index 78a247d8a9..b8846a9458 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1283,7 +1283,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
 	o2->flags = flags2;
 }
 
-static void gen_message_id(struct rev_info *info, char *base)
+static void gen_message_id(struct rev_info *info, const char *base)
 {
 	struct strbuf buf = STRBUF_INIT;
 	strbuf_addf(&buf, "%s.%"PRItime".git.%s", base,
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 3af9ddb8ae..fe6dbc5d05 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -113,8 +113,8 @@ static int populate_maildir_list(struct string_list *list, const char *path)
 	DIR *dir;
 	struct dirent *dent;
 	char *name = NULL;
-	char *subs[] = { "cur", "new", NULL };
-	char **sub;
+	const char *subs[] = { "cur", "new", NULL };
+	const char **sub;
 	int ret = -1;
 
 	for (sub = subs; *sub; ++sub) {
diff --git a/builtin/pull.c b/builtin/pull.c
index d622202bce..2d0429f14f 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -71,48 +71,48 @@ static const char * const pull_usage[] = {
 
 /* Shared options */
 static int opt_verbosity;
-static char *opt_progress;
+static const char *opt_progress;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
 
 /* Options passed to git-merge or git-rebase */
 static enum rebase_type opt_rebase = -1;
-static char *opt_diffstat;
-static char *opt_log;
-static char *opt_signoff;
-static char *opt_squash;
-static char *opt_commit;
-static char *opt_edit;
-static char *cleanup_arg;
-static char *opt_ff;
-static char *opt_verify_signatures;
-static char *opt_verify;
+static const char *opt_diffstat;
+static const char *opt_log;
+static const char *opt_signoff;
+static const char *opt_squash;
+static const char *opt_commit;
+static const char *opt_edit;
+static const char *cleanup_arg;
+static const char *opt_ff;
+static const char *opt_verify_signatures;
+static const char *opt_verify;
 static int opt_autostash = -1;
 static int config_autostash;
 static int check_trust_level = 1;
 static struct strvec opt_strategies = STRVEC_INIT;
 static struct strvec opt_strategy_opts = STRVEC_INIT;
-static char *opt_gpg_sign;
+static const char *opt_gpg_sign;
 static int opt_allow_unrelated_histories;
 
 /* Options passed to git-fetch */
-static char *opt_all;
-static char *opt_append;
-static char *opt_upload_pack;
+static const char *opt_all;
+static const char *opt_append;
+static const char *opt_upload_pack;
 static int opt_force;
-static char *opt_tags;
-static char *opt_prune;
-static char *max_children;
+static const char *opt_tags;
+static const char *opt_prune;
+static const char *max_children;
 static int opt_dry_run;
-static char *opt_keep;
-static char *opt_depth;
-static char *opt_unshallow;
-static char *opt_update_shallow;
-static char *opt_refmap;
-static char *opt_ipv4;
-static char *opt_ipv6;
+static const char *opt_keep;
+static const char *opt_depth;
+static const char *opt_unshallow;
+static const char *opt_update_shallow;
+static const char *opt_refmap;
+static const char *opt_ipv4;
+static const char *opt_ipv6;
 static int opt_show_forced_updates = -1;
-static char *set_upstream;
+static const char *set_upstream;
 static struct strvec opt_fetch = STRVEC_INIT;
 
 static struct option pull_options[] = {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 01c1f04ece..c8d12ee0a7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1249,7 +1249,7 @@ static int run_proc_receive_hook(struct command *commands,
 	return code;
 }
 
-static char *refuse_unconfigured_deny_msg =
+static const char *refuse_unconfigured_deny_msg =
 	N_("By default, updating the current branch in a non-bare repository\n"
 	   "is denied, because it will make the index and work tree inconsistent\n"
 	   "with what you pushed, and will require 'git reset --hard' to match\n"
@@ -1269,7 +1269,7 @@ static void refuse_unconfigured_deny(void)
 	rp_error("%s", _(refuse_unconfigured_deny_msg));
 }
 
-static char *refuse_unconfigured_deny_delete_current_msg =
+static const char *refuse_unconfigured_deny_delete_current_msg =
 	N_("By default, deleting the current branch is denied, because the next\n"
 	   "'git clone' won't result in any file checked out, causing confusion.\n"
 	   "\n"
diff --git a/builtin/revert.c b/builtin/revert.c
index 53935d2c68..7bf2b4e11d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -179,7 +179,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 
 	/* Check for incompatible command line arguments */
 	if (cmd) {
-		char *this_operation;
+		const char *this_operation;
 		if (cmd == 'q')
 			this_operation = "--quit";
 		else if (cmd == 'c')
diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c
index 2bc0f1187a..6c5d455e92 100644
--- a/compat/regex/regcomp.c
+++ b/compat/regex/regcomp.c
@@ -848,7 +848,7 @@ init_dfa (re_dfa_t *dfa, size_t pat_len)
 {
   unsigned int table_size;
 #ifndef _LIBC
-  char *codeset_name;
+  const char *codeset_name;
 #endif
 
   memset (dfa, '\0', sizeof (re_dfa_t));
diff --git a/diff.c b/diff.c
index e70301df76..ffd867ef6c 100644
--- a/diff.c
+++ b/diff.c
@@ -3764,7 +3764,7 @@ static void builtin_diff(const char *name_a,
 	return;
 }
 
-static char *get_compact_summary(const struct diff_filepair *p, int is_renamed)
+static const char *get_compact_summary(const struct diff_filepair *p, int is_renamed)
 {
 	if (!is_renamed) {
 		if (p->status == DIFF_STATUS_ADDED) {
@@ -4076,7 +4076,7 @@ static int reuse_worktree_file(struct index_state *istate,
 static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
 {
 	struct strbuf buf = STRBUF_INIT;
-	char *dirty = "";
+	const char *dirty = "";
 
 	/* Are we looking at the work tree? */
 	if (s->dirty_submodule)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 5a6e2bcac7..0e1adb87df 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -406,7 +406,7 @@ static const char *get_highest_rename_path(struct strintmap *counts)
 	return highest_destination_dir;
 }
 
-static char *UNKNOWN_DIR = "/";  /* placeholder -- short, illegal directory */
+static const char *UNKNOWN_DIR = "/";  /* placeholder -- short, illegal directory */
 
 static int dir_rename_already_determinable(struct strintmap *counts)
 {
@@ -429,8 +429,8 @@ static int dir_rename_already_determinable(struct strintmap *counts)
 }
 
 static void increment_count(struct dir_rename_info *info,
-			    char *old_dir,
-			    char *new_dir)
+			    const char *old_dir,
+			    const char *new_dir)
 {
 	struct strintmap *counts;
 	struct strmap_entry *e;
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 7d144b803a..5af63ab5ab 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -447,7 +447,7 @@ static void fmt_merge_msg_title(struct strbuf *out,
 				const char *current_branch)
 {
 	int i = 0;
-	char *sep = "";
+	const char *sep = "";
 
 	strbuf_addstr(out, "Merge ");
 	for (i = 0; i < srcs.nr; i++) {
diff --git a/fsck.c b/fsck.c
index 7dff41413e..61cd48aa25 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1231,7 +1231,7 @@ int fsck_object(struct object *obj, void *data, unsigned long size,
 }
 
 int fsck_buffer(const struct object_id *oid, enum object_type type,
-		void *data, unsigned long size,
+		const void *data, unsigned long size,
 		struct fsck_options *options)
 {
 	if (type == OBJ_BLOB)
diff --git a/fsck.h b/fsck.h
index 17fa2dda5d..4f0c4e6479 100644
--- a/fsck.h
+++ b/fsck.h
@@ -202,7 +202,7 @@ int fsck_object(struct object *obj, void *data, unsigned long size,
  * struct.
  */
 int fsck_buffer(const struct object_id *oid, enum object_type,
-		void *data, unsigned long size,
+		const void *data, unsigned long size,
 		struct fsck_options *options);
 
 /*
diff --git a/gpg-interface.c b/gpg-interface.c
index 5193223714..71a9382a61 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -727,7 +727,7 @@ static int git_gpg_config(const char *var, const char *value,
 			  void *cb UNUSED)
 {
 	struct gpg_format *fmt = NULL;
-	char *fmtname = NULL;
+	const char *fmtname = NULL;
 	char *trust;
 	int ret;
 
diff --git a/http-backend.c b/http-backend.c
index 5b65287ac9..5b4dca65ed 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -753,7 +753,7 @@ static int bad_request(struct strbuf *hdr, const struct service_cmd *c)
 
 int cmd_main(int argc UNUSED, const char **argv UNUSED)
 {
-	char *method = getenv("REQUEST_METHOD");
+	const char *method = getenv("REQUEST_METHOD");
 	const char *proto_header;
 	char *dir;
 	struct service_cmd *cmd = NULL;
diff --git a/imap-send.c b/imap-send.c
index a5d1510180..8b723b34a5 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1215,9 +1215,9 @@ static int imap_store_msg(struct imap_store *ctx, struct strbuf *msg)
 static void wrap_in_html(struct strbuf *msg)
 {
 	struct strbuf buf = STRBUF_INIT;
-	static char *content_type = "Content-Type: text/html;\n";
-	static char *pre_open = "<pre>\n";
-	static char *pre_close = "</pre>\n";
+	static const char *content_type = "Content-Type: text/html;\n";
+	static const char *pre_open = "<pre>\n";
+	static const char *pre_close = "</pre>\n";
 	const char *body = strstr(msg->buf, "\n\n");
 
 	if (!body)
diff --git a/pretty.c b/pretty.c
index 22a81506b7..ec05db5655 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1325,7 +1325,7 @@ int format_set_trailers_options(struct process_trailer_options *opts,
 static size_t parse_describe_args(const char *start, struct strvec *args)
 {
 	struct {
-		char *name;
+		const char *name;
 		enum {
 			DESCRIBE_ARG_BOOL,
 			DESCRIBE_ARG_INTEGER,
diff --git a/refs.c b/refs.c
index 8260c27cde..292e8d947e 100644
--- a/refs.c
+++ b/refs.c
@@ -159,7 +159,7 @@ void update_ref_namespace(enum ref_namespace namespace, char *ref)
 {
 	struct ref_namespace_info *info = &ref_namespace[namespace];
 	if (info->ref_updated)
-		free(info->ref);
+		free((char *)info->ref);
 	info->ref = ref;
 	info->ref_updated = 1;
 }
diff --git a/refs.h b/refs.h
index 34568ee1fb..923f751d18 100644
--- a/refs.h
+++ b/refs.h
@@ -975,7 +975,7 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt);
  */
 
 struct ref_namespace_info {
-	char *ref;
+	const char *ref;
 	enum decoration_type decoration;
 
 	/*
diff --git a/reftable/basics.c b/reftable/basics.c
index fea711db7e..0058619ca6 100644
--- a/reftable/basics.c
+++ b/reftable/basics.c
@@ -67,9 +67,9 @@ void free_names(char **a)
 	reftable_free(a);
 }
 
-size_t names_length(char **names)
+size_t names_length(const char **names)
 {
-	char **p = names;
+	const char **p = names;
 	while (*p)
 		p++;
 	return p - names;
@@ -102,15 +102,12 @@ void parse_names(char *buf, int size, char ***namesp)
 	*namesp = names;
 }
 
-int names_equal(char **a, char **b)
+int names_equal(const char **a, const char **b)
 {
-	int i = 0;
-	for (; a[i] && b[i]; i++) {
-		if (strcmp(a[i], b[i])) {
+	size_t i = 0;
+	for (; a[i] && b[i]; i++)
+		if (strcmp(a[i], b[i]))
 			return 0;
-		}
-	}
-
 	return a[i] == b[i];
 }
 
diff --git a/reftable/basics.h b/reftable/basics.h
index 523ecd5307..c8fec68d4e 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -42,10 +42,10 @@ void free_names(char **a);
 void parse_names(char *buf, int size, char ***namesp);
 
 /* compares two NULL-terminated arrays of strings. */
-int names_equal(char **a, char **b);
+int names_equal(const char **a, const char **b);
 
 /* returns the array size of a NULL-terminated array of strings. */
-size_t names_length(char **names);
+size_t names_length(const char **names);
 
 /* Allocation routines; they invoke the functions set through
  * reftable_set_alloc() */
diff --git a/reftable/basics_test.c b/reftable/basics_test.c
index 997c4d9e01..13bc761817 100644
--- a/reftable/basics_test.c
+++ b/reftable/basics_test.c
@@ -58,8 +58,8 @@ static void test_binsearch(void)
 
 static void test_names_length(void)
 {
-	char *a[] = { "a", "b", NULL };
-	EXPECT(names_length(a) == 2);
+	const char *names[] = { "a", "b", NULL };
+	EXPECT(names_length(names) == 2);
 }
 
 static void test_parse_names_normal(void)
diff --git a/reftable/record.c b/reftable/record.c
index 5506f3e913..a2cba5ef74 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -116,7 +116,7 @@ static int decode_string(struct strbuf *dest, struct string_view in)
 	return start_len - in.len;
 }
 
-static int encode_string(char *str, struct string_view s)
+static int encode_string(const char *str, struct string_view s)
 {
 	struct string_view start = s;
 	int l = strlen(str);
@@ -969,9 +969,9 @@ static int reftable_log_record_decode(void *rec, struct strbuf key,
 	return REFTABLE_FORMAT_ERROR;
 }
 
-static int null_streq(char *a, char *b)
+static int null_streq(const char *a, const char *b)
 {
-	char *empty = "";
+	const char *empty = "";
 	if (!a)
 		a = empty;
 
diff --git a/reftable/stack.c b/reftable/stack.c
index a59ebe038d..09549c51c9 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -204,7 +204,8 @@ static struct reftable_reader **stack_copy_readers(struct reftable_stack *st,
 	return cur;
 }
 
-static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
+static int reftable_stack_reload_once(struct reftable_stack *st,
+				      const char **names,
 				      int reuse_open)
 {
 	size_t cur_len = !st->merged ? 0 : st->merged->stack_len;
@@ -222,7 +223,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
 
 	while (*names) {
 		struct reftable_reader *rd = NULL;
-		char *name = *names++;
+		const char *name = *names++;
 
 		/* this is linear; we assume compaction keeps the number of
 		   tables under control so this is not quadratic. */
@@ -354,7 +355,7 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
 				goto out;
 		}
 
-		err = reftable_stack_reload_once(st, names, reuse_open);
+		err = reftable_stack_reload_once(st, (const char **) names, reuse_open);
 		if (!err)
 			break;
 		if (err != REFTABLE_NOT_EXIST_ERROR)
@@ -368,7 +369,8 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
 		err = read_lines(st->list_file, &names_after);
 		if (err < 0)
 			goto out;
-		if (names_equal(names_after, names)) {
+		if (names_equal((const char **) names_after,
+				(const char **) names)) {
 			err = REFTABLE_NOT_EXIST_ERROR;
 			goto out;
 		}
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 7889f818d1..07d89b45da 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -83,7 +83,7 @@ static void test_read_file(void)
 	char out[1024] = "line1\n\nline2\nline3";
 	int n, err;
 	char **names = NULL;
-	char *want[] = { "line1", "line2", "line3" };
+	const char *want[] = { "line1", "line2", "line3" };
 	int i = 0;
 
 	EXPECT(fd > 0);
@@ -116,9 +116,9 @@ static void test_parse_names(void)
 
 static void test_names_equal(void)
 {
-	char *a[] = { "a", "b", "c", NULL };
-	char *b[] = { "a", "b", "d", NULL };
-	char *c[] = { "a", "b", NULL };
+	const char *a[] = { "a", "b", "c", NULL };
+	const char *b[] = { "a", "b", "d", NULL };
+	const char *c[] = { "a", "b", NULL };
 
 	EXPECT(names_equal(a, a));
 	EXPECT(!names_equal(a, b));
diff --git a/run-command.c b/run-command.c
index 1b821042b4..7600531fb6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -663,7 +663,7 @@ int start_command(struct child_process *cmd)
 	int need_in, need_out, need_err;
 	int fdin[2], fdout[2], fderr[2];
 	int failed_errno;
-	char *str;
+	const char *str;
 
 	/*
 	 * In case of errors we must keep the promise to close FDs
diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c
index 0eb0b3d49c..2912899558 100644
--- a/t/helper/test-hashmap.c
+++ b/t/helper/test-hashmap.c
@@ -36,7 +36,8 @@ static int test_entry_cmp(const void *cmp_data,
 }
 
 static struct test_entry *alloc_test_entry(unsigned int hash,
-					   char *key, char *value)
+					   const char *key,
+					   const char *value)
 {
 	size_t klen = strlen(key);
 	size_t vlen = strlen(value);
diff --git a/t/helper/test-json-writer.c b/t/helper/test-json-writer.c
index afe393f597..ed52eb76bf 100644
--- a/t/helper/test-json-writer.c
+++ b/t/helper/test-json-writer.c
@@ -174,7 +174,7 @@ static void make_arr4(int pretty)
 	jw_end(&arr4);
 }
 
-static char *expect_nest1 =
+static const char *expect_nest1 =
 	"{\"obj1\":{\"a\":\"abc\",\"b\":42,\"c\":true},\"arr1\":[\"abc\",42,true]}";
 
 static struct json_writer nest1 = JSON_WRITER_INIT;
@@ -195,10 +195,10 @@ static void make_nest1(int pretty)
 	jw_release(&arr1);
 }
 
-static char *expect_inline1 =
+static const char *expect_inline1 =
 	"{\"obj1\":{\"a\":\"abc\",\"b\":42,\"c\":true},\"arr1\":[\"abc\",42,true]}";
 
-static char *pretty_inline1 =
+static const char *pretty_inline1 =
 	("{\n"
 	 "  \"obj1\": {\n"
 	 "    \"a\": \"abc\",\n"
@@ -236,10 +236,10 @@ static void make_inline1(int pretty)
 	jw_end(&inline1);
 }
 
-static char *expect_inline2 =
+static const char *expect_inline2 =
 	"[[1,2],[3,4],{\"a\":\"abc\"}]";
 
-static char *pretty_inline2 =
+static const char *pretty_inline2 =
 	("[\n"
 	 "  [\n"
 	 "    1,\n"
diff --git a/t/helper/test-regex.c b/t/helper/test-regex.c
index 80042eafc2..366bd70976 100644
--- a/t/helper/test-regex.c
+++ b/t/helper/test-regex.c
@@ -20,8 +20,8 @@ static struct reg_flag reg_flags[] = {
 
 static int test_regex_bug(void)
 {
-	char *pat = "[^={} \t]+";
-	char *str = "={}\nfred";
+	const char *pat = "[^={} \t]+";
+	const char *str = "={}\nfred";
 	regex_t r;
 	regmatch_t m[1];
 
diff --git a/t/helper/test-rot13-filter.c b/t/helper/test-rot13-filter.c
index f8d564c622..7e1d9e0ee4 100644
--- a/t/helper/test-rot13-filter.c
+++ b/t/helper/test-rot13-filter.c
@@ -136,7 +136,7 @@ static void free_delay_entries(void)
 	strmap_clear(&delay, 0);
 }
 
-static void add_delay_entry(char *pathname, int count, int requested)
+static void add_delay_entry(const char *pathname, int count, int requested)
 {
 	struct delay_entry *entry = xcalloc(1, sizeof(*entry));
 	entry->count = count;
@@ -189,7 +189,8 @@ static void reply_list_available_blobs_cmd(void)
 static void command_loop(void)
 {
 	for (;;) {
-		char *buf, *output;
+		char *buf;
+		const char *output;
 		char *pathname;
 		struct delay_entry *entry;
 		struct strbuf input = STRBUF_INIT;
diff --git a/t/unit-tests/t-strbuf.c b/t/unit-tests/t-strbuf.c
index de434a4441..6027dafef7 100644
--- a/t/unit-tests/t-strbuf.c
+++ b/t/unit-tests/t-strbuf.c
@@ -2,7 +2,8 @@
 #include "strbuf.h"
 
 /* wrapper that supplies tests with an empty, initialized strbuf */
-static void setup(void (*f)(struct strbuf*, void*), void *data)
+static void setup(void (*f)(struct strbuf*, const void*),
+		  const void *data)
 {
 	struct strbuf buf = STRBUF_INIT;
 
@@ -13,7 +14,8 @@ static void setup(void (*f)(struct strbuf*, void*), void *data)
 }
 
 /* wrapper that supplies tests with a populated, initialized strbuf */
-static void setup_populated(void (*f)(struct strbuf*, void*), char *init_str, void *data)
+static void setup_populated(void (*f)(struct strbuf*, const void*),
+			    const char *init_str, const void *data)
 {
 	struct strbuf buf = STRBUF_INIT;
 
@@ -64,7 +66,7 @@ static void t_dynamic_init(void)
 	strbuf_release(&buf);
 }
 
-static void t_addch(struct strbuf *buf, void *data)
+static void t_addch(struct strbuf *buf, const void *data)
 {
 	const char *p_ch = data;
 	const char ch = *p_ch;
@@ -83,7 +85,7 @@ static void t_addch(struct strbuf *buf, void *data)
 	check_char(buf->buf[buf->len], ==, '\0');
 }
 
-static void t_addstr(struct strbuf *buf, void *data)
+static void t_addstr(struct strbuf *buf, const void *data)
 {
 	const char *text = data;
 	size_t len = strlen(text);
diff --git a/trailer.c b/trailer.c
index 2bcb9ba8f7..72e5136c73 100644
--- a/trailer.c
+++ b/trailer.c
@@ -63,7 +63,7 @@ struct arg_item {
 
 static LIST_HEAD(conf_head);
 
-static char *separators = ":";
+static const char *separators = ":";
 
 static int configured;
 
diff --git a/wt-status.c b/wt-status.c
index ff4be071ca..7912545e4e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2408,7 +2408,7 @@ static void wt_porcelain_v2_print_unmerged_entry(
 		int mode;
 		struct object_id oid;
 	} stages[3];
-	char *key;
+	const char *key;
 	char submodule_token[5];
 	char unmerged_prefix = 'u';
 	char eol_char = s->null_termination ? '\0' : '\n';

From c113c5df7911bf7bc6a4542131ac5bf983532a97 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:37:43 +0200
Subject: [PATCH 02/27] global: convert intentionally-leaking config strings to
 consts

There are multiple cases where we intentionally leak config strings:

  - `struct gpg_format` is used to track programs that can be used for
    signing commits, either via gpg(1), gpgsm(1) or ssh-keygen(1). The
    user can override the commands via several config variables. As the
    array is populated once, only, and the struct memers are never
    written to or free'd.

  - `struct ll_merge_driver` is used to track merge drivers. Same as
    with the GPG format, these drivers are populated once and then
    reused. Its data is never written to or free'd, either.

  - `struct userdiff_funcname` and `struct userdiff_driver` can be
    configured via `diff.<driver>.*` to add additional drivers. Again,
    these have a global lifetime and are never written to or free'd.

All of these are intentionally kept alive and are never written to.
Furthermore, all of these are being assigned both string constants in
some places, and allocated strings in other places. This will cause
warnings once we enable `-Wwrite-strings`, so let's mark the respective
fields as `const char *` and cast away the constness when assigning
those values.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 gpg-interface.c |  4 ++--
 merge-ll.c      | 11 ++++++++---
 userdiff.c      | 10 +++++-----
 userdiff.h      | 12 ++++++------
 4 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 71a9382a61..5c824aeb25 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -34,7 +34,7 @@ static enum signature_trust_level configured_min_trust_level = TRUST_UNDEFINED;
 
 struct gpg_format {
 	const char *name;
-	char *program;
+	const char *program;
 	const char **verify_args;
 	const char **sigs;
 	int (*verify_signed_buffer)(struct signature_check *sigc,
@@ -783,7 +783,7 @@ static int git_gpg_config(const char *var, const char *value,
 
 	if (fmtname) {
 		fmt = get_format_by_name(fmtname);
-		return git_config_string(&fmt->program, var, value);
+		return git_config_string((char **) &fmt->program, var, value);
 	}
 
 	return 0;
diff --git a/merge-ll.c b/merge-ll.c
index e29b15fa4a..180c19df67 100644
--- a/merge-ll.c
+++ b/merge-ll.c
@@ -27,7 +27,7 @@ typedef enum ll_merge_result (*ll_merge_fn)(const struct ll_merge_driver *,
 
 struct ll_merge_driver {
 	const char *name;
-	char *description;
+	const char *description;
 	ll_merge_fn fn;
 	char *recursive;
 	struct ll_merge_driver *next;
@@ -304,8 +304,13 @@ static int read_merge_config(const char *var, const char *value,
 		ll_user_merge_tail = &(fn->next);
 	}
 
-	if (!strcmp("name", key))
-		return git_config_string(&fn->description, var, value);
+	if (!strcmp("name", key)) {
+		/*
+		 * The description is leaking, but that's okay as we want to
+		 * keep around the merge drivers anyway.
+		 */
+		return git_config_string((char **) &fn->description, var, value);
+	}
 
 	if (!strcmp("driver", key)) {
 		if (!value)
diff --git a/userdiff.c b/userdiff.c
index 82bc76b910..371032a413 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -399,7 +399,7 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *name, size_t
 static int parse_funcname(struct userdiff_funcname *f, const char *k,
 		const char *v, int cflags)
 {
-	if (git_config_string(&f->pattern, k, v) < 0)
+	if (git_config_string((char **) &f->pattern, k, v) < 0)
 		return -1;
 	f->cflags = cflags;
 	return 0;
@@ -445,15 +445,15 @@ int userdiff_config(const char *k, const char *v)
 	if (!strcmp(type, "binary"))
 		return parse_tristate(&drv->binary, k, v);
 	if (!strcmp(type, "command"))
-		return git_config_string(&drv->external, k, v);
+		return git_config_string((char **) &drv->external, k, v);
 	if (!strcmp(type, "textconv"))
-		return git_config_string(&drv->textconv, k, v);
+		return git_config_string((char **) &drv->textconv, k, v);
 	if (!strcmp(type, "cachetextconv"))
 		return parse_bool(&drv->textconv_want_cache, k, v);
 	if (!strcmp(type, "wordregex"))
-		return git_config_string(&drv->word_regex, k, v);
+		return git_config_string((char **) &drv->word_regex, k, v);
 	if (!strcmp(type, "algorithm"))
-		return git_config_string(&drv->algorithm, k, v);
+		return git_config_string((char **) &drv->algorithm, k, v);
 
 	return 0;
 }
diff --git a/userdiff.h b/userdiff.h
index cc8e5abfef..d726804c3e 100644
--- a/userdiff.h
+++ b/userdiff.h
@@ -7,19 +7,19 @@ struct index_state;
 struct repository;
 
 struct userdiff_funcname {
-	char *pattern;
+	const char *pattern;
 	int cflags;
 };
 
 struct userdiff_driver {
 	const char *name;
-	char *external;
-	char *algorithm;
+	const char *external;
+	const char *algorithm;
 	int binary;
 	struct userdiff_funcname funcname;
-	char *word_regex;
-	char *word_regex_multi_byte;
-	char *textconv;
+	const char *word_regex;
+	const char *word_regex_multi_byte;
+	const char *textconv;
 	struct notes_cache *textconv_cache;
 	int textconv_want_cache;
 };

From 23c32511b31f2123fd194ba3a89c758ba3e55143 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:37:48 +0200
Subject: [PATCH 03/27] refs/reftable: stop micro-optimizing refname
 allocations on copy

When copying refs, we execute `write_copy_table()` to write the new
table. As the names are given to us via `arg->newname` and
`arg->oldname`, respectively, we optimize away some allocations by
assigning those fields to the reftable records we are about to write
directly, without duplicating them. This requires us to cast the input
to `char *` pointers as they are in fact constant strings. Later on, we
then unset the refname for all of the records before calling
`reftable_log_record_release()` on them.

We also do this when assigning the "HEAD" constant, but here we do not
cast because its type is `char[]` by default. It's about to be turned
into `const char *` though once we enable `-Wwrite-strings` and will
thus cause another warning.

It's quite dubious whether this micro-optimization really helps. We're
about to write to disk anyway, which is going to be way slower than a
small handful of allocations. Let's drop the optimization altogther and
instead copy arguments to simplify the code and avoid the future warning
with `-Wwrite-strings`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 refs/reftable-backend.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 1af86bbdec..e77faa2b9d 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1340,10 +1340,10 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 	 * old reference.
 	 */
 	refs[0] = old_ref;
-	refs[0].refname = (char *)arg->newname;
+	refs[0].refname = xstrdup(arg->newname);
 	refs[0].update_index = creation_ts;
 	if (arg->delete_old) {
-		refs[1].refname = (char *)arg->oldname;
+		refs[1].refname = xstrdup(arg->oldname);
 		refs[1].value_type = REFTABLE_REF_DELETION;
 		refs[1].update_index = deletion_ts;
 	}
@@ -1366,7 +1366,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 		ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
 		memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
 		fill_reftable_log_record(&logs[logs_nr], &committer_ident);
-		logs[logs_nr].refname = (char *)arg->newname;
+		logs[logs_nr].refname = xstrdup(arg->newname);
 		logs[logs_nr].update_index = deletion_ts;
 		logs[logs_nr].value.update.message =
 			xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2);
@@ -1387,7 +1387,13 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 		if (append_head_reflog) {
 			ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
 			logs[logs_nr] = logs[logs_nr - 1];
-			logs[logs_nr].refname = "HEAD";
+			logs[logs_nr].refname = xstrdup("HEAD");
+			logs[logs_nr].value.update.name =
+				xstrdup(logs[logs_nr].value.update.name);
+			logs[logs_nr].value.update.email =
+				xstrdup(logs[logs_nr].value.update.email);
+			logs[logs_nr].value.update.message =
+				xstrdup(logs[logs_nr].value.update.message);
 			logs_nr++;
 		}
 	}
@@ -1398,7 +1404,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 	ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
 	memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
 	fill_reftable_log_record(&logs[logs_nr], &committer_ident);
-	logs[logs_nr].refname = (char *)arg->newname;
+	logs[logs_nr].refname = xstrdup(arg->newname);
 	logs[logs_nr].update_index = creation_ts;
 	logs[logs_nr].value.update.message =
 		xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2);
@@ -1430,7 +1436,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 		 */
 		ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
 		logs[logs_nr] = old_log;
-		logs[logs_nr].refname = (char *)arg->newname;
+		logs[logs_nr].refname = xstrdup(arg->newname);
 		logs_nr++;
 
 		/*
@@ -1439,7 +1445,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 		if (arg->delete_old) {
 			ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
 			memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
-			logs[logs_nr].refname = (char *)arg->oldname;
+			logs[logs_nr].refname = xstrdup(arg->oldname);
 			logs[logs_nr].value_type = REFTABLE_LOG_DELETION;
 			logs[logs_nr].update_index = old_log.update_index;
 			logs_nr++;
@@ -1462,13 +1468,11 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 	reftable_iterator_destroy(&it);
 	string_list_clear(&skip, 0);
 	strbuf_release(&errbuf);
-	for (i = 0; i < logs_nr; i++) {
-		if (!strcmp(logs[i].refname, "HEAD"))
-			continue;
-		logs[i].refname = NULL;
+	for (i = 0; i < logs_nr; i++)
 		reftable_log_record_release(&logs[i]);
-	}
 	free(logs);
+	for (i = 0; i < ARRAY_SIZE(refs); i++)
+		reftable_ref_record_release(&refs[i]);
 	reftable_ref_record_release(&old_ref);
 	reftable_log_record_release(&old_log);
 	return ret;

From 66f892bb075f19bed784b86c7850a89c9a865aca Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:37:52 +0200
Subject: [PATCH 04/27] reftable: cast away constness when assigning constants
 to records

The reftable records are used in multiple ways throughout the reftable
library. In many of those cases they merely act as input to a function
without getting modified by it at all. Most importantly, this happens
when writing records and when querying for records.

We rely on this in our tests and thus assign string constants to those
fields, which is about to generate warnings as those fields are of type
`char *`. While we could go through the process and instead allocate
those strings in all of our tests, this feels quite unnecessary.

Instead, add casts to `char *` for all of those strings. As this is part
of our tests, this also nicely serves as a demonstration that nothing
writes or frees those string constants, which would otherwise lead to
segfaults.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 reftable/block_test.c     |  2 +-
 reftable/merged_test.c    | 44 +++++++++++++++++------------------
 reftable/readwrite_test.c | 32 +++++++++++++-------------
 reftable/stack_test.c     | 48 +++++++++++++++++++--------------------
 4 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/reftable/block_test.c b/reftable/block_test.c
index 26a9cfbc83..90aecd5a7c 100644
--- a/reftable/block_test.c
+++ b/reftable/block_test.c
@@ -42,7 +42,7 @@ static void test_block_read_write(void)
 	block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,
 			  header_off, hash_size(GIT_SHA1_FORMAT_ID));
 
-	rec.u.ref.refname = "";
+	rec.u.ref.refname = (char *) "";
 	rec.u.ref.value_type = REFTABLE_REF_DELETION;
 	n = block_writer_add(&bw, &rec);
 	EXPECT(n == REFTABLE_API_ERROR);
diff --git a/reftable/merged_test.c b/reftable/merged_test.c
index 530fc82d1c..6d1159d12d 100644
--- a/reftable/merged_test.c
+++ b/reftable/merged_test.c
@@ -124,13 +124,13 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)
 static void test_merged_between(void)
 {
 	struct reftable_ref_record r1[] = { {
-		.refname = "b",
+		.refname = (char *) "b",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_VAL1,
 		.value.val1 = { 1, 2, 3, 0 },
 	} };
 	struct reftable_ref_record r2[] = { {
-		.refname = "a",
+		.refname = (char *) "a",
 		.update_index = 2,
 		.value_type = REFTABLE_REF_DELETION,
 	} };
@@ -165,38 +165,38 @@ static void test_merged(void)
 {
 	struct reftable_ref_record r1[] = {
 		{
-			.refname = "a",
+			.refname = (char *) "a",
 			.update_index = 1,
 			.value_type = REFTABLE_REF_VAL1,
 			.value.val1 = { 1 },
 		},
 		{
-			.refname = "b",
+			.refname = (char *) "b",
 			.update_index = 1,
 			.value_type = REFTABLE_REF_VAL1,
 			.value.val1 = { 1 },
 		},
 		{
-			.refname = "c",
+			.refname = (char *) "c",
 			.update_index = 1,
 			.value_type = REFTABLE_REF_VAL1,
 			.value.val1 = { 1 },
 		}
 	};
 	struct reftable_ref_record r2[] = { {
-		.refname = "a",
+		.refname = (char *) "a",
 		.update_index = 2,
 		.value_type = REFTABLE_REF_DELETION,
 	} };
 	struct reftable_ref_record r3[] = {
 		{
-			.refname = "c",
+			.refname = (char *) "c",
 			.update_index = 3,
 			.value_type = REFTABLE_REF_VAL1,
 			.value.val1 = { 2 },
 		},
 		{
-			.refname = "d",
+			.refname = (char *) "d",
 			.update_index = 3,
 			.value_type = REFTABLE_REF_VAL1,
 			.value.val1 = { 1 },
@@ -291,46 +291,46 @@ static void test_merged_logs(void)
 {
 	struct reftable_log_record r1[] = {
 		{
-			.refname = "a",
+			.refname = (char *) "a",
 			.update_index = 2,
 			.value_type = REFTABLE_LOG_UPDATE,
 			.value.update = {
 				.old_hash = { 2 },
 				/* deletion */
-				.name = "jane doe",
-				.email = "jane@invalid",
-				.message = "message2",
+				.name = (char *) "jane doe",
+				.email = (char *) "jane@invalid",
+				.message = (char *) "message2",
 			}
 		},
 		{
-			.refname = "a",
+			.refname = (char *) "a",
 			.update_index = 1,
 			.value_type = REFTABLE_LOG_UPDATE,
 			.value.update = {
 				.old_hash = { 1 },
 				.new_hash = { 2 },
-				.name = "jane doe",
-				.email = "jane@invalid",
-				.message = "message1",
+				.name = (char *) "jane doe",
+				.email = (char *) "jane@invalid",
+				.message = (char *) "message1",
 			}
 		},
 	};
 	struct reftable_log_record r2[] = {
 		{
-			.refname = "a",
+			.refname = (char *) "a",
 			.update_index = 3,
 			.value_type = REFTABLE_LOG_UPDATE,
 			.value.update = {
 				.new_hash = { 3 },
-				.name = "jane doe",
-				.email = "jane@invalid",
-				.message = "message3",
+				.name = (char *) "jane doe",
+				.email = (char *) "jane@invalid",
+				.message = (char *) "message3",
 			}
 		},
 	};
 	struct reftable_log_record r3[] = {
 		{
-			.refname = "a",
+			.refname = (char *) "a",
 			.update_index = 2,
 			.value_type = REFTABLE_LOG_DELETION,
 		},
@@ -406,7 +406,7 @@ static void test_default_write_opts(void)
 		reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
 
 	struct reftable_ref_record rec = {
-		.refname = "master",
+		.refname = (char *) "master",
 		.update_index = 1,
 	};
 	int err;
diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
index a6dbd214c5..c55019232b 100644
--- a/reftable/readwrite_test.c
+++ b/reftable/readwrite_test.c
@@ -86,7 +86,7 @@ static void write_table(char ***names, struct strbuf *buf, int N,
 		log.update_index = update_index;
 		log.value_type = REFTABLE_LOG_UPDATE;
 		set_test_hash(log.value.update.new_hash, i);
-		log.value.update.message = "message";
+		log.value.update.message = (char *) "message";
 
 		n = reftable_writer_add_log(w, &log);
 		EXPECT(n == 0);
@@ -118,15 +118,15 @@ static void test_log_buffer_size(void)
 	int err;
 	int i;
 	struct reftable_log_record
-		log = { .refname = "refs/heads/master",
+		log = { .refname = (char *) "refs/heads/master",
 			.update_index = 0xa,
 			.value_type = REFTABLE_LOG_UPDATE,
 			.value = { .update = {
-					   .name = "Han-Wen Nienhuys",
-					   .email = "hanwen@google.com",
+					   .name = (char *) "Han-Wen Nienhuys",
+					   .email = (char *) "hanwen@google.com",
 					   .tz_offset = 100,
 					   .time = 0x5e430672,
-					   .message = "commit: 9\n",
+					   .message = (char *) "commit: 9\n",
 				   } } };
 	struct reftable_writer *w =
 		reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
@@ -156,15 +156,15 @@ static void test_log_overflow(void)
 	};
 	int err;
 	struct reftable_log_record log = {
-		.refname = "refs/heads/master",
+		.refname = (char *) "refs/heads/master",
 		.update_index = 0xa,
 		.value_type = REFTABLE_LOG_UPDATE,
 		.value = {
 			.update = {
 				.old_hash = { 1 },
 				.new_hash = { 2 },
-				.name = "Han-Wen Nienhuys",
-				.email = "hanwen@google.com",
+				.name = (char *) "Han-Wen Nienhuys",
+				.email = (char *) "hanwen@google.com",
 				.tz_offset = 100,
 				.time = 0x5e430672,
 				.message = msg,
@@ -293,14 +293,14 @@ static void test_log_zlib_corruption(void)
 	char message[100] = { 0 };
 	int err, i, n;
 	struct reftable_log_record log = {
-		.refname = "refname",
+		.refname = (char *) "refname",
 		.value_type = REFTABLE_LOG_UPDATE,
 		.value = {
 			.update = {
 				.new_hash = { 1 },
 				.old_hash = { 2 },
-				.name = "My Name",
-				.email = "myname@invalid",
+				.name = (char *) "My Name",
+				.email = (char *) "myname@invalid",
 				.message = message,
 			},
 		},
@@ -728,7 +728,7 @@ static void test_write_empty_key(void)
 	struct reftable_writer *w =
 		reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
 	struct reftable_ref_record ref = {
-		.refname = "",
+		.refname = (char *) "",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_DELETION,
 	};
@@ -752,18 +752,18 @@ static void test_write_key_order(void)
 		reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
 	struct reftable_ref_record refs[2] = {
 		{
-			.refname = "b",
+			.refname = (char *) "b",
 			.update_index = 1,
 			.value_type = REFTABLE_REF_SYMREF,
 			.value = {
-				.symref = "target",
+				.symref = (char *) "target",
 			},
 		}, {
-			.refname = "a",
+			.refname = (char *) "a",
 			.update_index = 1,
 			.value_type = REFTABLE_REF_SYMREF,
 			.value = {
-				.symref = "target",
+				.symref = (char *) "target",
 			},
 		}
 	};
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 07d89b45da..4abf92636d 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -156,10 +156,10 @@ static void test_reftable_stack_add_one(void)
 	struct reftable_stack *st = NULL;
 	int err;
 	struct reftable_ref_record ref = {
-		.refname = "HEAD",
+		.refname = (char *) "HEAD",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_SYMREF,
-		.value.symref = "master",
+		.value.symref = (char *) "master",
 	};
 	struct reftable_ref_record dest = { NULL };
 	struct stat stat_result = { 0 };
@@ -216,16 +216,16 @@ static void test_reftable_stack_uptodate(void)
 
 	int err;
 	struct reftable_ref_record ref1 = {
-		.refname = "HEAD",
+		.refname = (char *) "HEAD",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_SYMREF,
-		.value.symref = "master",
+		.value.symref = (char *) "master",
 	};
 	struct reftable_ref_record ref2 = {
-		.refname = "branch2",
+		.refname = (char *) "branch2",
 		.update_index = 2,
 		.value_type = REFTABLE_REF_SYMREF,
-		.value.symref = "master",
+		.value.symref = (char *) "master",
 	};
 
 
@@ -264,10 +264,10 @@ static void test_reftable_stack_transaction_api(void)
 	struct reftable_addition *add = NULL;
 
 	struct reftable_ref_record ref = {
-		.refname = "HEAD",
+		.refname = (char *) "HEAD",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_SYMREF,
-		.value.symref = "master",
+		.value.symref = (char *) "master",
 	};
 	struct reftable_ref_record dest = { NULL };
 
@@ -313,7 +313,7 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
 		struct reftable_ref_record ref = {
 			.update_index = reftable_stack_next_update_index(st),
 			.value_type = REFTABLE_REF_SYMREF,
-			.value.symref = "master",
+			.value.symref = (char *) "master",
 		};
 		char name[100];
 
@@ -356,7 +356,7 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
 static void test_reftable_stack_auto_compaction_fails_gracefully(void)
 {
 	struct reftable_ref_record ref = {
-		.refname = "refs/heads/master",
+		.refname = (char *) "refs/heads/master",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_VAL1,
 		.value.val1 = {0x01},
@@ -409,16 +409,16 @@ static void test_reftable_stack_update_index_check(void)
 	struct reftable_stack *st = NULL;
 	int err;
 	struct reftable_ref_record ref1 = {
-		.refname = "name1",
+		.refname = (char *) "name1",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_SYMREF,
-		.value.symref = "master",
+		.value.symref = (char *) "master",
 	};
 	struct reftable_ref_record ref2 = {
-		.refname = "name2",
+		.refname = (char *) "name2",
 		.update_index = 1,
 		.value_type = REFTABLE_REF_SYMREF,
-		.value.symref = "master",
+		.value.symref = (char *) "master",
 	};
 
 	err = reftable_new_stack(&st, dir, cfg);
@@ -561,7 +561,7 @@ static void test_reftable_stack_log_normalize(void)
 	struct reftable_stack *st = NULL;
 	char *dir = get_tmp_dir(__LINE__);
 	struct reftable_log_record input = {
-		.refname = "branch",
+		.refname = (char *) "branch",
 		.update_index = 1,
 		.value_type = REFTABLE_LOG_UPDATE,
 		.value = {
@@ -582,11 +582,11 @@ static void test_reftable_stack_log_normalize(void)
 	err = reftable_new_stack(&st, dir, cfg);
 	EXPECT_ERR(err);
 
-	input.value.update.message = "one\ntwo";
+	input.value.update.message = (char *) "one\ntwo";
 	err = reftable_stack_add(st, &write_test_log, &arg);
 	EXPECT(err == REFTABLE_API_ERROR);
 
-	input.value.update.message = "one";
+	input.value.update.message = (char *) "one";
 	err = reftable_stack_add(st, &write_test_log, &arg);
 	EXPECT_ERR(err);
 
@@ -594,7 +594,7 @@ static void test_reftable_stack_log_normalize(void)
 	EXPECT_ERR(err);
 	EXPECT(0 == strcmp(dest.value.update.message, "one\n"));
 
-	input.value.update.message = "two\n";
+	input.value.update.message = (char *) "two\n";
 	arg.update_index = 2;
 	err = reftable_stack_add(st, &write_test_log, &arg);
 	EXPECT_ERR(err);
@@ -697,9 +697,9 @@ static void test_reftable_stack_hash_id(void)
 	int err;
 
 	struct reftable_ref_record ref = {
-		.refname = "master",
+		.refname = (char *) "master",
 		.value_type = REFTABLE_REF_SYMREF,
-		.value.symref = "target",
+		.value.symref = (char *) "target",
 		.update_index = 1,
 	};
 	struct reftable_write_options cfg32 = { .hash_id = GIT_SHA256_FORMAT_ID };
@@ -879,7 +879,7 @@ static void test_reftable_stack_auto_compaction(void)
 			.refname = name,
 			.update_index = reftable_stack_next_update_index(st),
 			.value_type = REFTABLE_REF_SYMREF,
-			.value.symref = "master",
+			.value.symref = (char *) "master",
 		};
 		snprintf(name, sizeof(name), "branch%04d", i);
 
@@ -913,7 +913,7 @@ static void test_reftable_stack_add_performs_auto_compaction(void)
 		struct reftable_ref_record ref = {
 			.update_index = reftable_stack_next_update_index(st),
 			.value_type = REFTABLE_REF_SYMREF,
-			.value.symref = "master",
+			.value.symref = (char *) "master",
 		};
 
 		/*
@@ -964,7 +964,7 @@ static void test_reftable_stack_compaction_concurrent(void)
 			.refname = name,
 			.update_index = reftable_stack_next_update_index(st1),
 			.value_type = REFTABLE_REF_SYMREF,
-			.value.symref = "master",
+			.value.symref = (char *) "master",
 		};
 		snprintf(name, sizeof(name), "branch%04d", i);
 
@@ -1014,7 +1014,7 @@ static void test_reftable_stack_compaction_concurrent_clean(void)
 			.refname = name,
 			.update_index = reftable_stack_next_update_index(st1),
 			.value_type = REFTABLE_REF_SYMREF,
-			.value.symref = "master",
+			.value.symref = (char *) "master",
 		};
 		snprintf(name, sizeof(name), "branch%04d", i);
 

From 235ac3f81ad1950f2e6b47b30561eb96844e1c85 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:37:57 +0200
Subject: [PATCH 05/27] refspec: remove global tag refspec structure

We have a global tag refspec structure that is used by both git-clone(1)
and git-fetch(1). Initialization of the structure will break once we
enable `-Wwrite-strings`, even though the breakage is harmless. While we
could just add casts, the structure isn't really required in the first
place as we can simply initialize the structures at the respective
callsites.

Refactor the code accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/clone.c |  8 ++++++--
 builtin/fetch.c | 11 ++++++++---
 refspec.c       | 13 -------------
 refspec.h       |  1 -
 4 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 92ab7d7165..bde1d284a2 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -523,6 +523,9 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
 	struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
 	struct ref *local_refs = head;
 	struct ref **tail = head ? &head->next : &local_refs;
+	struct refspec_item tag_refspec;
+
+	refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
 
 	if (option_single_branch) {
 		struct ref *remote_head = NULL;
@@ -545,7 +548,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
 					      &tail, 0);
 
 			/* if --branch=tag, pull the requested tag explicitly */
-			get_fetch_map(remote_head, tag_refspec, &tail, 0);
+			get_fetch_map(remote_head, &tag_refspec, &tail, 0);
 		}
 		free_refs(remote_head);
 	} else {
@@ -555,8 +558,9 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
 	}
 
 	if (!option_mirror && !option_single_branch && !option_no_tags)
-		get_fetch_map(refs, tag_refspec, &tail, 0);
+		get_fetch_map(refs, &tag_refspec, &tail, 0);
 
+	refspec_item_clear(&tag_refspec);
 	return local_refs;
 }
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 75255dc600..06b60867f5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -582,11 +582,16 @@ static struct ref *get_ref_map(struct remote *remote,
 		}
 	}
 
-	if (tags == TAGS_SET)
+	if (tags == TAGS_SET) {
+		struct refspec_item tag_refspec;
+
 		/* also fetch all tags */
-		get_fetch_map(remote_refs, tag_refspec, &tail, 0);
-	else if (tags == TAGS_DEFAULT && *autotags)
+		refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
+		get_fetch_map(remote_refs, &tag_refspec, &tail, 0);
+		refspec_item_clear(&tag_refspec);
+	} else if (tags == TAGS_DEFAULT && *autotags) {
 		find_non_local_tags(remote_refs, NULL, &ref_map, &tail);
+	}
 
 	/* Now append any refs to be updated opportunistically: */
 	*tail = orefs;
diff --git a/refspec.c b/refspec.c
index d60932f4de..1df5de6c2f 100644
--- a/refspec.c
+++ b/refspec.c
@@ -7,19 +7,6 @@
 #include "refspec.h"
 #include "strbuf.h"
 
-static struct refspec_item s_tag_refspec = {
-	.force = 0,
-	.pattern = 1,
-	.matching = 0,
-	.exact_sha1 = 0,
-	.negative = 0,
-	.src = "refs/tags/*",
-	.dst = "refs/tags/*",
-};
-
-/* See TAG_REFSPEC for the string version */
-const struct refspec_item *tag_refspec = &s_tag_refspec;
-
 /*
  * Parses the provided refspec 'refspec' and populates the refspec_item 'item'.
  * Returns 1 if successful and 0 if the refspec is invalid.
diff --git a/refspec.h b/refspec.h
index 8c0c446993..754be45cee 100644
--- a/refspec.h
+++ b/refspec.h
@@ -2,7 +2,6 @@
 #define REFSPEC_H
 
 #define TAG_REFSPEC "refs/tags/*:refs/tags/*"
-extern const struct refspec_item *tag_refspec;
 
 /**
  * A struct refspec_item holds the parsed interpretation of a refspec.  If it

From 81654d27bf9e2f3745fb6190cb5b0863f91a644e Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:02 +0200
Subject: [PATCH 06/27] builtin/remote: cast away constness in
 `get_head_names()`

In `get_head_names()`, we assign the "refs/heads/*" string constant to
`struct refspec_item::{src,dst}`, which are both non-constant pointers.
Ideally, we'd refactor the code such that both of these fields were
constant. But `struct refspec_item` is used for two different usecases
with conflicting requirements:

  - To query for a source or destination based on the given refspec. The
    caller either sets `src` or `dst` as the branch that we want to
    search for, and the respective other field gets populated. The
    fields should be constant when being used as a query parameter,
    which is owned by the caller, and non-constant when being used as an
    out parameter, which is owned by the refspec item. This is is
    contradictory in itself already.

  - To store refspec items with their respective source and destination
    branches, in which case both fields should be owned by the struct.

Ideally, we'd split up this interface to clearly separate between
querying and storing, which would enable us to clarify lifetimes of the
strings. This would be a much bigger undertaking though.

Instead, accept the status quo for now and cast away the constness of
the source and destination patterns. We know that those are not being
written to or freed, so while this is ugly it certainly is fine for now.

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

diff --git a/builtin/remote.c b/builtin/remote.c
index d52b1c0e10..b44f580b8c 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -493,12 +493,13 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
 {
 	struct ref *ref, *matches;
 	struct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;
-	struct refspec_item refspec;
+	struct refspec_item refspec = {
+		.force = 0,
+		.pattern = 1,
+		.src = (char *) "refs/heads/*",
+		.dst = (char *) "refs/heads/*",
+	};
 
-	memset(&refspec, 0, sizeof(refspec));
-	refspec.force = 0;
-	refspec.pattern = 1;
-	refspec.src = refspec.dst = "refs/heads/*";
 	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
 	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
 				    fetch_map, 1);
@@ -507,7 +508,6 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
 
 	free_refs(fetch_map);
 	free_refs(matches);
-
 	return 0;
 }
 

From 86badd4d0a51262c2da1ca45c36612a28aad3b59 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:06 +0200
Subject: [PATCH 07/27] diff: cast string constant in `fill_textconv()`

The `fill_textconv()` function is responsible for converting an input
file with a textconv driver, which is then passed to the caller. Weirdly
though, the function also handles the case where there is no textconv
driver at all. In that case, it will return either the contents of the
populated filespec, or an empty string if the filespec is invalid.

These two cases have differing memory ownership semantics. When there is
a textconv driver, then the result is an allocated string. Otherwise,
the result is either a string constant or owned by the filespec struct.
All callers are in fact aware of this weirdness and only end up freeing
the output buffer when they had a textconv driver.

Ideally, we'd split up this interface to only perform the conversion via
the textconv driver, and BUG in case the caller didn't provide one. This
would make memory ownership semantics much more straight forward. For
now though, let's simply cast the empty string constant to `char *` to
avoid a warning with `-Wwrite-strings`. This is equivalent to the same
cast that we already have in `fill_mmfile()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index ffd867ef6c..cecda216cf 100644
--- a/diff.c
+++ b/diff.c
@@ -7235,7 +7235,7 @@ size_t fill_textconv(struct repository *r,
 
 	if (!driver) {
 		if (!DIFF_FILE_VALID(df)) {
-			*outbuf = "";
+			*outbuf = (char *) "";
 			return 0;
 		}
 		if (diff_populate_filespec(r, df, NULL))

From 42d2ad55566013535c16b80fc2d445da93cceed3 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:11 +0200
Subject: [PATCH 08/27] line-log: stop assigning string constant to file parent
 buffer

Stop assigning a string constant to the file parent buffer and instead
assign an allocated string. While the code is fine in practice, it will
break once we compile with `-Wwrite-strings`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 line-log.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/line-log.c b/line-log.c
index 8ff6ccb772..bd3e663c24 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1032,6 +1032,7 @@ static int process_diff_filepair(struct rev_info *rev,
 	struct range_set tmp;
 	struct diff_ranges diff;
 	mmfile_t file_parent, file_target;
+	char *parent_data_to_free = NULL;
 
 	assert(pair->two->path);
 	while (rg) {
@@ -1056,7 +1057,7 @@ static int process_diff_filepair(struct rev_info *rev,
 		file_parent.ptr = pair->one->data;
 		file_parent.size = pair->one->size;
 	} else {
-		file_parent.ptr = "";
+		file_parent.ptr = parent_data_to_free = xstrdup("");
 		file_parent.size = 0;
 	}
 
@@ -1075,6 +1076,7 @@ static int process_diff_filepair(struct rev_info *rev,
 
 	diff_ranges_release(&diff);
 
+	free(parent_data_to_free);
 	return ((*diff_out)->parent.nr > 0);
 }
 

From 394affd46dc9b0c805df6e999837aa297f844fc0 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:16 +0200
Subject: [PATCH 09/27] line-log: always allocate the output prefix

The returned string by `output_prefix()` is sometimes a string constant
and sometimes an allocated string. This has been fine until now because
we always leak the allocated strings, and thus we never tried to free
the string constant.

Fix the code to always return an allocated string and free the returned
value at all callsites.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 line-log.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/line-log.c b/line-log.c
index bd3e663c24..67c80b39a0 100644
--- a/line-log.c
+++ b/line-log.c
@@ -899,14 +899,12 @@ static void print_line(const char *prefix, char first,
 
 static char *output_prefix(struct diff_options *opt)
 {
-	char *prefix = "";
-
 	if (opt->output_prefix) {
 		struct strbuf *sb = opt->output_prefix(opt, opt->output_prefix_data);
-		prefix = sb->buf;
+		return sb->buf;
+	} else {
+		return xstrdup("");
 	}
-
-	return prefix;
 }
 
 static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *range)
@@ -927,7 +925,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang
 	const char *c_context = diff_get_color(opt->use_color, DIFF_CONTEXT);
 
 	if (!pair || !diff)
-		return;
+		goto out;
 
 	if (pair->one->oid_valid)
 		fill_line_ends(rev->diffopt.repo, pair->one, &p_lines, &p_ends);
@@ -1002,8 +1000,10 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang
 				   c_context, c_reset, opt->file);
 	}
 
+out:
 	free(p_ends);
 	free(t_ends);
+	free(prefix);
 }
 
 /*
@@ -1012,7 +1012,11 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang
  */
 static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range)
 {
-	fprintf(rev->diffopt.file, "%s\n", output_prefix(&rev->diffopt));
+	char *prefix = output_prefix(&rev->diffopt);
+
+	fprintf(rev->diffopt.file, "%s\n", prefix);
+	free(prefix);
+
 	while (range) {
 		dump_diff_hacky_one(rev, range);
 		range = range->next;

From b31607a3e03b2002c468c16bb71fcaf2d1846e09 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:20 +0200
Subject: [PATCH 10/27] entry: refactor how we remove items for delayed
 checkouts

When finalizing a delayed checkout, we sort out several strings from the
passed-in string list by first assigning the empty string to those
filters and then calling `string_list_remove_empty_items()`. Assigning
the empty string will cause compiler warnings though as the string is
a `char *` once we enable `-Wwrite-strings`.

Refactor the code to use a `NULL` pointer with `filter_string_list()`
instead to avoid this warning.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 entry.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/entry.c b/entry.c
index b8c257f6f9..f291d8eee6 100644
--- a/entry.c
+++ b/entry.c
@@ -167,6 +167,11 @@ static int remove_available_paths(struct string_list_item *item, void *cb_data)
 	return !available;
 }
 
+static int string_is_not_null(struct string_list_item *item, void *data UNUSED)
+{
+	return !!item->string;
+}
+
 int finish_delayed_checkout(struct checkout *state, int show_progress)
 {
 	int errs = 0;
@@ -189,7 +194,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)
 			if (!async_query_available_blobs(filter->string, &available_paths)) {
 				/* Filter reported an error */
 				errs = 1;
-				filter->string = "";
+				filter->string = NULL;
 				continue;
 			}
 			if (available_paths.nr <= 0) {
@@ -199,7 +204,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)
 				 * filter from the list (see
 				 * "string_list_remove_empty_items" call below).
 				 */
-				filter->string = "";
+				filter->string = NULL;
 				continue;
 			}
 
@@ -225,7 +230,7 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)
 					 * Do not ask the filter for available blobs,
 					 * again, as the filter is likely buggy.
 					 */
-					filter->string = "";
+					filter->string = NULL;
 					continue;
 				}
 				ce = index_file_exists(state->istate, path->string,
@@ -239,7 +244,8 @@ int finish_delayed_checkout(struct checkout *state, int show_progress)
 					errs = 1;
 			}
 		}
-		string_list_remove_empty_items(&dco->filters, 0);
+
+		filter_string_list(&dco->filters, 0, string_is_not_null, NULL);
 	}
 	stop_progress(&progress);
 	string_list_clear(&dco->filters, 0);

From 32f9929109e2858b81403f2cbb2a80774f822859 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:25 +0200
Subject: [PATCH 11/27] ident: add casts for fallback name and GECOS

In `xgetpwuid_self()`, we return a fallback identity when it was not
possible to look up the current identity. This fallback identity needs
to be internal and must never be written to by the calles as specified
by getpwuid(3P). As both the `pw_name` and `pw_gecos` fields are marked
as non-constant though, it will cause a warning to assign constant
strings to them once compiling with `-Wwrite-strings`.

Add explicit casts to avoid the warning.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ident.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ident.c b/ident.c
index cc7afdbf81..caf41fb2a9 100644
--- a/ident.c
+++ b/ident.c
@@ -46,9 +46,9 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
 	pw = getpwuid(getuid());
 	if (!pw) {
 		static struct passwd fallback;
-		fallback.pw_name = "unknown";
+		fallback.pw_name = (char *) "unknown";
 #ifndef NO_GECOS_IN_PWENT
-		fallback.pw_gecos = "Unknown";
+		fallback.pw_gecos = (char *) "Unknown";
 #endif
 		pw = &fallback;
 		if (is_bogus)

From 724b6d1e18606ca2fa14d7cc48c3bf3884363e25 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:30 +0200
Subject: [PATCH 12/27] object-file: mark cached object buffers as const

The buffers of cached objects are never modified, but are still stored
as a non-constant pointer. This will cause a compiler warning once we
enable the `-Wwrite-strings` compiler warning as we assign an empty
constant string when initializing the static `empty_tree` cached object.

Convert the field to be constant. This requires us to shuffle around
the code a bit because we memcpy(3P) into the allocated buffer in
`pretend_object_file()`. This is easily fixed though by allocating the
buffer into a temporary variable first.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 object-file.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/object-file.c b/object-file.c
index 610b1f465c..08c00dcc02 100644
--- a/object-file.c
+++ b/object-file.c
@@ -277,7 +277,7 @@ int hash_algo_by_length(int len)
 static struct cached_object {
 	struct object_id oid;
 	enum object_type type;
-	void *buf;
+	const void *buf;
 	unsigned long size;
 } *cached_objects;
 static int cached_object_nr, cached_object_alloc;
@@ -1778,6 +1778,7 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,
 			struct object_id *oid)
 {
 	struct cached_object *co;
+	char *co_buf;
 
 	hash_object_file(the_hash_algo, buf, len, type, oid);
 	if (repo_has_object_file_with_flags(the_repository, oid, OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT) ||
@@ -1787,8 +1788,9 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,
 	co = &cached_objects[cached_object_nr++];
 	co->size = len;
 	co->type = type;
-	co->buf = xmalloc(len);
-	memcpy(co->buf, buf, len);
+	co_buf = xmalloc(len);
+	memcpy(co_buf, buf, len);
+	co->buf = co_buf;
 	oidcpy(&co->oid, oid);
 	return 0;
 }

From 9f03e4813a5b4e469b3a7f5ad4ada3a9c3f92bfd Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:35 +0200
Subject: [PATCH 13/27] object-file: make `buf` parameter of `index_mem()` a
 constant

The `buf` parameter of `index_mem()` is a non-constant string. This will
break once we enable `-Wwrite-strings` because we also pass constants
from at least one callsite.

Adapt the parameter to be a constant. As we cannot free the buffer
without casting now, this also requires us to move the lifetime of the
nested buffer around.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 object-file.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/object-file.c b/object-file.c
index 08c00dcc02..0b58751f94 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2484,12 +2484,13 @@ static int hash_format_check_report(struct fsck_options *opts UNUSED,
 }
 
 static int index_mem(struct index_state *istate,
-		     struct object_id *oid, void *buf, size_t size,
+		     struct object_id *oid,
+		     const void *buf, size_t size,
 		     enum object_type type,
 		     const char *path, unsigned flags)
 {
+	struct strbuf nbuf = STRBUF_INIT;
 	int ret = 0;
-	int re_allocated = 0;
 	int write_object = flags & HASH_WRITE_OBJECT;
 
 	if (!type)
@@ -2499,11 +2500,10 @@ static int index_mem(struct index_state *istate,
 	 * Convert blobs to git internal format
 	 */
 	if ((type == OBJ_BLOB) && path) {
-		struct strbuf nbuf = STRBUF_INIT;
 		if (convert_to_git(istate, path, buf, size, &nbuf,
 				   get_conv_flags(flags))) {
-			buf = strbuf_detach(&nbuf, &size);
-			re_allocated = 1;
+			buf = nbuf.buf;
+			size = nbuf.len;
 		}
 	}
 	if (flags & HASH_FORMAT_CHECK) {
@@ -2520,8 +2520,8 @@ static int index_mem(struct index_state *istate,
 		ret = write_object_file(buf, size, type, oid);
 	else
 		hash_object_file(the_hash_algo, buf, size, type, oid);
-	if (re_allocated)
-		free(buf);
+
+	strbuf_release(&nbuf);
 	return ret;
 }
 

From 9c076c32fbb15a0887a1ed4d2afa7e5a3fd74727 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:39 +0200
Subject: [PATCH 14/27] pretty: add casts for decoration option pointers

The `struct decoration_options` have a prefix and suffix field which are
both non-constant, but we assign a constant pointer to them. This is
safe to do because we pass them to `format_decorations()`, which never
modifies these pointers, and then immediately discard the structure. Add
explicit casts to avoid compilation warnings with `-Wwrite-strings`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pretty.c b/pretty.c
index ec05db5655..1df9d635fb 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1584,8 +1584,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	case 'D':
 		{
 			const struct decoration_options opts = {
-				.prefix = "",
-				.suffix = ""
+				.prefix = (char *) "",
+				.suffix = (char *) "",
 			};
 
 			format_decorations(sb, commit, c->auto_color, &opts);

From e7b40195ae0082d04ea8c0d1769d90ea700b76f2 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:44 +0200
Subject: [PATCH 15/27] compat/win32: fix const-correctness with string
 constants

Adjust various places in our Win32 compatibility layer where we are not
assigning string constants to `const char *` variables.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 compat/basename.c | 16 ++++++++++++++--
 compat/mingw.c    | 28 ++++++++++++++++------------
 compat/winansi.c  |  2 +-
 3 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/compat/basename.c b/compat/basename.c
index 96bd9533b4..c33579ef61 100644
--- a/compat/basename.c
+++ b/compat/basename.c
@@ -10,7 +10,13 @@ char *gitbasename (char *path)
 		skip_dos_drive_prefix(&path);
 
 	if (!path || !*path)
-		return ".";
+		/*
+		 * basename(3P) is mis-specified because it returns a
+		 * non-constant pointer even though it is specified to return a
+		 * pointer to internal memory at times. The cast is a result of
+		 * that.
+		 */
+		return (char *) ".";
 
 	for (base = path; *path; path++) {
 		if (!is_dir_sep(*path))
@@ -34,7 +40,13 @@ char *gitdirname(char *path)
 	int dos_drive_prefix;
 
 	if (!p)
-		return ".";
+		/*
+		 * dirname(3P) is mis-specified because it returns a
+		 * non-constant pointer even though it is specified to return a
+		 * pointer to internal memory at times. The cast is a result of
+		 * that.
+		 */
+		return (char *) ".";
 
 	if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)
 		goto dot;
diff --git a/compat/mingw.c b/compat/mingw.c
index 6b06ea540f..d378cd04cb 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2279,7 +2279,11 @@ struct passwd *getpwuid(int uid)
 	p->pw_name = user_name;
 	p->pw_gecos = get_extended_user_info(NameDisplay);
 	if (!p->pw_gecos)
-		p->pw_gecos = "unknown";
+		/*
+		 * Data returned by getpwuid(3P) is treated as internal and
+		 * must never be written to or freed.
+		 */
+		p->pw_gecos = (char *) "unknown";
 	p->pw_dir = NULL;
 
 	initialized = 1;
@@ -2800,16 +2804,16 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
 			strbuf_addf(report, "'%s' is on a file system that does "
 				    "not record ownership\n", path);
 		} else if (report) {
-			LPSTR str1, str2, str3, str4, to_free1 = NULL,
-			    to_free3 = NULL, to_local_free2 = NULL,
-			    to_local_free4 = NULL;
+			PCSTR str1, str2, str3, str4;
+			LPSTR to_free1 = NULL, to_free3 = NULL,
+			    to_local_free2 = NULL, to_local_free4 = NULL;
 
-			if (user_sid_to_user_name(sid, &str1))
-				to_free1 = str1;
+			if (user_sid_to_user_name(sid, &to_free1))
+				str1 = to_free1;
 			else
 				str1 = "(inconvertible)";
-			if (ConvertSidToStringSidA(sid, &str2))
-				to_local_free2 = str2;
+			if (ConvertSidToStringSidA(sid, &to_local_free2))
+				str2 = to_local_free2;
 			else
 				str2 = "(inconvertible)";
 
@@ -2822,13 +2826,13 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
 				str4 = "(invalid)";
 			} else {
 				if (user_sid_to_user_name(current_user_sid,
-							  &str3))
-					to_free3 = str3;
+							  &to_free3))
+					str3 = to_free3;
 				else
 					str3 = "(inconvertible)";
 				if (ConvertSidToStringSidA(current_user_sid,
-							   &str4))
-					to_local_free4 = str4;
+							   &to_local_free4))
+					str4 = to_local_free4;
 				else
 					str4 = "(inconvertible)";
 			}
diff --git a/compat/winansi.c b/compat/winansi.c
index f83610f684..575813bde8 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -139,7 +139,7 @@ static void write_console(unsigned char *str, size_t len)
 	/* convert utf-8 to utf-16 */
 	int wlen = xutftowcsn(wbuf, (char*) str, ARRAY_SIZE(wbuf), len);
 	if (wlen < 0) {
-		wchar_t *err = L"[invalid]";
+		const wchar_t *err = L"[invalid]";
 		WriteConsoleW(console, err, wcslen(err), &dummy, NULL);
 		return;
 	}

From 8d3a7ce441422f4b48f14c19a1b3be4ba7bfd30f Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:49 +0200
Subject: [PATCH 16/27] http: do not assign string constant to non-const field

In `write_accept_language()`, we put all acceptable languages into an
array. While all entries in that array are allocated strings, the final
entry in that array is a string constant. This is fine because we
explicitly skip over the last entry when freeing the array, but will
cause warnings once we enable `-Wwrite-strings`.

Adapt the code to also allocate the final entry.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 http.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/http.c b/http.c
index 67cc47d28f..2dea2d03da 100644
--- a/http.c
+++ b/http.c
@@ -1974,7 +1974,7 @@ static void write_accept_language(struct strbuf *buf)
 
 		/* add '*' */
 		REALLOC_ARRAY(language_tags, num_langs + 1);
-		language_tags[num_langs++] = "*"; /* it's OK; this won't be freed */
+		language_tags[num_langs++] = xstrdup("*");
 
 		/* compute decimal_places */
 		for (max_q = 1, decimal_places = 0;
@@ -2004,8 +2004,7 @@ static void write_accept_language(struct strbuf *buf)
 		}
 	}
 
-	/* free language tags -- last one is a static '*' */
-	for (i = 0; i < num_langs - 1; i++)
+	for (i = 0; i < num_langs; i++)
 		free(language_tags[i]);
 	free(language_tags);
 }

From e463c5e8a0c306b62c25ff5856087ab32403158a Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:54 +0200
Subject: [PATCH 17/27] parse-options: cast long name for OPTION_ALIAS

We assign the long name for OPTION_ALIAS options to a non-constant value
field. We know that the variable will never be written to, but this will
cause warnings once we enable `-Wwrite-strings`.

Cast away the constness to be prepared for this change.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index bd62e20268..ae15342390 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -355,7 +355,7 @@ struct option {
 	.type = OPTION_ALIAS, \
 	.short_name = (s), \
 	.long_name = (l), \
-	.value = (source_long_name), \
+	.value = (char *)(source_long_name), \
 }
 
 #define OPT_SUBCOMMAND_F(l, v, fn, f) { \

From 5bd0851d97be732470eab8cb8d7255c7050ef384 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:38:58 +0200
Subject: [PATCH 18/27] send-pack: always allocate receive status

In `receive_status()`, we record the reason why ref updates have been
rejected by the remote via the `remote_status`. But while we allocate
the assigned string when a reason was given, we assign a string constant
when no reason was given.

This has been working fine so far due to two reasons:

  - We don't ever free the refs in git-send-pack(1)'

  - Remotes always give a reason, at least as implemented by Git proper.

Adapt the code to always allocate the receive status string and free the
refs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/send-pack.c | 2 ++
 send-pack.c         | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 3df9eaad09..17cae6bbbd 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -336,5 +336,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 		/* stable plumbing output; do not modify or localize */
 		fprintf(stderr, "Everything up-to-date\n");
 
+	free_refs(remote_refs);
+	free_refs(local_refs);
 	return ret;
 }
diff --git a/send-pack.c b/send-pack.c
index 37f59d4f66..88e96d000b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -259,7 +259,7 @@ static int receive_status(struct packet_reader *reader, struct ref *refs)
 			if (p)
 				hint->remote_status = xstrdup(p);
 			else
-				hint->remote_status = "failed";
+				hint->remote_status = xstrdup("failed");
 		} else {
 			hint->status = REF_STATUS_OK;
 			hint->remote_status = xstrdup_or_null(p);

From a3da6948c3b6c272bf7a381b86df9a60bdf2b052 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:03 +0200
Subject: [PATCH 19/27] remote-curl: avoid assigning string constant to
 non-const variable

When processing remote options, we split the option line into two by
searching for a space. If there is one, we replace the space with '\0',
otherwise we implicitly assume that the value is "true" and thus assign
a string constant.

As the return value of strchr(3P) weirdly enough is a `char *` even
though it gets a `const char *` as input, the assigned-to variable also
is a non-constant. This is fine though because the argument is in fact
an allocated string, and thus we are allowed to modify it. But this will
break once we enable `-Wwrite-strings`.

Refactor the code stop splitting the fields with '\0' altogether.
Instead, we can pass the length of the option name to `set_option()` and
then use strncmp(3P) instead of strcmp(3P).

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote-curl.c | 53 ++++++++++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index cae98384da..d0f767df8e 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -58,9 +58,9 @@ struct options {
 static struct options options;
 static struct string_list cas_options = STRING_LIST_INIT_DUP;
 
-static int set_option(const char *name, const char *value)
+static int set_option(const char *name, size_t namelen, const char *value)
 {
-	if (!strcmp(name, "verbosity")) {
+	if (!strncmp(name, "verbosity", namelen)) {
 		char *end;
 		int v = strtol(value, &end, 10);
 		if (value == end || *end)
@@ -68,7 +68,7 @@ static int set_option(const char *name, const char *value)
 		options.verbosity = v;
 		return 0;
 	}
-	else if (!strcmp(name, "progress")) {
+	else if (!strncmp(name, "progress", namelen)) {
 		if (!strcmp(value, "true"))
 			options.progress = 1;
 		else if (!strcmp(value, "false"))
@@ -77,7 +77,7 @@ static int set_option(const char *name, const char *value)
 			return -1;
 		return 0;
 	}
-	else if (!strcmp(name, "depth")) {
+	else if (!strncmp(name, "depth", namelen)) {
 		char *end;
 		unsigned long v = strtoul(value, &end, 10);
 		if (value == end || *end)
@@ -85,15 +85,15 @@ static int set_option(const char *name, const char *value)
 		options.depth = v;
 		return 0;
 	}
-	else if (!strcmp(name, "deepen-since")) {
+	else if (!strncmp(name, "deepen-since", namelen)) {
 		options.deepen_since = xstrdup(value);
 		return 0;
 	}
-	else if (!strcmp(name, "deepen-not")) {
+	else if (!strncmp(name, "deepen-not", namelen)) {
 		string_list_append(&options.deepen_not, value);
 		return 0;
 	}
-	else if (!strcmp(name, "deepen-relative")) {
+	else if (!strncmp(name, "deepen-relative", namelen)) {
 		if (!strcmp(value, "true"))
 			options.deepen_relative = 1;
 		else if (!strcmp(value, "false"))
@@ -102,7 +102,7 @@ static int set_option(const char *name, const char *value)
 			return -1;
 		return 0;
 	}
-	else if (!strcmp(name, "followtags")) {
+	else if (!strncmp(name, "followtags", namelen)) {
 		if (!strcmp(value, "true"))
 			options.followtags = 1;
 		else if (!strcmp(value, "false"))
@@ -111,7 +111,7 @@ static int set_option(const char *name, const char *value)
 			return -1;
 		return 0;
 	}
-	else if (!strcmp(name, "dry-run")) {
+	else if (!strncmp(name, "dry-run", namelen)) {
 		if (!strcmp(value, "true"))
 			options.dry_run = 1;
 		else if (!strcmp(value, "false"))
@@ -120,7 +120,7 @@ static int set_option(const char *name, const char *value)
 			return -1;
 		return 0;
 	}
-	else if (!strcmp(name, "check-connectivity")) {
+	else if (!strncmp(name, "check-connectivity", namelen)) {
 		if (!strcmp(value, "true"))
 			options.check_self_contained_and_connected = 1;
 		else if (!strcmp(value, "false"))
@@ -129,7 +129,7 @@ static int set_option(const char *name, const char *value)
 			return -1;
 		return 0;
 	}
-	else if (!strcmp(name, "cas")) {
+	else if (!strncmp(name, "cas", namelen)) {
 		struct strbuf val = STRBUF_INIT;
 		strbuf_addstr(&val, "--force-with-lease=");
 		if (*value != '"')
@@ -139,7 +139,7 @@ static int set_option(const char *name, const char *value)
 		string_list_append(&cas_options, val.buf);
 		strbuf_release(&val);
 		return 0;
-	} else if (!strcmp(name, TRANS_OPT_FORCE_IF_INCLUDES)) {
+	} else if (!strncmp(name, TRANS_OPT_FORCE_IF_INCLUDES, namelen)) {
 		if (!strcmp(value, "true"))
 			options.force_if_includes = 1;
 		else if (!strcmp(value, "false"))
@@ -147,7 +147,7 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
-	} else if (!strcmp(name, "cloning")) {
+	} else if (!strncmp(name, "cloning", namelen)) {
 		if (!strcmp(value, "true"))
 			options.cloning = 1;
 		else if (!strcmp(value, "false"))
@@ -155,7 +155,7 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
-	} else if (!strcmp(name, "update-shallow")) {
+	} else if (!strncmp(name, "update-shallow", namelen)) {
 		if (!strcmp(value, "true"))
 			options.update_shallow = 1;
 		else if (!strcmp(value, "false"))
@@ -163,7 +163,7 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
-	} else if (!strcmp(name, "pushcert")) {
+	} else if (!strncmp(name, "pushcert", namelen)) {
 		if (!strcmp(value, "true"))
 			options.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
 		else if (!strcmp(value, "false"))
@@ -173,7 +173,7 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
-	} else if (!strcmp(name, "atomic")) {
+	} else if (!strncmp(name, "atomic", namelen)) {
 		if (!strcmp(value, "true"))
 			options.atomic = 1;
 		else if (!strcmp(value, "false"))
@@ -181,7 +181,7 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
-	} else if (!strcmp(name, "push-option")) {
+	} else if (!strncmp(name, "push-option", namelen)) {
 		if (*value != '"')
 			string_list_append(&options.push_options, value);
 		else {
@@ -192,7 +192,7 @@ static int set_option(const char *name, const char *value)
 						 strbuf_detach(&unquoted, NULL));
 		}
 		return 0;
-	} else if (!strcmp(name, "family")) {
+	} else if (!strncmp(name, "family", namelen)) {
 		if (!strcmp(value, "ipv4"))
 			git_curl_ipresolve = CURL_IPRESOLVE_V4;
 		else if (!strcmp(value, "ipv6"))
@@ -202,16 +202,16 @@ static int set_option(const char *name, const char *value)
 		else
 			return -1;
 		return 0;
-	} else if (!strcmp(name, "from-promisor")) {
+	} else if (!strncmp(name, "from-promisor", namelen)) {
 		options.from_promisor = 1;
 		return 0;
-	} else if (!strcmp(name, "refetch")) {
+	} else if (!strncmp(name, "refetch", namelen)) {
 		options.refetch = 1;
 		return 0;
-	} else if (!strcmp(name, "filter")) {
+	} else if (!strncmp(name, "filter", namelen)) {
 		options.filter = xstrdup(value);
 		return 0;
-	} else if (!strcmp(name, "object-format")) {
+	} else if (!strncmp(name, "object-format", namelen)) {
 		options.object_format = 1;
 		if (strcmp(value, "true"))
 			die(_("unknown value for object-format: %s"), value);
@@ -1588,15 +1588,16 @@ int cmd_main(int argc, const char **argv)
 			parse_push(&buf);
 
 		} else if (skip_prefix(buf.buf, "option ", &arg)) {
-			char *value = strchr(arg, ' ');
+			const char *value = strchrnul(arg, ' ');
+			size_t arglen = value - arg;
 			int result;
 
-			if (value)
-				*value++ = '\0';
+			if (*value)
+				value++; /* skip over SP */
 			else
 				value = "true";
 
-			result = set_option(arg, value);
+			result = set_option(arg, arglen, value);
 			if (!result)
 				printf("ok\n");
 			else if (result < 0)

From 844d190677216c0754287f14d9474a55adf606a4 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:07 +0200
Subject: [PATCH 20/27] revision: always store allocated strings in output
 encoding

The `git_log_output_encoding` variable can be set via the `--encoding=`
option. When doing so, we conditionally either assign it to the passed
value, or if the value is "none" we assign it the empty string.
Depending on which of the both code paths we pick though, the variable
may end up being assigned either an allocated string or a string
constant.

This is somewhat risky and may easily lead to bugs when a different code
path may want to reassign a new value to it, freeing the previous value.
We already to this when parsing the "i18n.logoutputencoding" config in
`git_default_i18n_config()`. But because the config is typically parsed
before we parse command line options this has been fine so far.

Regardless of that, safeguard the code such that the variable always
contains an allocated string. While at it, also free the old value in
case there was any to plug a potential memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 revision.c             | 3 ++-
 t/t3900-i18n-commit.sh | 1 +
 t/t3901-i18n-patch.sh  | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index 7ddf0f151a..2ee6886078 100644
--- a/revision.c
+++ b/revision.c
@@ -2650,10 +2650,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--invert-grep")) {
 		revs->grep_filter.no_body_match = 1;
 	} else if ((argcount = parse_long_opt("encoding", argv, &optarg))) {
+		free(git_log_output_encoding);
 		if (strcmp(optarg, "none"))
 			git_log_output_encoding = xstrdup(optarg);
 		else
-			git_log_output_encoding = "";
+			git_log_output_encoding = xstrdup("");
 		return argcount;
 	} else if (!strcmp(arg, "--reverse")) {
 		revs->reverse ^= 1;
diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index f27d09cfd9..db7b403bc1 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -5,6 +5,7 @@
 
 test_description='commit and log output encodings'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 compare_with () {
diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index 4b37f78829..5f0b9afc3f 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -8,6 +8,7 @@ test_description='i18n settings and format-patch | am pipe'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 check_encoding () {

From c77756015e66e5fc601cfb6a368c37ef1f94285a Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:12 +0200
Subject: [PATCH 21/27] mailmap: always store allocated strings in mailmap blob

Same as with the preceding commit, the `git_mailmap_blob` may sometimes
contain an allocated string and sometimes it may contain a string
constant. This is risky and can easily lead to bugs in case the variable
is getting re-assigned, where the code may then try to free the previous
value to avoid memory leaks.

Safeguard the code by always storing allocated strings in the variable.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 mailmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mailmap.c b/mailmap.c
index b2efe29b3d..3d1e092fef 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -216,7 +216,7 @@ int read_mailmap(struct string_list *map)
 	map->cmp = namemap_cmp;
 
 	if (!git_mailmap_blob && is_bare_repository())
-		git_mailmap_blob = "HEAD:.mailmap";
+		git_mailmap_blob = xstrdup("HEAD:.mailmap");
 
 	if (!startup_info->have_repository || !is_bare_repository())
 		err |= read_mailmap_file(map, ".mailmap",

From cea1ff7f1fb65daaf1059bdc0e5b65e2a2e3b89b Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:17 +0200
Subject: [PATCH 22/27] imap-send: drop global `imap_server_conf` variable

In "imap-send.c", we have a global `sturct imap_server_conf` variable
that keeps track of the configuration of the IMAP server. This variable
is being populated mostly via the Git configuration.

Refactor the code to allocate the structure on the stack instead of
having it globally. This change allows us to track its lifetime more
closely.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 imap-send.c | 57 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 8b723b34a5..67a7a6c456 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -82,10 +82,6 @@ struct imap_server_conf {
 	char *auth_method;
 };
 
-static struct imap_server_conf server = {
-	.ssl_verify = 1,
-};
-
 struct imap_socket {
 	int fd[2];
 	SSL *ssl;
@@ -110,6 +106,7 @@ struct imap {
 };
 
 struct imap_store {
+	const struct imap_server_conf *cfg;
 	/* currently open mailbox */
 	const char *name; /* foreign! maybe preset? */
 	int uidvalidity;
@@ -194,8 +191,8 @@ static void socket_perror(const char *func, struct imap_socket *sock, int ret)
 
 #ifdef NO_OPENSSL
 static int ssl_socket_connect(struct imap_socket *sock UNUSED,
-			      int use_tls_only UNUSED,
-			      int verify UNUSED)
+			      const struct imap_server_conf *cfg,
+			      int use_tls_only UNUSED)
 {
 	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
 	return -1;
@@ -250,7 +247,9 @@ static int verify_hostname(X509 *cert, const char *hostname)
 		     cname, hostname);
 }
 
-static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int verify)
+static int ssl_socket_connect(struct imap_socket *sock,
+			      const struct imap_server_conf *cfg,
+			      int use_tls_only)
 {
 #if (OPENSSL_VERSION_NUMBER >= 0x10000000L)
 	const SSL_METHOD *meth;
@@ -279,7 +278,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	if (use_tls_only)
 		SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
 
-	if (verify)
+	if (cfg->ssl_verify)
 		SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
 
 	if (!SSL_CTX_set_default_verify_paths(ctx)) {
@@ -306,9 +305,9 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	 * OpenSSL does not document this function, but the implementation
 	 * returns 1 on success, 0 on failure after calling SSLerr().
 	 */
-	ret = SSL_set_tlsext_host_name(sock->ssl, server.host);
+	ret = SSL_set_tlsext_host_name(sock->ssl, cfg->host);
 	if (ret != 1)
-		warning("SSL_set_tlsext_host_name(%s) failed.", server.host);
+		warning("SSL_set_tlsext_host_name(%s) failed.", cfg->host);
 #endif
 
 	ret = SSL_connect(sock->ssl);
@@ -317,12 +316,12 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 		return -1;
 	}
 
-	if (verify) {
+	if (cfg->ssl_verify) {
 		/* make sure the hostname matches that of the certificate */
 		cert = SSL_get_peer_certificate(sock->ssl);
 		if (!cert)
 			return error("unable to get peer certificate.");
-		if (verify_hostname(cert, server.host) < 0)
+		if (verify_hostname(cert, cfg->host) < 0)
 			return -1;
 	}
 
@@ -895,7 +894,7 @@ static int auth_cram_md5(struct imap_store *ctx, const char *prompt)
 	int ret;
 	char *response;
 
-	response = cram(prompt, server.user, server.pass);
+	response = cram(prompt, ctx->cfg->user, ctx->cfg->pass);
 
 	ret = socket_write(&ctx->imap->buf.sock, response, strlen(response));
 	if (ret != strlen(response))
@@ -935,6 +934,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
 
 	CALLOC_ARRAY(ctx, 1);
 
+	ctx->cfg = srvc;
 	ctx->imap = CALLOC_ARRAY(imap, 1);
 	imap->buf.sock.fd[0] = imap->buf.sock.fd[1] = -1;
 	imap->in_progress_append = &imap->in_progress;
@@ -1035,7 +1035,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
 		imap->buf.sock.fd[1] = dup(s);
 
 		if (srvc->use_ssl &&
-		    ssl_socket_connect(&imap->buf.sock, 0, srvc->ssl_verify)) {
+		    ssl_socket_connect(&imap->buf.sock, srvc, 0)) {
 			close(s);
 			goto bail;
 		}
@@ -1068,8 +1068,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
 		if (!srvc->use_ssl && CAP(STARTTLS)) {
 			if (imap_exec(ctx, NULL, "STARTTLS") != RESP_OK)
 				goto bail;
-			if (ssl_socket_connect(&imap->buf.sock, 1,
-					       srvc->ssl_verify))
+			if (ssl_socket_connect(&imap->buf.sock, srvc, 1))
 				goto bail;
 			/* capabilities may have changed, so get the new capabilities */
 			if (imap_exec(ctx, NULL, "CAPABILITY") != RESP_OK)
@@ -1299,23 +1298,24 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
 static int git_imap_config(const char *var, const char *val,
 			   const struct config_context *ctx, void *cb)
 {
+	struct imap_server_conf *cfg = cb;
 
 	if (!strcmp("imap.sslverify", var))
-		server.ssl_verify = git_config_bool(var, val);
+		cfg->ssl_verify = git_config_bool(var, val);
 	else if (!strcmp("imap.preformattedhtml", var))
-		server.use_html = git_config_bool(var, val);
+		cfg->use_html = git_config_bool(var, val);
 	else if (!strcmp("imap.folder", var))
-		return git_config_string(&server.folder, var, val);
+		return git_config_string(&cfg->folder, var, val);
 	else if (!strcmp("imap.user", var))
-		return git_config_string(&server.user, var, val);
+		return git_config_string(&cfg->user, var, val);
 	else if (!strcmp("imap.pass", var))
-		return git_config_string(&server.pass, var, val);
+		return git_config_string(&cfg->pass, var, val);
 	else if (!strcmp("imap.tunnel", var))
-		return git_config_string(&server.tunnel, var, val);
+		return git_config_string(&cfg->tunnel, var, val);
 	else if (!strcmp("imap.authmethod", var))
-		return git_config_string(&server.auth_method, var, val);
+		return git_config_string(&cfg->auth_method, var, val);
 	else if (!strcmp("imap.port", var))
-		server.port = git_config_int(var, val, ctx->kvi);
+		cfg->port = git_config_int(var, val, ctx->kvi);
 	else if (!strcmp("imap.host", var)) {
 		if (!val) {
 			return config_error_nonbool(var);
@@ -1324,11 +1324,11 @@ static int git_imap_config(const char *var, const char *val,
 				val += 5;
 			else if (starts_with(val, "imaps:")) {
 				val += 6;
-				server.use_ssl = 1;
+				cfg->use_ssl = 1;
 			}
 			if (starts_with(val, "//"))
 				val += 2;
-			server.host = xstrdup(val);
+			cfg->host = xstrdup(val);
 		}
 	} else
 		return git_default_config(var, val, ctx, cb);
@@ -1497,12 +1497,15 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,
 
 int cmd_main(int argc, const char **argv)
 {
+	struct imap_server_conf server = {
+		.ssl_verify = 1,
+	};
 	struct strbuf all_msgs = STRBUF_INIT;
 	int total;
 	int nongit_ok;
 
 	setup_git_directory_gently(&nongit_ok);
-	git_config(git_imap_config, NULL);
+	git_config(git_imap_config, &server);
 
 	argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
 

From 6d1f198f346fb218e06e730bac28609aa402a648 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:22 +0200
Subject: [PATCH 23/27] imap-send: fix leaking memory in `imap_server_conf`

We never free any of the config strings that we populate into the
`struct imap_server_conf`. Fix this by creating a common exit path where
we can free resources.

While at it, drop the unused member `imap_server_conf::name`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 imap-send.c | 63 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 67a7a6c456..da3e7ec17e 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -69,7 +69,6 @@ static void imap_warn(const char *, ...);
 static char *next_arg(char **);
 
 struct imap_server_conf {
-	const char *name;
 	char *tunnel;
 	char *host;
 	int port;
@@ -1300,23 +1299,28 @@ static int git_imap_config(const char *var, const char *val,
 {
 	struct imap_server_conf *cfg = cb;
 
-	if (!strcmp("imap.sslverify", var))
+	if (!strcmp("imap.sslverify", var)) {
 		cfg->ssl_verify = git_config_bool(var, val);
-	else if (!strcmp("imap.preformattedhtml", var))
+	} else if (!strcmp("imap.preformattedhtml", var)) {
 		cfg->use_html = git_config_bool(var, val);
-	else if (!strcmp("imap.folder", var))
+	} else if (!strcmp("imap.folder", var)) {
+		FREE_AND_NULL(cfg->folder);
 		return git_config_string(&cfg->folder, var, val);
-	else if (!strcmp("imap.user", var))
+	} else if (!strcmp("imap.user", var)) {
+		FREE_AND_NULL(cfg->folder);
 		return git_config_string(&cfg->user, var, val);
-	else if (!strcmp("imap.pass", var))
+	} else if (!strcmp("imap.pass", var)) {
+		FREE_AND_NULL(cfg->folder);
 		return git_config_string(&cfg->pass, var, val);
-	else if (!strcmp("imap.tunnel", var))
+	} else if (!strcmp("imap.tunnel", var)) {
+		FREE_AND_NULL(cfg->folder);
 		return git_config_string(&cfg->tunnel, var, val);
-	else if (!strcmp("imap.authmethod", var))
+	} else if (!strcmp("imap.authmethod", var)) {
+		FREE_AND_NULL(cfg->folder);
 		return git_config_string(&cfg->auth_method, var, val);
-	else if (!strcmp("imap.port", var))
+	} else if (!strcmp("imap.port", var)) {
 		cfg->port = git_config_int(var, val, ctx->kvi);
-	else if (!strcmp("imap.host", var)) {
+	} else if (!strcmp("imap.host", var)) {
 		if (!val) {
 			return config_error_nonbool(var);
 		} else {
@@ -1330,8 +1334,9 @@ static int git_imap_config(const char *var, const char *val,
 				val += 2;
 			cfg->host = xstrdup(val);
 		}
-	} else
+	} else {
 		return git_default_config(var, val, ctx, cb);
+	}
 
 	return 0;
 }
@@ -1503,6 +1508,7 @@ int cmd_main(int argc, const char **argv)
 	struct strbuf all_msgs = STRBUF_INIT;
 	int total;
 	int nongit_ok;
+	int ret;
 
 	setup_git_directory_gently(&nongit_ok);
 	git_config(git_imap_config, &server);
@@ -1529,42 +1535,55 @@ int cmd_main(int argc, const char **argv)
 
 	if (!server.folder) {
 		fprintf(stderr, "no imap store specified\n");
-		return 1;
+		ret = 1;
+		goto out;
 	}
 	if (!server.host) {
 		if (!server.tunnel) {
 			fprintf(stderr, "no imap host specified\n");
-			return 1;
+			ret = 1;
+			goto out;
 		}
-		server.host = "tunnel";
+		server.host = xstrdup("tunnel");
 	}
 
 	/* read the messages */
 	if (strbuf_read(&all_msgs, 0, 0) < 0) {
 		error_errno(_("could not read from stdin"));
-		return 1;
+		ret = 1;
+		goto out;
 	}
 
 	if (all_msgs.len == 0) {
 		fprintf(stderr, "nothing to send\n");
-		return 1;
+		ret = 1;
+		goto out;
 	}
 
 	total = count_messages(&all_msgs);
 	if (!total) {
 		fprintf(stderr, "no messages to send\n");
-		return 1;
+		ret = 1;
+		goto out;
 	}
 
 	/* write it to the imap server */
 
 	if (server.tunnel)
-		return append_msgs_to_imap(&server, &all_msgs, total);
-
+		ret = append_msgs_to_imap(&server, &all_msgs, total);
 #ifdef USE_CURL_FOR_IMAP_SEND
-	if (use_curl)
-		return curl_append_msgs_to_imap(&server, &all_msgs, total);
+	else if (use_curl)
+		ret = curl_append_msgs_to_imap(&server, &all_msgs, total);
 #endif
+	else
+		ret = append_msgs_to_imap(&server, &all_msgs, total);
 
-	return append_msgs_to_imap(&server, &all_msgs, total);
+out:
+	free(server.tunnel);
+	free(server.host);
+	free(server.folder);
+	free(server.user);
+	free(server.pass);
+	free(server.auth_method);
+	return ret;
 }

From 25a47ffac0c50d3bae4058e6327c7bd799240df9 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:26 +0200
Subject: [PATCH 24/27] builtin/rebase: do not assign default backend to
 non-constant field

The `struct rebase_options::default_backend` field is a non-constant
string, but is being assigned a constant via `REBASE_OPTIONS_INIT`.
Fix this by using `xstrdup()` to assign the variable and introduce a new
function `rebase_options_release()` that releases memory held by the
structure, including the newly-allocated variable.

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

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 14d4f0a5e6..adc990b55e 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -135,7 +135,7 @@ struct rebase_options {
 		.type = REBASE_UNSPECIFIED,	  	\
 		.empty = EMPTY_UNSPECIFIED,	  	\
 		.keep_empty = 1,			\
-		.default_backend = "merge",	  	\
+		.default_backend = xstrdup("merge"),  	\
 		.flags = REBASE_NO_QUIET, 		\
 		.git_am_opts = STRVEC_INIT,		\
 		.exec = STRING_LIST_INIT_NODUP,		\
@@ -151,6 +151,19 @@ struct rebase_options {
 		.strategy_opts = STRING_LIST_INIT_NODUP,\
 	}
 
+static void rebase_options_release(struct rebase_options *opts)
+{
+	free(opts->default_backend);
+	free(opts->reflog_action);
+	free(opts->head_name);
+	strvec_clear(&opts->git_am_opts);
+	free(opts->gpg_sign_opt);
+	string_list_clear(&opts->exec, 0);
+	free(opts->strategy);
+	string_list_clear(&opts->strategy_opts, 0);
+	strbuf_release(&opts->git_format_patch_opt);
+}
+
 static struct replay_opts get_replay_opts(const struct rebase_options *opts)
 {
 	struct replay_opts replay = REPLAY_OPTS_INIT;
@@ -796,6 +809,7 @@ static int rebase_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "rebase.backend")) {
+		FREE_AND_NULL(opts->default_backend);
 		return git_config_string(&opts->default_backend, var, value);
 	}
 
@@ -1833,14 +1847,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 cleanup:
 	strbuf_release(&buf);
 	strbuf_release(&revisions);
-	free(options.reflog_action);
-	free(options.head_name);
-	strvec_clear(&options.git_am_opts);
-	free(options.gpg_sign_opt);
-	string_list_clear(&options.exec, 0);
-	free(options.strategy);
-	string_list_clear(&options.strategy_opts, 0);
-	strbuf_release(&options.git_format_patch_opt);
+	rebase_options_release(&options);
 	free(squash_onto_name);
 	free(keep_base_onto_name);
 	return !!ret;

From fc066767662ead1b279a20545ff02ab17d835f34 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:31 +0200
Subject: [PATCH 25/27] builtin/rebase: always store allocated string in
 `options.strategy`

The `struct rebase_options::strategy` field is a `char *`, but we do end
up assigning string constants to it in two cases:

  - When being passed a `--strategy=` option via the command line.

  - When being passed a strategy option via `--strategy-option=`, but
    not a strategy.

This will cause warnings once we enable `-Wwrite-strings`.

Ideally, we'd just convert the field to be a `const char *`. But we also
assign to this field via the GIT_TEST_MERGE_ALGORITHM envvar, which we
have to strdup(3P) into it.

Instead, refactor the code to make sure that we only ever assign
allocated strings to this field.

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

diff --git a/builtin/rebase.c b/builtin/rebase.c
index adc990b55e..4506bae768 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1061,6 +1061,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 {
 	struct rebase_options options = REBASE_OPTIONS_INIT;
 	const char *branch_name;
+	const char *strategy_opt = NULL;
 	int ret, flags, total_argc, in_progress = 0;
 	int keep_base = 0;
 	int ok_to_skip_pre_rebase = 0;
@@ -1175,7 +1176,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			PARSE_OPT_OPTARG, parse_opt_rebase_merges),
 		OPT_BOOL(0, "fork-point", &options.fork_point,
 			 N_("use 'merge-base --fork-point' to refine upstream")),
-		OPT_STRING('s', "strategy", &options.strategy,
+		OPT_STRING('s', "strategy", &strategy_opt,
 			   N_("strategy"), N_("use the given merge strategy")),
 		OPT_STRING_LIST('X', "strategy-option", &options.strategy_opts,
 				N_("option"),
@@ -1484,13 +1485,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	if (options.strategy_opts.nr && !options.strategy)
-		options.strategy = "ort";
-
-	if (options.strategy) {
-		options.strategy = xstrdup(options.strategy);
+	if (strategy_opt)
+		options.strategy = xstrdup(strategy_opt);
+	else if (options.strategy_opts.nr && !options.strategy)
+		options.strategy = xstrdup("ort");
+	if (options.strategy)
 		imply_merge(&options, "--strategy");
-	}
 
 	if (options.root && !options.onto_name)
 		imply_merge(&options, "--root without --onto");

From 71e01a0ebd50fed1d494e1b05374ec0977437248 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:35 +0200
Subject: [PATCH 26/27] builtin/merge: always store allocated strings in
 `pull_twohead`

The `pull_twohead` configuration may sometimes contain an allocated
string, and sometimes it may contain a string constant. Refactor this to
instead always store an allocated string such that we can release its
resources without risk.

While at it, manage the lifetime of other config strings, as well. Note
that we explicitly don't free `cleanup_arg` here. This is because the
variable may be assigned a string constant via command line options.

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

diff --git a/builtin/merge.c b/builtin/merge.c
index daed2d4e1e..fb3eb15b89 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -611,17 +611,19 @@ static int git_merge_config(const char *k, const char *v,
 		return 0;
 	}
 
-	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
+	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat")) {
 		show_diffstat = git_config_bool(k, v);
-	else if (!strcmp(k, "merge.verifysignatures"))
+	} else if (!strcmp(k, "merge.verifysignatures")) {
 		verify_signatures = git_config_bool(k, v);
-	else if (!strcmp(k, "pull.twohead"))
+	} else if (!strcmp(k, "pull.twohead")) {
+		FREE_AND_NULL(pull_twohead);
 		return git_config_string(&pull_twohead, k, v);
-	else if (!strcmp(k, "pull.octopus"))
+	} else if (!strcmp(k, "pull.octopus")) {
+		FREE_AND_NULL(pull_octopus);
 		return git_config_string(&pull_octopus, k, v);
-	else if (!strcmp(k, "commit.cleanup"))
+	} else if (!strcmp(k, "commit.cleanup")) {
 		return git_config_string(&cleanup_arg, k, v);
-	else if (!strcmp(k, "merge.ff")) {
+	} else if (!strcmp(k, "merge.ff")) {
 		int boolval = git_parse_maybe_bool(v);
 		if (0 <= boolval) {
 			fast_forward = boolval ? FF_ALLOW : FF_NO;
@@ -1294,7 +1296,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (!pull_twohead) {
 		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
 		if (default_strategy && !strcmp(default_strategy, "ort"))
-			pull_twohead = "ort";
+			pull_twohead = xstrdup("ort");
 	}
 
 	init_diff_ui_defaults();
@@ -1793,6 +1795,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	}
 	strbuf_release(&buf);
 	free(branch_to_free);
+	free(pull_twohead);
+	free(pull_octopus);
 	discard_index(the_repository->index);
 	return ret;
 }

From d66fe0726bfb3fb1e3665f7e64b160440007d98e Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 7 Jun 2024 08:39:40 +0200
Subject: [PATCH 27/27] config.mak.dev: enable `-Wwrite-strings` warning

Writing to string constants is undefined behaviour and must be avoided
in C. Even so, the compiler does not help us with this by default
because those constants are not in fact marked as `const`. This makes it
rather easy to accidentally assign a constant to a non-const variable or
field and then later on try to either free it or write to it.

Enable `-Wwrite-strings` to catch such mistakes. With this warning
enabled, the type of string constants is changed to `const char[]` and
will thus cause compiler warnings when being assigned to non-const
fields and variables.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 config.mak.dev | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.dev b/config.mak.dev
index 981304727c..1ce4c70613 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -37,6 +37,7 @@ DEVELOPER_CFLAGS += -Wpointer-arith
 DEVELOPER_CFLAGS += -Wstrict-prototypes
 DEVELOPER_CFLAGS += -Wunused
 DEVELOPER_CFLAGS += -Wvla
+DEVELOPER_CFLAGS += -Wwrite-strings
 DEVELOPER_CFLAGS += -fno-common
 
 ifneq ($(filter clang4,$(COMPILER_FEATURES)),)