{"thread":{"id":"65361","subject":"[PATCH 5/6] do not discard const: keep signature","startedAt":"2026-03-26T15:23:03Z","lastAt":"2026-03-27T17:54:30Z","messageCount":24,"participants":["Michael J Gruber","D. Ben Knoble","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"540093","messageId":"9a90f93111ec54e5eb9675cb84ac1d70ad95e118.1774537954.git.git@grubix.eu","threadId":"65361","inReplyTo":"cover.1774537954.git.git@grubix.eu","subject":"[PATCH 5/6] do not discard const: keep signature","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-26T15:22:51Z","receivedAt":"2026-03-26T15:23:03Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Here, while we do not mutate the struct itself, many other signatures\nexpect a non-const argument - possibly unnecessarily - so we opt to keep\nthe original signature by casting to non-const.\n---\n pseudo-merge.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex a2d5bd85f9..ac81792e65 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -644,7 +644,7 @@ static struct pseudo_merge_commit *find_pseudo_merge(const struct pseudo_merge_m\n \tif (!pm->commits_nr)\n \t\treturn NULL;\n \n-\treturn bsearch(&pos, pm->commits, pm->commits_nr,\n+\treturn (struct pseudo_merge_commit *) bsearch(&pos, pm->commits, pm->commits_nr,\n \t\t       PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp);\n }\n \n-- \n2.53.0.1195.g771ffcb452\n\n"},{"id":"540094","messageId":"fe9c86af4825a81b2618ae8ffc8be12300058af2.1774537954.git.git@grubix.eu","threadId":"65361","inReplyTo":"cover.1774537954.git.git@grubix.eu","subject":"[PATCH 6/6] do not discard const: the ugly truth","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-26T15:22:52Z","receivedAt":"2026-03-26T15:23:03Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"ISOC23 reveals that we mutate argv strings in place. Confess to this\nwith explicit casts.\n---\n builtin/rev-parse.c | 8 ++++----\n revision.c          | 8 ++++----\n 2 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 01a62800e8..f429793b6f 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -265,7 +265,7 @@ static int show_file(const char *arg, int output_prefix)\n \treturn 0;\n }\n \n-static int try_difference(const char *arg)\n+static int try_difference(char *arg)\n {\n \tchar *dotdot;\n \tstruct object_id start_oid;\n@@ -325,7 +325,7 @@ static int try_difference(const char *arg)\n \treturn 0;\n }\n \n-static int try_parent_shorthands(const char *arg)\n+static int try_parent_shorthands(char *arg)\n {\n \tchar *dotdot;\n \tstruct object_id oid;\n@@ -1145,9 +1145,9 @@ int cmd_rev_parse(int argc,\n \t\t}\n \n \t\t/* Not a flag argument */\n-\t\tif (try_difference(arg))\n+\t\tif (try_difference((char *) arg))\n \t\t\tcontinue;\n-\t\tif (try_parent_shorthands(arg))\n+\t\tif (try_parent_shorthands((char *) arg))\n \t\t\tcontinue;\n \t\tname = arg;\n \t\ttype = NORMAL;\ndiff --git a/revision.c b/revision.c\nindex 31808e3df0..a28b14a2ea 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2132,7 +2132,7 @@ static int handle_dotdot(const char *arg,\n \t\t\t int cant_be_filename)\n {\n \tstruct object_context a_oc = {0}, b_oc = {0};\n-\tchar *dotdot = strstr(arg, \"..\");\n+\tchar *dotdot = (char *) strstr(arg, \"..\");\n \tint ret;\n \n \tif (!dotdot)\n@@ -2176,7 +2176,7 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl\n \t\tgoto out;\n \t}\n \n-\tmark = strstr(arg, \"^@\");\n+\tmark = (char *) strstr(arg, \"^@\");\n \tif (mark && !mark[2]) {\n \t\t*mark = 0;\n \t\tif (add_parents_only(revs, arg, flags, 0)) {\n@@ -2185,13 +2185,13 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl\n \t\t}\n \t\t*mark = '^';\n \t}\n-\tmark = strstr(arg, \"^!\");\n+\tmark = (char *) strstr(arg, \"^!\");\n \tif (mark && !mark[2]) {\n \t\t*mark = 0;\n \t\tif (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), 0))\n \t\t\t*mark = '^';\n \t}\n-\tmark = strstr(arg, \"^-\");\n+\tmark = (char *) strstr(arg, \"^-\");\n \tif (mark) {\n \t\tint exclude_parent = 1;\n \n-- \n2.53.0.1195.g771ffcb452\n\n"},{"id":"540095","messageId":"a3a1d2759a0ec5a3ee285689832832e5e3a63768.1774537954.git.git@grubix.eu","threadId":"65361","inReplyTo":"cover.1774537954.git.git@grubix.eu","subject":"[PATCH 1/6] do not discard const: the simple cases","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-26T15:22:47Z","receivedAt":"2026-03-26T15:29:25Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Depending on glibc version and compiler flags (ISOC23), strchr() and\nfriends from string.h may return const pointers for const arguments and\nnon-const for non-const (rather than non-const for all argument types).\nIn particular, our current code base gives warnings such as:\n\n```\nbuiltin/config.c: In function 'get_urlmatch':\nbuiltin/config.c:855:22: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]\n  855 |         section_tail = strchr(config.section, '.');\n      |                      ^\n```\n\nNote that with\n```\nconst char *foo;\nchar *bar;\n```\nwe can always assign `foo = bar` but `bar = foo` throws said warning. In\nparticular, we often pass in `char *` for a `const char *` argument and\nexpect `char *` back but get a `const char *` in said scenario.\n\nThis patch covers the easy cases where we deal with a non-const pointer\nto begin with. It is solved by the cast `bar = (char *) foo`.\n---\n builtin/config.c       | 2 +-\n builtin/receive-pack.c | 6 +++---\n http.c                 | 2 +-\n pager.c                | 2 +-\n range-diff.c           | 2 +-\n refs/files-backend.c   | 2 +-\n remote.c               | 2 +-\n send-pack.c            | 6 +++---\n transport-helper.c     | 2 +-\n 9 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 7c4857be62..bd277e5911 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -852,7 +852,7 @@ static int get_urlmatch(const struct config_location_options *opts,\n \t\tdie(\"%s\", config.url.err);\n \n \tconfig.section = xstrdup_tolower(var);\n-\tsection_tail = strchr(config.section, '.');\n+\tsection_tail = (char *) strchr(config.section, '.');\n \tif (section_tail) {\n \t\t*section_tail = '\\0';\n \t\tconfig.key = section_tail + 1;\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e34edff406..7712a1c3c2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1042,7 +1042,7 @@ static int read_proc_receive_report(struct packet_reader *reader,\n \t\tresponse++;\n \n \t\thead = reader->line;\n-\t\tp = strchr(head, ' ');\n+\t\tp = (char *) strchr(head, ' ');\n \t\tif (!p) {\n \t\t\tstrbuf_addf(errmsg, \"proc-receive reported incomplete status line: '%s'\\n\", head);\n \t\t\tcode = -1;\n@@ -1072,7 +1072,7 @@ static int read_proc_receive_report(struct packet_reader *reader,\n \t\t\t\tnew_report = 0;\n \t\t\t}\n \t\t\tkey = p;\n-\t\t\tp = strchr(key, ' ');\n+\t\t\tp = (char *) strchr(key, ' ');\n \t\t\tif (p)\n \t\t\t\t*p++ = '\\0';\n \t\t\tval = p;\n@@ -1095,7 +1095,7 @@ static int read_proc_receive_report(struct packet_reader *reader,\n \t\treport = NULL;\n \t\tnew_report = 0;\n \t\trefname = p;\n-\t\tp = strchr(refname, ' ');\n+\t\tp = (char *) strchr(refname, ' ');\n \t\tif (p)\n \t\t\t*p++ = '\\0';\n \t\tif (strcmp(head, \"ok\") && strcmp(head, \"ng\")) {\ndiff --git a/http.c b/http.c\nindex d8d016891b..02c4fbb234 100644\n--- a/http.c\n+++ b/http.c\n@@ -774,7 +774,7 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset)\n \n \t\twhile (cookie) {\n \t\t\tchar *equals;\n-\t\t\tchar *semicolon = strstr(cookie, \"; \");\n+\t\t\tchar *semicolon = (char *) strstr(cookie, \"; \");\n \t\t\tif (semicolon)\n \t\t\t\t*semicolon = 0;\n \t\t\tequals = strchrnul(cookie, '=');\ndiff --git a/pager.c b/pager.c\nindex 5531fff50e..eb7011bfde 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -118,7 +118,7 @@ static void setup_pager_env(struct strvec *env)\n \t\t\tsplit_cmdline_strerror(n));\n \n \tfor (i = 0; i < n; i++) {\n-\t\tchar *cp = strchr(argv[i], '=');\n+\t\tchar *cp = (char *) strchr(argv[i], '=');\n \n \t\tif (!cp)\n \t\t\tdie(\"malformed build-time PAGER_ENV\");\ndiff --git a/range-diff.c b/range-diff.c\nindex 2712a9a107..47e36a391f 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -106,7 +106,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\t\tstrbuf_reset(&buf);\n \t\t\t}\n \t\t\tCALLOC_ARRAY(util, 1);\n-\t\t\tif (include_merges && (q = strstr(p, \" (from \")))\n+\t\t\tif (include_merges && (q = (char *) strstr(p, \" (from \")))\n \t\t\t\t*q = '\\0';\n \t\t\tif (repo_get_oid(the_repository, p, &util->oid)) {\n \t\t\t\terror(_(\"could not parse commit '%s'\"), p);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0537a72b2a..71cab7e003 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2196,7 +2196,7 @@ static int show_one_reflog_ent(struct files_ref_store *refs,\n \tif (!sb->len || sb->buf[sb->len - 1] != '\\n' ||\n \t    parse_oid_hex_algop(p, &ooid, &p, refs->base.repo->hash_algo) || *p++ != ' ' ||\n \t    parse_oid_hex_algop(p, &noid, &p, refs->base.repo->hash_algo) || *p++ != ' ' ||\n-\t    !(email_end = strchr(p, '>')) ||\n+\t    !(email_end = (char *) strchr(p, '>')) ||\n \t    email_end[1] != ' ' ||\n \t    !(timestamp = parse_timestamp(email_end + 2, &message, 10)) ||\n \t    !message || message[0] != ' ' ||\ndiff --git a/remote.c b/remote.c\nindex 7ca2a6501b..d7a37016b5 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2861,7 +2861,7 @@ void remote_state_clear(struct remote_state *remote_state)\n  */\n static int chop_last_dir(char **remoteurl, int is_relative)\n {\n-\tchar *rfind = find_last_dir_sep(*remoteurl);\n+\tchar *rfind = (char *) find_last_dir_sep(*remoteurl);\n \tif (rfind) {\n \t\t*rfind = '\\0';\n \t\treturn 0;\ndiff --git a/send-pack.c b/send-pack.c\nindex 07ecfae4de..8b9f7e2f2f 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -181,7 +181,7 @@ static int receive_status(struct repository *r,\n \t\tif (packet_reader_read(reader) != PACKET_READ_NORMAL)\n \t\t\tbreak;\n \t\thead = reader->line;\n-\t\tp = strchr(head, ' ');\n+\t\tp = (char *) strchr(head, ' ');\n \t\tif (!p) {\n \t\t\terror(\"invalid status line from remote: %s\", reader->line);\n \t\t\tret = -1;\n@@ -212,7 +212,7 @@ static int receive_status(struct repository *r,\n \t\t\t\tnew_report = 0;\n \t\t\t}\n \t\t\tkey = p;\n-\t\t\tp = strchr(key, ' ');\n+\t\t\tp = (char *) strchr(key, ' ');\n \t\t\tif (p)\n \t\t\t\t*p++ = '\\0';\n \t\t\tval = p;\n@@ -237,7 +237,7 @@ static int receive_status(struct repository *r,\n \t\t\tbreak;\n \t\t}\n \t\trefname = p;\n-\t\tp = strchr(refname, ' ');\n+\t\tp = (char *) strchr(refname, ' ');\n \t\tif (p)\n \t\t\t*p++ = '\\0';\n \t\t/* first try searching at our hint, falling back to all refs */\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 4d95d84f9e..e7f2cb1812 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -800,7 +800,7 @@ static int push_update_ref_status(struct strbuf *buf,\n \t\t\tstate->new_report = 0;\n \t\t}\n \t\tkey = buf->buf + 7;\n-\t\tp = strchr(key, ' ');\n+\t\tp = (char *) strchr(key, ' ');\n \t\tif (p)\n \t\t\t*p++ = '\\0';\n \t\tval = p;\n-- \n2.53.0.1195.g771ffcb452\n\n"},{"id":"540096","messageId":"5325f5cfb25765252c2aa197b239cd99a72d3f28.1774537954.git.git@grubix.eu","threadId":"65361","inReplyTo":"cover.1774537954.git.git@grubix.eu","subject":"[PATCH 4/6] do not discard const: declare const where we stay const","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-26T15:22:50Z","receivedAt":"2026-03-26T15:29:29Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"This may sound like the easiest case, but for non ISOC23 with non-const\nstrchr() this involves an implicit cast to const.\n---\n convert.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/convert.c b/convert.c\nindex a34ec6ecdc..eae36c8a59 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1168,7 +1168,8 @@ static int ident_to_worktree(const char *src, size_t len,\n \t\t\t     struct strbuf *buf, int ident)\n {\n \tstruct object_id oid;\n-\tchar *to_free = NULL, *dollar, *spc;\n+\tchar *to_free = NULL;\n+\tconst char *dollar, *spc;\n \tint cnt;\n \n \tif (!ident)\n-- \n2.53.0.1195.g771ffcb452\n\n"},{"id":"540097","messageId":"cfea3c6f006f926319da79bb7d97d57fb3b580e9.1774537954.git.git@grubix.eu","threadId":"65361","inReplyTo":"cover.1774537954.git.git@grubix.eu","subject":"[PATCH 2/6] do not discard const: make git-compat-util ISOC23-like","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-26T15:22:48Z","receivedAt":"2026-03-26T15:29:29Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"find_last_dir() should and can return a const pointer. This change fixes\nthe warnings with ISOC23 for git-compat-util and - via explicit casts -\nmakes it clear where we mutate the returned memory.\n---\n git-compat-util.h | 2 +-\n scalar.c          | 2 +-\n submodule.c       | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 4b4ea2498f..3c3dbe298c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -335,7 +335,7 @@ static inline int is_path_owned_by_current_uid(const char *path,\n #endif\n \n #ifndef find_last_dir_sep\n-static inline char *git_find_last_dir_sep(const char *path)\n+static inline const char *git_find_last_dir_sep(const char *path)\n {\n \treturn strrchr(path, '/');\n }\ndiff --git a/scalar.c b/scalar.c\nindex 4efb6ac36d..44f432d7f0 100644\n--- a/scalar.c\n+++ b/scalar.c\n@@ -479,7 +479,7 @@ static int cmd_clone(int argc, const char **argv)\n \t\t/* Strip suffix `.git`, if any */\n \t\tstrbuf_strip_suffix(&buf, \".git\");\n \n-\t\tenlistment = find_last_dir_sep(buf.buf);\n+\t\tenlistment = (char *) find_last_dir_sep(buf.buf);\n \t\tif (!enlistment) {\n \t\t\tdie(_(\"cannot deduce worktree name from '%s'\"), url);\n \t\t}\ndiff --git a/submodule.c b/submodule.c\nindex b1a0363f9d..57933386bc 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2268,7 +2268,7 @@ static int check_casefolding_conflict(const char *git_dir,\n \tDIR *dir = NULL;\n \tint ret = 0;\n \n-\tif ((p = find_last_dir_sep(modules_dir)))\n+\tif ((p = (char *) find_last_dir_sep(modules_dir)))\n \t\t*p = '\\0';\n \n \t/* No conflict is possible if modules_dir doesn't exist (first clone) */\n-- \n2.53.0.1195.g771ffcb452\n\n"},{"id":"540098","messageId":"cover.1774537954.git.git@grubix.eu","threadId":"65361","inReplyTo":null,"subject":"[PATCH 0/6] ISOC23: quell warnings on discarding const","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-26T15:22:46Z","receivedAt":"2026-03-26T15:29:29Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Hi there\n\nFedora 44 beta (gcc-16.0.1, glibc-2.43) brought some fun new warnings\nwhen building git. In essence, we're not always explicit about\nconst-ness or lack thereof of certain pointers. Before, strchr()'s\nsignature which turns const arguments into non-const return values\ncovered this up. With ISOC23, strchr() and friends return const\npointers.\n\nThis little series takes a middle-ground: no new data types (no new\nconst versions of non-const data types) but more explicit casts.\n\nMichael J Gruber (6):\n  do not discard const: the simple cases\n  do not discard const: make git-compat-util ISOC23-like\n  do not discard const: adjust to non-const data types\n  do not discard const: declare const where we stay const\n  do not discard const: keep signature\n  do not discard const: the ugly truth\n\n builtin/config.c       | 2 +-\n builtin/receive-pack.c | 6 +++---\n builtin/rev-parse.c    | 8 ++++----\n convert.c              | 3 ++-\n git-compat-util.h      | 2 +-\n http-push.c            | 2 +-\n http.c                 | 2 +-\n pager.c                | 2 +-\n pseudo-merge.c         | 2 +-\n range-diff.c           | 2 +-\n refs/files-backend.c   | 2 +-\n remote.c               | 2 +-\n revision.c             | 8 ++++----\n run-command.c          | 2 +-\n scalar.c               | 2 +-\n send-pack.c            | 6 +++---\n submodule.c            | 2 +-\n transport-helper.c     | 2 +-\n 18 files changed, 29 insertions(+), 28 deletions(-)\n\n-- \n2.53.0.1195.g771ffcb452\n\n"},{"id":"540099","messageId":"8a65ada967b6b1308ea4cffca82102d4de8e9dd9.1774537954.git.git@grubix.eu","threadId":"65361","inReplyTo":"cover.1774537954.git.git@grubix.eu","subject":"[PATCH 3/6] do not discard const: adjust to non-const data types","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-26T15:22:49Z","receivedAt":"2026-03-26T15:29:29Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"We use data types (such as string_list's util member) which are not\nnecessarily \"non-const in practice\" (such as the list of environment\nvariables in run-command.c) but are not declared \"const\". Rather than\nduplicating data types (e.g. with a new constr_string_list), discard the\nconst explicitly for now to quell ISOC23 warnings.\n---\n http-push.c   | 2 +-\n run-command.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 9ae6062198..acc7f1d8fa 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1772,7 +1772,7 @@ int cmd_main(int argc, const char **argv)\n \t\t\tstr_end_url_with_slash(arg, &repo->url);\n \t\t\trepo->path_len = strlen(repo->url);\n \t\t\tif (path) {\n-\t\t\t\trepo->path = strchr(path+2, '/');\n+\t\t\t\trepo->path = (char *) strchr(path+2, '/');\n \t\t\t\tif (repo->path)\n \t\t\t\t\trepo->path_len = strlen(repo->path);\n \t\t\t}\ndiff --git a/run-command.c b/run-command.c\nindex 32c290ee6a..1db02ef030 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -604,7 +604,7 @@ static void trace_add_env(struct strbuf *dst, const char *const *deltaenv)\n \t/* Last one wins, see run-command.c:prep_childenv() for context */\n \tfor (e = deltaenv; e && *e; e++) {\n \t\tstruct strbuf key = STRBUF_INIT;\n-\t\tchar *equals = strchr(*e, '=');\n+\t\tchar *equals = (char *) strchr(*e, '=');\n \n \t\tif (equals) {\n \t\t\tstrbuf_add(&key, *e, equals - *e);\n-- \n2.53.0.1195.g771ffcb452\n\n"},{"id":"540103","messageId":"CALnO6CA0ZfzAk8FU7xOYAW-emLwdVJ9Ed7Vt-77gfuY97FR=1A@mail.gmail.com","threadId":"65361","inReplyTo":"cover.1774537954.git.git@grubix.eu","subject":"Re: [PATCH 0/6] ISOC23: quell warnings on discarding const","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-03-26T16:26:35Z","receivedAt":"2026-03-26T16:26:46Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Thu, Mar 26, 2026 at 11:40 AM Michael J Gruber <git@grubix.eu> wrote:\n>\n> Hi there\n>\n> Fedora 44 beta (gcc-16.0.1, glibc-2.43) brought some fun new warnings\n> when building git. In essence, we're not always explicit about\n> const-ness or lack thereof of certain pointers. Before, strchr()'s\n> signature which turns const arguments into non-const return values\n> covered this up. With ISOC23, strchr() and friends return const\n> pointers.\n>\n> This little series takes a middle-ground: no new data types (no new\n> const versions of non-const data types) but more explicit casts.\n\nI think a few folks were working on similar things; hopefully I've\nCC'd some relevant parties.\n\n>\n> Michael J Gruber (6):\n>   do not discard const: the simple cases\n>   do not discard const: make git-compat-util ISOC23-like\n>   do not discard const: adjust to non-const data types\n>   do not discard const: declare const where we stay const\n>   do not discard const: keep signature\n>   do not discard const: the ugly truth\n>\n>  builtin/config.c       | 2 +-\n>  builtin/receive-pack.c | 6 +++---\n>  builtin/rev-parse.c    | 8 ++++----\n>  convert.c              | 3 ++-\n>  git-compat-util.h      | 2 +-\n>  http-push.c            | 2 +-\n>  http.c                 | 2 +-\n>  pager.c                | 2 +-\n>  pseudo-merge.c         | 2 +-\n>  range-diff.c           | 2 +-\n>  refs/files-backend.c   | 2 +-\n>  remote.c               | 2 +-\n>  revision.c             | 8 ++++----\n>  run-command.c          | 2 +-\n>  scalar.c               | 2 +-\n>  send-pack.c            | 6 +++---\n>  submodule.c            | 2 +-\n>  transport-helper.c     | 2 +-\n>  18 files changed, 29 insertions(+), 28 deletions(-)\n>\n> --\n> 2.53.0.1195.g771ffcb452\n\n-- \nD. Ben Knoble\n"},{"id":"540106","messageId":"xmqqfr5mr028.fsf@gitster.g","threadId":"65361","inReplyTo":"fe9c86af4825a81b2618ae8ffc8be12300058af2.1774537954.git.git@grubix.eu","subject":"Re: [PATCH 6/6] do not discard const: the ugly truth","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-26T17:07:59Z","receivedAt":"2026-03-26T17:08:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@grubix.eu> writes:\n\n> ISOC23 reveals that we mutate argv strings in place. Confess to this\n> with explicit casts.\n> ---\n>  builtin/rev-parse.c | 8 ++++----\n>  revision.c          | 8 ++++----\n>  2 files changed, 8 insertions(+), 8 deletions(-)\n\nForgot to sign-off?\n\n> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> index 01a62800e8..f429793b6f 100644\n> --- a/builtin/rev-parse.c\n> +++ b/builtin/rev-parse.c\n> @@ -265,7 +265,7 @@ static int show_file(const char *arg, int output_prefix)\n>  \treturn 0;\n>  }\n>  \n> -static int try_difference(const char *arg)\n> +static int try_difference(char *arg)\n>  {\n>  \tchar *dotdot;\n>  \tstruct object_id start_oid;\n> @@ -325,7 +325,7 @@ static int try_difference(const char *arg)\n>  \treturn 0;\n>  }\n\nThis one is unfortunate in that in the end the incoming arg is\ntemporarily truncated by substituting the first \".\" in the \"..\"\nfound in the string with \"\\0\", and then restored to the original\nvalue before returning to the caller, so unless the caller is\nhanding a piece of memory in a read-only segment, nobody should\nhurt or even notice.\n\n> -static int try_parent_shorthands(const char *arg)\n> +static int try_parent_shorthands(char *arg)\n>  {\n>  \tchar *dotdot;\n>  \tstruct object_id oid;\n> @@ -1145,9 +1145,9 @@ int cmd_rev_parse(int argc,\n>  \t\t}\n>  \n>  \t\t/* Not a flag argument */\n> -\t\tif (try_difference(arg))\n> +\t\tif (try_difference((char *) arg))\n>  \t\t\tcontinue;\n> -\t\tif (try_parent_shorthands(arg))\n> +\t\tif (try_parent_shorthands((char *) arg))\n>  \t\t\tcontinue;\n>  \t\tname = arg;\n>  \t\ttype = NORMAL;\n\nThe same, with \"^\" in magic sequences \"^!\", \"^@\", and \"^-\".\n\n> diff --git a/revision.c b/revision.c\n> index 31808e3df0..a28b14a2ea 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2132,7 +2132,7 @@ static int handle_dotdot(const char *arg,\n>  \t\t\t int cant_be_filename)\n>  {\n>  \tstruct object_context a_oc = {0}, b_oc = {0};\n> -\tchar *dotdot = strstr(arg, \"..\");\n> +\tchar *dotdot = (char *) strstr(arg, \"..\");\n>  \tint ret;\n>  \n>  \tif (!dotdot)\n\nThe patch takes a different strategy to deal with this one, even\nthough the pattern should be exactly the same as try_difference() we\nsaw earlier.  Shouldn't we take the same \"internally we know we muck\nwith the string temporarily, but externally we pretend that we take\na const pointer because we revert our temporary modification\"\napproach in builtin/rev-parse.c too?\n\nOne thing that _could_ break if we did so is when the callers do\npass a string in read-only segment to these functions, trusting the\nfunction signature that takes a const pointer promises them that it\nis safe.  And to prepare for it, the approach you took in\nbuiltin/rev-parse.c to be honest about it to the callers is safer.\n\nSo in that sense, perhaps this function should be updated to take a\nnon-const pointer to arg instead of sprinkling casts in the body?\n\n> @@ -2176,7 +2176,7 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl\n>  \t\tgoto out;\n>  \t}\n>  \n> -\tmark = strstr(arg, \"^@\");\n> +\tmark = (char *) strstr(arg, \"^@\");\n>  \tif (mark && !mark[2]) {\n>  \t\t*mark = 0;\n>  \t\tif (add_parents_only(revs, arg, flags, 0)) {\n> @@ -2185,13 +2185,13 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl\n>  \t\t}\n>  \t\t*mark = '^';\n>  \t}\n> -\tmark = strstr(arg, \"^!\");\n> +\tmark = (char *) strstr(arg, \"^!\");\n>  \tif (mark && !mark[2]) {\n>  \t\t*mark = 0;\n>  \t\tif (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), 0))\n>  \t\t\t*mark = '^';\n>  \t}\n> -\tmark = strstr(arg, \"^-\");\n> +\tmark = (char *) strstr(arg, \"^-\");\n>  \tif (mark) {\n>  \t\tint exclude_parent = 1;\n\nDitto.\n\n\n"},{"id":"540108","messageId":"xmqqbjgaqzk3.fsf@gitster.g","threadId":"65361","inReplyTo":"9a90f93111ec54e5eb9675cb84ac1d70ad95e118.1774537954.git.git@grubix.eu","subject":"Re: [PATCH 5/6] do not discard const: keep signature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-26T17:18:52Z","receivedAt":"2026-03-26T17:18:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@grubix.eu> writes:\n\n> Here, while we do not mutate the struct itself, many other signatures\n> expect a non-const argument - possibly unnecessarily - so we opt to keep\n> the original signature by casting to non-const.\n> ---\n\nSorry, but I do not understand the above description, or the code\nchange.  Doesn't bsearch() returns non-const \"void *\" pointer?\n\nAh, the constness of the return value in C23 depends on the\nconstness of pm->commits[] array, which inherits its constness from\nthe constness of parameter pm to the function, and you cast the\nvalue we are going to return explicitly to a non-const pointer.\n\nOK.  In the context of \"C23 constness\" patch series, that may be\nobvious to you, but I suspect a future reader who finds this single\ncommit from the output of \"git blame\" or something would be puzzled\nunless we say this is about adjusting to C23 that makes bsearch() a\nqualifier-preserving function somewhere in the log message.\n\n\n>  pseudo-merge.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/pseudo-merge.c b/pseudo-merge.c\n> index a2d5bd85f9..ac81792e65 100644\n> --- a/pseudo-merge.c\n> +++ b/pseudo-merge.c\n> @@ -644,7 +644,7 @@ static struct pseudo_merge_commit *find_pseudo_merge(const struct pseudo_merge_m\n>  \tif (!pm->commits_nr)\n>  \t\treturn NULL;\n>  \n> -\treturn bsearch(&pos, pm->commits, pm->commits_nr,\n> +\treturn (struct pseudo_merge_commit *) bsearch(&pos, pm->commits, pm->commits_nr,\n>  \t\t       PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp);\n>  }\n"},{"id":"540109","messageId":"xmqq5x6iqz3d.fsf@gitster.g","threadId":"65361","inReplyTo":"8a65ada967b6b1308ea4cffca82102d4de8e9dd9.1774537954.git.git@grubix.eu","subject":"Re: [PATCH 3/6] do not discard const: adjust to non-const data types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-26T17:28:54Z","receivedAt":"2026-03-26T17:29:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@grubix.eu> writes:\n\n> We use data types (such as string_list's util member) which are not\n> necessarily \"non-const in practice\" (such as the list of environment\n> variables in run-command.c) but are not declared \"const\". Rather than\n> duplicating data types (e.g. with a new constr_string_list), discard the\n> const explicitly for now to quell ISOC23 warnings.\n> ---\n>  http-push.c   | 2 +-\n>  run-command.c | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/http-push.c b/http-push.c\n> index 9ae6062198..acc7f1d8fa 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1772,7 +1772,7 @@ int cmd_main(int argc, const char **argv)\n>  \t\t\tstr_end_url_with_slash(arg, &repo->url);\n>  \t\t\trepo->path_len = strlen(repo->url);\n>  \t\t\tif (path) {\n> -\t\t\t\trepo->path = strchr(path+2, '/');\n> +\t\t\t\trepo->path = (char *) strchr(path+2, '/');\n>  \t\t\t\tif (repo->path)\n>  \t\t\t\t\trepo->path_len = strlen(repo->path);\n>  \t\t\t}\n> diff --git a/run-command.c b/run-command.c\n> index 32c290ee6a..1db02ef030 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -604,7 +604,7 @@ static void trace_add_env(struct strbuf *dst, const char *const *deltaenv)\n>  \t/* Last one wins, see run-command.c:prep_childenv() for context */\n>  \tfor (e = deltaenv; e && *e; e++) {\n>  \t\tstruct strbuf key = STRBUF_INIT;\n> -\t\tchar *equals = strchr(*e, '=');\n> +\t\tchar *equals = (char *) strchr(*e, '=');\n>  \n>  \t\tif (equals) {\n>  \t\t\tstrbuf_add(&key, *e, equals - *e);\n\nI didn't look at the other http-push.c one, but this part with a bit\nwider context reads like this:\n\n\tfor (e = deltaenv; e && *e; e++) {\n\t\tstruct strbuf key = STRBUF_INIT;\n\t\tchar *equals = strchr(*e, '=');\n\n\t\tif (equals) {\n\t\t\tstrbuf_add(&key, *e, equals - *e);\n\t\t\tstring_list_insert(&envs, key.buf)->util = equals + 1;\n\t\t} else {\n\t\t\tstring_list_insert(&envs, *e)->util = NULL;\n\t\t}\n\t\tstrbuf_release(&key);\n\t}\n\nI wonder if the cast to strip away constness wants to go near the\nassignment to ->util.\n\n\n"},{"id":"540111","messageId":"20260326173402.GB2447148@coredump.intra.peff.net","threadId":"65361","inReplyTo":"a3a1d2759a0ec5a3ee285689832832e5e3a63768.1774537954.git.git@grubix.eu","subject":"Re: [PATCH 1/6] do not discard const: the simple cases","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T17:34:02Z","receivedAt":"2026-03-26T17:34:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 26, 2026 at 04:22:47PM +0100, Michael J Gruber wrote:\n\n> This patch covers the easy cases where we deal with a non-const pointer\n> to begin with. It is solved by the cast `bar = (char *) foo`.\n\nI think we can often do better, though. For example, in this case:\n\n> diff --git a/builtin/config.c b/builtin/config.c\n> index 7c4857be62..bd277e5911 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -852,7 +852,7 @@ static int get_urlmatch(const struct config_location_options *opts,\n>  \t\tdie(\"%s\", config.url.err);\n>  \n>  \tconfig.section = xstrdup_tolower(var);\n> -\tsection_tail = strchr(config.section, '.');\n> +\tsection_tail = (char *) strchr(config.section, '.');\n>  \tif (section_tail) {\n>  \t\t*section_tail = '\\0';\n>  \t\tconfig.key = section_tail + 1;\n\nWe know that it is OK to cast away the const-ness because config.section\nis writeable, which we know because it just came from xstrdup(). So why\nis it const in the first place? Because the pointer is in a struct which\nmay be used with other const strings.\n\nBut we can untangle this for the compiler without having to cast by\nusing a non-const alias, like:\n\n  char *section;\n  ...\n  config.section = section = xstrdup_tolower(var);\n  section_tail = strchr(section, '.');\n\nWhich I think is safer and shows the intent more clearly.\n\nSome of the other cases below can use similar techniques (e.g., I think\npacket_reader's line probably ought to be non-const).\n\n-Peff\n"},{"id":"540112","messageId":"20260326174204.GC2447148@coredump.intra.peff.net","threadId":"65361","inReplyTo":"fe9c86af4825a81b2618ae8ffc8be12300058af2.1774537954.git.git@grubix.eu","subject":"Re: [PATCH 6/6] do not discard const: the ugly truth","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T17:42:04Z","receivedAt":"2026-03-26T17:42:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 26, 2026 at 04:22:52PM +0100, Michael J Gruber wrote:\n\n> ISOC23 reveals that we mutate argv strings in place. Confess to this\n> with explicit casts.\n\nCollin and I looked at this one a bit in the earlier thread:\n\n  https://lore.kernel.org/git/e6f7e2eddbc9aef1c21f661420a4b8cb9cd8e2c1.1770095829.git.collin.funk1@gmail.com/\n\nI think it is technically legal to mutate argv strings (which is why\nthis doesn't segfault now), though I think we would prefer to treat them\nas conceptually const. You do get a segfault with:\n\n  handle_revision_arg(\"..HEAD\", &revs, 0, 0);\n\nwhich we fortunately never do (we do pass string literals, but never\nwith a range operator).\n\nIMHO the right solution here is to teach the revision-parser not to\ntouch the incoming buffers. We do it only to tie off strings, which can\nmostly be replaced with xmemdupz(). That's slightly less efficient, but\nI don't think it would be measurable (it's one allocation that tends to\nhappen a handful of times per program execution, and the rest of the\nparsing is going to allocate things like commit structs anyway).\n\nI have some patches in that direction, but I haven't gotten around to\npolishing them yet.\n\n-Peff\n"},{"id":"540113","messageId":"xmqqy0jepjqy.fsf@gitster.g","threadId":"65361","inReplyTo":"20260326173402.GB2447148@coredump.intra.peff.net","subject":"Re: [PATCH 1/6] do not discard const: the simple cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-26T17:45:41Z","receivedAt":"2026-03-26T17:45:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Mar 26, 2026 at 04:22:47PM +0100, Michael J Gruber wrote:\n>\n>> This patch covers the easy cases where we deal with a non-const pointer\n>> to begin with. It is solved by the cast `bar = (char *) foo`.\n>\n> I think we can often do better, though. For example, in this case:\n>\n>> diff --git a/builtin/config.c b/builtin/config.c\n>> index 7c4857be62..bd277e5911 100644\n>> --- a/builtin/config.c\n>> +++ b/builtin/config.c\n>> @@ -852,7 +852,7 @@ static int get_urlmatch(const struct config_location_options *opts,\n>>  \t\tdie(\"%s\", config.url.err);\n>>  \n>>  \tconfig.section = xstrdup_tolower(var);\n>> -\tsection_tail = strchr(config.section, '.');\n>> +\tsection_tail = (char *) strchr(config.section, '.');\n>>  \tif (section_tail) {\n>>  \t\t*section_tail = '\\0';\n>>  \t\tconfig.key = section_tail + 1;\n>\n> We know that it is OK to cast away the const-ness because config.section\n> is writeable, which we know because it just came from xstrdup(). So why\n> is it const in the first place? Because the pointer is in a struct which\n> may be used with other const strings.\n>\n> But we can untangle this for the compiler without having to cast by\n> using a non-const alias, like:\n>\n>   char *section;\n>   ...\n>   config.section = section = xstrdup_tolower(var);\n>   section_tail = strchr(section, '.');\n>\n> Which I think is safer and shows the intent more clearly.\n\nYeah, this is much clearer.\n"},{"id":"540117","messageId":"20260326190243.GA412983@coredump.intra.peff.net","threadId":"65361","inReplyTo":"20260326174204.GC2447148@coredump.intra.peff.net","subject":"[PATCH 0/4] fix const issues in revision parser","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T19:02:43Z","receivedAt":"2026-03-26T19:02:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 26, 2026 at 01:42:04PM -0400, Jeff King wrote:\n\n> IMHO the right solution here is to teach the revision-parser not to\n> touch the incoming buffers. We do it only to tie off strings, which can\n> mostly be replaced with xmemdupz(). That's slightly less efficient, but\n> I don't think it would be measurable (it's one allocation that tends to\n> happen a handful of times per program execution, and the rest of the\n> parsing is going to allocate things like commit structs anyway).\n> \n> I have some patches in that direction, but I haven't gotten around to\n> polishing them yet.\n\nHere it is. There were a few oddities to untangle, but I think the\nresult makes the whole thing a bit easier to understand. I may be biased\nas the author, though. ;)\n\n  [1/4]: revision: make handle_dotdot() interface less confusing\n  [2/4]: rev-parse: simplify dotdot parsing\n  [3/4]: revision: avoid writing to const string for parent marks\n  [4/4]: rev-parse: avoid writing to const string for parent marks\n\n builtin/rev-parse.c | 40 +++++++++++++--------------\n revision.c          | 67 +++++++++++++++++++++++----------------------\n 2 files changed, 54 insertions(+), 53 deletions(-)\n\n-Peff\n"},{"id":"540118","messageId":"20260326190444.GA415796@coredump.intra.peff.net","threadId":"65361","inReplyTo":"20260326190243.GA412983@coredump.intra.peff.net","subject":"[PATCH 1/4] revision: make handle_dotdot() interface less confusing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T19:04:44Z","receivedAt":"2026-03-26T19:04:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are two very subtle bits to the way we parse \"..\" (and \"...\")\nrange operators:\n\n 1. In handle_dotdot_1(), we assume that the incoming arguments \"dotdot\"\n    and \"arg\" are part of the same string, with the first digit of the\n    range-operator blanked to a NUL. Then when we want the full name\n    (e.g., to report an error), we replace the NUL with a dot to restore\n    the original string.\n\n 2. In handle_dotdot(), we take in a const string, but then we modify it\n    by overwriting the range operator with a NUL. This has worked OK in\n    practice since we tend to pass in buffers that are actually\n    writeable (including argv), but segfaults with something like:\n\n      handle_revision_arg(\"..HEAD\", &revs, 0, 0);\n\n    On top of that, building with recent versions of glibc causes the\n    compiler to complain, because it notices when we use strchr() or\n    strstr() to launder away constness (basically detecting the\n    possibility of the segfault above via the type system).\n\nInstead of munging the buffer, let's instead make a temporary copy of\nthe left-hand side of the range operator. That avoids any const\nviolations, and lets us pass around the parsed elements independently:\nthe left-hand side, the right-hand side, the number of dots (via the\n\"symmetric\" flag), and the original full string for error messages.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n revision.c | 42 +++++++++++++++++++-----------------------\n 1 file changed, 19 insertions(+), 23 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 31808e3df0..f61262436f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2038,41 +2038,32 @@ static void prepare_show_merge(struct rev_info *revs)\n \tfree(prune);\n }\n \n-static int dotdot_missing(const char *arg, char *dotdot,\n+static int dotdot_missing(const char *full_name,\n \t\t\t  struct rev_info *revs, int symmetric)\n {\n \tif (revs->ignore_missing)\n \t\treturn 0;\n-\t/* de-munge so we report the full argument */\n-\t*dotdot = '.';\n \tdie(symmetric\n \t    ? \"Invalid symmetric difference expression %s\"\n-\t    : \"Invalid revision range %s\", arg);\n+\t    : \"Invalid revision range %s\", full_name);\n }\n \n-static int handle_dotdot_1(const char *arg, char *dotdot,\n+static int handle_dotdot_1(const char *a_name, const char *b_name,\n+\t\t\t   const char *full_name, int symmetric,\n \t\t\t   struct rev_info *revs, int flags,\n \t\t\t   int cant_be_filename,\n \t\t\t   struct object_context *a_oc,\n \t\t\t   struct object_context *b_oc)\n {\n-\tconst char *a_name, *b_name;\n \tstruct object_id a_oid, b_oid;\n \tstruct object *a_obj, *b_obj;\n \tunsigned int a_flags, b_flags;\n-\tint symmetric = 0;\n \tunsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);\n \tunsigned int oc_flags = GET_OID_COMMITTISH | GET_OID_RECORD_PATH;\n \n-\ta_name = arg;\n \tif (!*a_name)\n \t\ta_name = \"HEAD\";\n \n-\tb_name = dotdot + 2;\n-\tif (*b_name == '.') {\n-\t\tsymmetric = 1;\n-\t\tb_name++;\n-\t}\n \tif (!*b_name)\n \t\tb_name = \"HEAD\";\n \n@@ -2081,15 +2072,13 @@ static int handle_dotdot_1(const char *arg, char *dotdot,\n \t\treturn -1;\n \n \tif (!cant_be_filename) {\n-\t\t*dotdot = '.';\n-\t\tverify_non_filename(revs->prefix, arg);\n-\t\t*dotdot = '\\0';\n+\t\tverify_non_filename(revs->prefix, full_name);\n \t}\n \n \ta_obj = parse_object(revs->repo, &a_oid);\n \tb_obj = parse_object(revs->repo, &b_oid);\n \tif (!a_obj || !b_obj)\n-\t\treturn dotdot_missing(arg, dotdot, revs, symmetric);\n+\t\treturn dotdot_missing(full_name, revs, symmetric);\n \n \tif (!symmetric) {\n \t\t/* just A..B */\n@@ -2103,7 +2092,7 @@ static int handle_dotdot_1(const char *arg, char *dotdot,\n \t\ta = lookup_commit_reference(revs->repo, &a_obj->oid);\n \t\tb = lookup_commit_reference(revs->repo, &b_obj->oid);\n \t\tif (!a || !b)\n-\t\t\treturn dotdot_missing(arg, dotdot, revs, symmetric);\n+\t\t\treturn dotdot_missing(full_name, revs, symmetric);\n \n \t\tif (repo_get_merge_bases(the_repository, a, b, &exclude) < 0) {\n \t\t\tcommit_list_free(exclude);\n@@ -2132,16 +2121,23 @@ static int handle_dotdot(const char *arg,\n \t\t\t int cant_be_filename)\n {\n \tstruct object_context a_oc = {0}, b_oc = {0};\n-\tchar *dotdot = strstr(arg, \"..\");\n+\tconst char *dotdot = strstr(arg, \"..\");\n+\tchar *tmp;\n+\tint symmetric = 0;\n \tint ret;\n \n \tif (!dotdot)\n \t\treturn -1;\n \n-\t*dotdot = '\\0';\n-\tret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename,\n-\t\t\t      &a_oc, &b_oc);\n-\t*dotdot = '.';\n+\ttmp = xmemdupz(arg, dotdot - arg);\n+\tdotdot += 2;\n+\tif (*dotdot == '.') {\n+\t\tsymmetric = 1;\n+\t\tdotdot++;\n+\t}\n+\tret = handle_dotdot_1(tmp, dotdot, arg, symmetric, revs, flags,\n+\t\t\t      cant_be_filename, &a_oc, &b_oc);\n+\tfree(tmp);\n \n \tobject_context_release(&a_oc);\n \tobject_context_release(&b_oc);\n-- \n2.53.0.1081.gf77a8b8145\n\n"},{"id":"540119","messageId":"20260326190525.GB415796@coredump.intra.peff.net","threadId":"65361","inReplyTo":"20260326190243.GA412983@coredump.intra.peff.net","subject":"[PATCH 2/4] rev-parse: simplify dotdot parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T19:05:25Z","receivedAt":"2026-03-26T19:05:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The previous commit simplified the way that revision.c parses \"..\" and\n\"...\" range operators. But there's roughly similar code in rev-parse.\nThis is less likely to trigger a segfault, as there is no library\nfunction which we'd pass a string literal to, but it still causes the\ncompiler to complain about laundering away constness via strstr().\n\nLet's give it the same treatment, copying the left-hand side of the\nrange operator into its own string.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/rev-parse.c | 15 +++++++--------\n 1 file changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 01a62800e8..5da9537113 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -267,21 +267,20 @@ static int show_file(const char *arg, int output_prefix)\n \n static int try_difference(const char *arg)\n {\n-\tchar *dotdot;\n+\tconst char *dotdot;\n \tstruct object_id start_oid;\n \tstruct object_id end_oid;\n \tconst char *end;\n \tconst char *start;\n+\tchar *to_free;\n \tint symmetric;\n \tstatic const char head_by_default[] = \"HEAD\";\n \n \tif (!(dotdot = strstr(arg, \"..\")))\n \t\treturn 0;\n+\tstart = to_free = xmemdupz(arg, dotdot - arg);\n \tend = dotdot + 2;\n-\tstart = arg;\n \tsymmetric = (*end == '.');\n-\n-\t*dotdot = 0;\n \tend += symmetric;\n \n \tif (!*end)\n@@ -295,7 +294,7 @@ static int try_difference(const char *arg)\n \t\t * Just \"..\"?  That is not a range but the\n \t\t * pathspec for the parent directory.\n \t\t */\n-\t\t*dotdot = '.';\n+\t\tfree(to_free);\n \t\treturn 0;\n \t}\n \n@@ -308,7 +307,7 @@ static int try_difference(const char *arg)\n \t\t\ta = lookup_commit_reference(the_repository, &start_oid);\n \t\t\tb = lookup_commit_reference(the_repository, &end_oid);\n \t\t\tif (!a || !b) {\n-\t\t\t\t*dotdot = '.';\n+\t\t\t\tfree(to_free);\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\tif (repo_get_merge_bases(the_repository, a, b, &exclude) < 0)\n@@ -318,10 +317,10 @@ static int try_difference(const char *arg)\n \t\t\t\tshow_rev(REVERSED, &commit->object.oid, NULL);\n \t\t\t}\n \t\t}\n-\t\t*dotdot = '.';\n+\t\tfree(to_free);\n \t\treturn 1;\n \t}\n-\t*dotdot = '.';\n+\tfree(to_free);\n \treturn 0;\n }\n \n-- \n2.53.0.1081.gf77a8b8145\n\n"},{"id":"540121","messageId":"20260326191318.GC415796@coredump.intra.peff.net","threadId":"65361","inReplyTo":"20260326190243.GA412983@coredump.intra.peff.net","subject":"[PATCH 3/4] revision: avoid writing to const string for parent marks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T19:13:18Z","receivedAt":"2026-03-26T19:13:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We take in a \"const char *\", but may write a NUL into it when parsing\nparent marks like \"foo^-\", since we want to isolate \"foo\" as a string\nfor further parsing. This is usually OK, as our \"const\" strings are\noften actually argv strings which are technically writeable, but we'd\nsegfault with a string literal like:\n\n  handle_revision_arg(\"HEAD^-\", &revs, 0, 0);\n\nSimilar to how we handled dotdot in a previous commit, we can avoid this\nby making a temporary copy of the left-hand side of the string. The cost\nshould negligible compared to the rest of the parsing (like actually\nparsing commits to create their parent linked-lists).\n\nThere is one slightly tricky thing, though. We parse some of the marks\nprogressively, so that if we see \"foo^!\" for example, we'll strip that\ndown to \"foo\" not just for calling add_parents_only(), but also for the\nrest of the function. That makes sense since we eventually want to pass\n\"foo\" to get_oid_with_context(). But it also means that we'll keep\nlooking for other marks. In particular, \"foo^-^!\" is valid, though oddly\n\"foo^!^-\" would ignore the \"^-\". I'm not sure if this is a weird\nhistorical artifact of the implementation, or if there are important\ncorner cases.\n\nSo I've left the behavior unchanged. Each mark we find allocates a\nstring with the mark stripped, which means we could allocate multiple\ntimes (and carry a free-able pointer for each to the end). But in\npractice we won't, because of the three marks, \"^@\" jumps immediately to\nthe end without further parsing, and \"^-^!\" is nonsense that nobody\nwould pass. So you'd get one allocation in general, and never more than\ntwo.\n\nAnother obvious option would be to just copy \"arg\" up front and be OK\nwith munging it. But that means we pay the cost even when we find no\nmarks. We could make a single copy upon finding a mark and then munge,\nbut that adds extra code to each site (checking whether somebody else\nallocated, and if not, adjusting our \"mark\" pointer to be relative to\nthe copied string).\n\nI aimed for something that was clear and obvious, if a bit verbose.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nAlso one other weirdness I noticed while proof-reading: if we\nsuccessfully parse a mark, we never restore the original string! So if\nyou call:\n\n  char buf[] = \"foo^!\";\n  handle_revision_arg(buf, &revs, 0, 0);\n\nThen \"buf\" would have \"foo\\0!\" after it returns. I guess no callers\ncare, because they only look at the arg again if there was an error.\nBut it incidentally is fixed by this patch.\n\n revision.c | 25 +++++++++++++++----------\n 1 file changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex f61262436f..fda405bf65 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2147,7 +2147,10 @@ static int handle_dotdot(const char *arg,\n static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int flags, unsigned revarg_opt)\n {\n \tstruct object_context oc = {0};\n-\tchar *mark;\n+\tconst char *mark;\n+\tchar *arg_minus_at = NULL;\n+\tchar *arg_minus_excl = NULL;\n+\tchar *arg_minus_dash = NULL;\n \tstruct object *object;\n \tstruct object_id oid;\n \tint local_flags;\n@@ -2174,18 +2177,17 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl\n \n \tmark = strstr(arg, \"^@\");\n \tif (mark && !mark[2]) {\n-\t\t*mark = 0;\n-\t\tif (add_parents_only(revs, arg, flags, 0)) {\n+\t\targ_minus_at = xmemdupz(arg, mark - arg);\n+\t\tif (add_parents_only(revs, arg_minus_at, flags, 0)) {\n \t\t\tret = 0;\n \t\t\tgoto out;\n \t\t}\n-\t\t*mark = '^';\n \t}\n \tmark = strstr(arg, \"^!\");\n \tif (mark && !mark[2]) {\n-\t\t*mark = 0;\n-\t\tif (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), 0))\n-\t\t\t*mark = '^';\n+\t\targ_minus_excl = xmemdupz(arg, mark - arg);\n+\t\tif (add_parents_only(revs, arg_minus_excl, flags ^ (UNINTERESTING | BOTTOM), 0))\n+\t\t\targ = arg_minus_excl;\n \t}\n \tmark = strstr(arg, \"^-\");\n \tif (mark) {\n@@ -2199,9 +2201,9 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl\n \t\t\t}\n \t\t}\n \n-\t\t*mark = 0;\n-\t\tif (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), exclude_parent))\n-\t\t\t*mark = '^';\n+\t\targ_minus_dash = xmemdupz(arg, mark - arg);\n+\t\tif (add_parents_only(revs, arg_minus_dash, flags ^ (UNINTERESTING | BOTTOM), exclude_parent))\n+\t\t\targ = arg_minus_dash;\n \t}\n \n \tlocal_flags = 0;\n@@ -2236,6 +2238,9 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl\n \n out:\n \tobject_context_release(&oc);\n+\tfree(arg_minus_at);\n+\tfree(arg_minus_excl);\n+\tfree(arg_minus_dash);\n \treturn ret;\n }\n \n-- \n2.53.0.1081.gf77a8b8145\n\n"},{"id":"540127","messageId":"20260326191424.GD415796@coredump.intra.peff.net","threadId":"65361","inReplyTo":"20260326190243.GA412983@coredump.intra.peff.net","subject":"[PATCH 4/4] rev-parse: avoid writing to const string for parent marks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T19:14:24Z","receivedAt":"2026-03-26T19:14:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The previous commit cleared up some const confusion in handling parent\nmarks in revision.c, but we have roughly the same code duplicated in\nrev-parse. This one is much easier to fix, because the handling of the\nshortened string is all done in one place, after detecting any marks\n(but without shortening the string between marks).\n\n  As a side note, I suspect this means that it behaves differently than\n  the revision.c parser for weird stuff like \"foo^!^@^-\", but that is\n  outside the scope of this patch.\n\nWhile we are here, let's also rename the variable \"dotdot\", which is\ntotally misleading (and which we already fixed in revision.c long ago\nvia f632dedd8d (handle_revision_arg: stop using \"dotdot\" as a generic\npointer, 2017-05-19)).\n\nDoing that here makes the diff a little messier, but it also lets the\ncompiler help us make sure we did not miss any stray mentions of the\nvariable while we are changing its semantics.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/rev-parse.c | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 5da9537113..218b5f34d6 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -326,46 +326,47 @@ static int try_difference(const char *arg)\n \n static int try_parent_shorthands(const char *arg)\n {\n-\tchar *dotdot;\n+\tconst char *mark;\n \tstruct object_id oid;\n \tstruct commit *commit;\n \tstruct commit_list *parents;\n \tint parent_number;\n \tint include_rev = 0;\n \tint include_parents = 0;\n \tint exclude_parent = 0;\n+\tchar *to_free;\n \n-\tif ((dotdot = strstr(arg, \"^!\"))) {\n+\tif ((mark = strstr(arg, \"^!\"))) {\n \t\tinclude_rev = 1;\n-\t\tif (dotdot[2])\n+\t\tif (mark[2])\n \t\t\treturn 0;\n-\t} else if ((dotdot = strstr(arg, \"^@\"))) {\n+\t} else if ((mark = strstr(arg, \"^@\"))) {\n \t\tinclude_parents = 1;\n-\t\tif (dotdot[2])\n+\t\tif (mark[2])\n \t\t\treturn 0;\n-\t} else if ((dotdot = strstr(arg, \"^-\"))) {\n+\t} else if ((mark = strstr(arg, \"^-\"))) {\n \t\tinclude_rev = 1;\n \t\texclude_parent = 1;\n \n-\t\tif (dotdot[2]) {\n+\t\tif (mark[2]) {\n \t\t\tchar *end;\n-\t\t\texclude_parent = strtoul(dotdot + 2, &end, 10);\n+\t\t\texclude_parent = strtoul(mark + 2, &end, 10);\n \t\t\tif (*end != '\\0' || !exclude_parent)\n \t\t\t\treturn 0;\n \t\t}\n \t} else\n \t\treturn 0;\n \n-\t*dotdot = 0;\n+\targ = to_free = xmemdupz(arg, mark - arg);\n \tif (repo_get_oid_committish(the_repository, arg, &oid) ||\n \t    !(commit = lookup_commit_reference(the_repository, &oid))) {\n-\t\t*dotdot = '^';\n+\t\tfree(to_free);\n \t\treturn 0;\n \t}\n \n \tif (exclude_parent &&\n \t    exclude_parent > commit_list_count(commit->parents)) {\n-\t\t*dotdot = '^';\n+\t\tfree(to_free);\n \t\treturn 0;\n \t}\n \n@@ -386,7 +387,7 @@ static int try_parent_shorthands(const char *arg)\n \t\tfree(name);\n \t}\n \n-\t*dotdot = '^';\n+\tfree(to_free);\n \treturn 1;\n }\n \n-- \n2.53.0.1081.gf77a8b8145\n"},{"id":"540130","messageId":"20260326192320.GA418281@coredump.intra.peff.net","threadId":"65361","inReplyTo":"xmqqy0jepjqy.fsf@gitster.g","subject":"[PATCH] config: store allocated string in non-const pointer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T19:23:20Z","receivedAt":"2026-03-26T19:23:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 26, 2026 at 10:45:41AM -0700, Junio C Hamano wrote:\n\n> > But we can untangle this for the compiler without having to cast by\n> > using a non-const alias, like:\n> [...]\n> Yeah, this is much clearer.\n\nHere it is as a patch with a commit message. I was eventually planning\nto do a complete series that cleans up all the warnings, and this would\nbe part of it. But since other people are starting to work on it, too,\nit may make sense to just send them out as we have them to avoid too\nmuch duplication.\n\n-- >8 --\nSubject: [PATCH] config: store allocated string in non-const pointer\n\nWhen git-config matches a url, we copy the variable section name and\nstore it in the \"section\" member of a urlmatch_config struct. That\nmember is const, since the url-matcher will not touch it (and other\ncallers really will have a const string).\n\nBut that means that we have only a const pointer to our allocated\nstring. We have to cast away the constness when we free it, and likewise\nwhen we assign NUL to tie off the \".\" separating the subsection and key.\nThis latter happens implicitly via a strchr() call, but recent versions\nof glibc have added annotations that let the compiler detect that and\ncomplain.\n\nLet's keep our own \"section\" pointer for the non-const string, and then\njust point config.section at it. That avoids all of the casting, both\nexplicit and implicit.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/config.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 7c4857be62..cf4ba0f7cc 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -838,6 +838,7 @@ static int get_urlmatch(const struct config_location_options *opts,\n \t\t\tconst char *var, const char *url)\n {\n \tint ret;\n+\tchar *section;\n \tchar *section_tail;\n \tstruct config_display_options display_opts = *_display_opts;\n \tstruct string_list_item *item;\n@@ -851,8 +852,8 @@ static int get_urlmatch(const struct config_location_options *opts,\n \tif (!url_normalize(url, &config.url))\n \t\tdie(\"%s\", config.url.err);\n \n-\tconfig.section = xstrdup_tolower(var);\n-\tsection_tail = strchr(config.section, '.');\n+\tconfig.section = section = xstrdup_tolower(var);\n+\tsection_tail = strchr(section, '.');\n \tif (section_tail) {\n \t\t*section_tail = '\\0';\n \t\tconfig.key = section_tail + 1;\n@@ -886,7 +887,7 @@ static int get_urlmatch(const struct config_location_options *opts,\n \tstring_list_clear(&values, 1);\n \tfree(config.url.url);\n \n-\tfree((void *)config.section);\n+\tfree(section);\n \treturn ret;\n }\n \n-- \n2.53.0.1081.gf77a8b8145\n\n"},{"id":"540132","messageId":"xmqqikaipf00.fsf@gitster.g","threadId":"65361","inReplyTo":"20260326190444.GA415796@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] revision: make handle_dotdot() interface less confusing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-26T19:28:15Z","receivedAt":"2026-03-26T19:28:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> There are two very subtle bits to the way we parse \"..\" (and \"...\")\n> range operators:\n>\n>  1. In handle_dotdot_1(), we assume that the incoming arguments \"dotdot\"\n>     and \"arg\" are part of the same string, with the first digit of the\n\n\"digit\" -> \"dot\".\n\n>     range-operator blanked to a NUL. Then when we want the full name\n>     (e.g., to report an error), we replace the NUL with a dot to restore\n>     the original string.\n>\n>  2. In handle_dotdot(), we take in a const string, but then we modify it\n>     by overwriting the range operator with a NUL. This has worked OK in\n>     practice since we tend to pass in buffers that are actually\n>     writeable (including argv), but segfaults with something like:\n>\n>       handle_revision_arg(\"..HEAD\", &revs, 0, 0);\n>\n>     On top of that, building with recent versions of glibc causes the\n>     compiler to complain, because it notices when we use strchr() or\n>     strstr() to launder away constness (basically detecting the\n>     possibility of the segfault above via the type system).\n>\n> Instead of munging the buffer, let's instead make a temporary copy of\n> the left-hand side of the range operator. That avoids any const\n> violations, and lets us pass around the parsed elements independently:\n> the left-hand side, the right-hand side, the number of dots (via the\n> \"symmetric\" flag), and the original full string for error messages.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n\nOK.  I was hoping if we can do without a temporary allocation, but\nthe const-string \"..HEAD\" example does make it clear that it is not\nsomething we can achieve easily.\n\nAnd once we accept that it is inevitable to make a copy, everything\nelse falls into the right place.\n\n> diff --git a/revision.c b/revision.c\n> index 31808e3df0..f61262436f 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2038,41 +2038,32 @@ static void prepare_show_merge(struct rev_info *revs)\n>  \tfree(prune);\n>  }\n>  \n> -static int dotdot_missing(const char *arg, char *dotdot,\n> +static int dotdot_missing(const char *full_name,\n>  \t\t\t  struct rev_info *revs, int symmetric)\n>  {\n>  \tif (revs->ignore_missing)\n>  \t\treturn 0;\n> -\t/* de-munge so we report the full argument */\n> -\t*dotdot = '.';\n>  \tdie(symmetric\n>  \t    ? \"Invalid symmetric difference expression %s\"\n> -\t    : \"Invalid revision range %s\", arg);\n> +\t    : \"Invalid revision range %s\", full_name);\n>  }\n>  \n> -static int handle_dotdot_1(const char *arg, char *dotdot,\n> +static int handle_dotdot_1(const char *a_name, const char *b_name,\n> +\t\t\t   const char *full_name, int symmetric,\n>  \t\t\t   struct rev_info *revs, int flags,\n>  \t\t\t   int cant_be_filename,\n>  \t\t\t   struct object_context *a_oc,\n>  \t\t\t   struct object_context *b_oc)\n>  {\n> -\tconst char *a_name, *b_name;\n>  \tstruct object_id a_oid, b_oid;\n>  \tstruct object *a_obj, *b_obj;\n>  \tunsigned int a_flags, b_flags;\n> -\tint symmetric = 0;\n>  \tunsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);\n>  \tunsigned int oc_flags = GET_OID_COMMITTISH | GET_OID_RECORD_PATH;\n>  \n> -\ta_name = arg;\n>  \tif (!*a_name)\n>  \t\ta_name = \"HEAD\";\n>  \n> -\tb_name = dotdot + 2;\n> -\tif (*b_name == '.') {\n> -\t\tsymmetric = 1;\n> -\t\tb_name++;\n> -\t}\n>  \tif (!*b_name)\n>  \t\tb_name = \"HEAD\";\n>  \n> @@ -2081,15 +2072,13 @@ static int handle_dotdot_1(const char *arg, char *dotdot,\n>  \t\treturn -1;\n>  \n>  \tif (!cant_be_filename) {\n> -\t\t*dotdot = '.';\n> -\t\tverify_non_filename(revs->prefix, arg);\n> -\t\t*dotdot = '\\0';\n> +\t\tverify_non_filename(revs->prefix, full_name);\n>  \t}\n>  \n>  \ta_obj = parse_object(revs->repo, &a_oid);\n>  \tb_obj = parse_object(revs->repo, &b_oid);\n>  \tif (!a_obj || !b_obj)\n> -\t\treturn dotdot_missing(arg, dotdot, revs, symmetric);\n> +\t\treturn dotdot_missing(full_name, revs, symmetric);\n>  \n>  \tif (!symmetric) {\n>  \t\t/* just A..B */\n> @@ -2103,7 +2092,7 @@ static int handle_dotdot_1(const char *arg, char *dotdot,\n>  \t\ta = lookup_commit_reference(revs->repo, &a_obj->oid);\n>  \t\tb = lookup_commit_reference(revs->repo, &b_obj->oid);\n>  \t\tif (!a || !b)\n> -\t\t\treturn dotdot_missing(arg, dotdot, revs, symmetric);\n> +\t\t\treturn dotdot_missing(full_name, revs, symmetric);\n>  \n>  \t\tif (repo_get_merge_bases(the_repository, a, b, &exclude) < 0) {\n>  \t\t\tcommit_list_free(exclude);\n> @@ -2132,16 +2121,23 @@ static int handle_dotdot(const char *arg,\n>  \t\t\t int cant_be_filename)\n>  {\n>  \tstruct object_context a_oc = {0}, b_oc = {0};\n> -\tchar *dotdot = strstr(arg, \"..\");\n> +\tconst char *dotdot = strstr(arg, \"..\");\n> +\tchar *tmp;\n> +\tint symmetric = 0;\n>  \tint ret;\n>  \n>  \tif (!dotdot)\n>  \t\treturn -1;\n>  \n> -\t*dotdot = '\\0';\n> -\tret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename,\n> -\t\t\t      &a_oc, &b_oc);\n> -\t*dotdot = '.';\n> +\ttmp = xmemdupz(arg, dotdot - arg);\n> +\tdotdot += 2;\n> +\tif (*dotdot == '.') {\n> +\t\tsymmetric = 1;\n> +\t\tdotdot++;\n> +\t}\n> +\tret = handle_dotdot_1(tmp, dotdot, arg, symmetric, revs, flags,\n> +\t\t\t      cant_be_filename, &a_oc, &b_oc);\n> +\tfree(tmp);\n>  \n>  \tobject_context_release(&a_oc);\n>  \tobject_context_release(&b_oc);\n"},{"id":"540151","messageId":"20260326231415.GA420281@coredump.intra.peff.net","threadId":"65361","inReplyTo":"xmqqikaipf00.fsf@gitster.g","subject":"Re: [PATCH 1/4] revision: make handle_dotdot() interface less confusing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-26T23:14:15Z","receivedAt":"2026-03-26T23:14:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 26, 2026 at 12:28:15PM -0700, Junio C Hamano wrote:\n\n> > There are two very subtle bits to the way we parse \"..\" (and \"...\")\n> > range operators:\n> >\n> >  1. In handle_dotdot_1(), we assume that the incoming arguments \"dotdot\"\n> >     and \"arg\" are part of the same string, with the first digit of the\n> \n> \"digit\" -> \"dot\".\n\nOops, yeah.\n\n> OK.  I was hoping if we can do without a temporary allocation, but\n> the const-string \"..HEAD\" example does make it clear that it is not\n> something we can achieve easily.\n> \n> And once we accept that it is inevitable to make a copy, everything\n> else falls into the right place.\n\nYeah, I don't think there is another good option. We can drop the\n\"const\" from the interface, which would be more honest, but then callers\nthat use string literals have to either make their own copy, or cast\naway the constness and pray.\n\nThe only \"right\" solution that avoids copying is if all of the\nlower-level functions learned to work with ptr/len pairs instead of\nNUL-terminated strings. But having done that sort of conversion before,\nit ends up quite messy and is prone to errors. Somebody is welcome to\ntry tackling that if they want, but I don't. :)\n\n-Peff\n"},{"id":"540184","messageId":"xmqqv7ehmfmh.fsf@gitster.g","threadId":"65361","inReplyTo":"20260326231415.GA420281@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] revision: make handle_dotdot() interface less confusing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-27T15:55:18Z","receivedAt":"2026-03-27T15:55:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> And once we accept that it is inevitable to make a copy, everything\n>> else falls into the right place.\n>\n> Yeah, I don't think there is another good option. We can drop the\n> \"const\" from the interface, which would be more honest, but then callers\n> that use string literals have to either make their own copy, or cast\n> away the constness and pray.\n>\n> The only \"right\" solution that avoids copying is if all of the\n> lower-level functions learned to work with ptr/len pairs instead of\n> NUL-terminated strings. But having done that sort of conversion before,\n> it ends up quite messy and is prone to errors. Somebody is welcome to\n> try tackling that if they want, but I don't. :)\n\nWe would need to call out to a library function or system call\neventually down the callchain, at which point you'd need to somehow\ncome up with a NUL-terminated equivalent of that <ptr, len> pair.\n\nSo I would avoid going down that path, unless the language itself\nhas already abstracted that difference away, and C is not among\nthose languages.\n"},{"id":"540211","messageId":"177463354761.155656.13826706408579146455.git@grubix.eu","threadId":"65361","inReplyTo":"CALnO6CA0ZfzAk8FU7xOYAW-emLwdVJ9Ed7Vt-77gfuY97FR=1A@mail.gmail.com","subject":"Re: [PATCH 0/6] ISOC23: quell warnings on discarding const","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2026-03-27T17:45:47Z","receivedAt":"2026-03-27T17:54:30Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"D. Ben Knoble venit, vidit, dixit 2026-03-26 17:26:35:\n> On Thu, Mar 26, 2026 at 11:40 AM Michael J Gruber <git@grubix.eu> wrote:\n> >\n> > Hi there\n> >\n> > Fedora 44 beta (gcc-16.0.1, glibc-2.43) brought some fun new warnings\n> > when building git. In essence, we're not always explicit about\n> > const-ness or lack thereof of certain pointers. Before, strchr()'s\n> > signature which turns const arguments into non-const return values\n> > covered this up. With ISOC23, strchr() and friends return const\n> > pointers.\n> >\n> > This little series takes a middle-ground: no new data types (no new\n> > const versions of non-const data types) but more explicit casts.\n> \n> I think a few folks were working on similar things; hopefully I've\n> CC'd some relevant parties.\n\nThanks for catching this. I had checked the list cursorily (I'm not a\nregular) but overlooked it. Peff's going all in on it, as usual ;-)\n\nMichael\n"}]}