{"thread":{"id":"65400","subject":"[PATCH 0/12] fixing the remainder of the C23 strchr warnings","startedAt":"2026-03-31T23:38:58Z","lastAt":"2026-04-04T05:42:19Z","messageCount":50,"participants":["Jeff King","Phillip Wood","Patrick Steinhardt","Junio C Hamano","Taylor Blau","Toon Claes"],"isPatch":true,"patchVersion":1,"patchTotal":12},"messages":[{"id":"540584","messageId":"20260331233856.GA2327197@coredump.intra.peff.net","threadId":"65400","inReplyTo":null,"subject":"[PATCH 0/12] fixing the remainder of the C23 strchr warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:38:56Z","receivedAt":"2026-03-31T23:38:58Z","isPatch":true,"body":"This series fixes the rest of the warnings you might see on recent glibc\nor other C23 libc where:\n\n const char *in = ...;\n char *out = strchr(in, ...);\n\nnow complains instead of quietly assigning the const pointer to a\nnon-const one. It's all textually independent of the other fixes, but\nif you want a clean build you'll need the others. I think Collin's fixes\nhave hit master already, but my jk/c23-const-preserving-fixes are still\nslated for 'next'.\n\nSome of my fixes are similar to what Michael posted in:\n\n https://lore.kernel.org/git/cover.1774537954.git.git@grubix.eu/\n\nbut for most of them I took a somewhat different approach. So this would\nbe applied instead of those patches.\n\nThe patches are:\n\n [01/12]: convert: add const to fix strchr() warnings\n [02/12]: http: add const to fix strchr() warnings\n [03/12]: transport-helper: drop const to fix strchr() warnings\n\n These ones are obvious fixes that just match the type declarations\n to their uses.\n\n [04/12]: pager: explicitly cast away strchr() constness\n [05/12]: run-command: explicitly cast away constness when assigning to void\n\n These are ones where I think an explicit cast is the least-bad\n option.\n\n [06/12]: find_last_dir_sep(): convert inline function to macro\n\n This is the one that gets repeated a zillion times when you build\n because it's in a header file. ;) It takes a slightly different\n approach than Collin's in:\n\n https://lore.kernel.org/git/e6f7e2eddbc9aef1c21f661420a4b8cb9cd8e2c1.1770095829.git.collin.funk1@gmail.com/\n\n which I think reduces the fallout through the rest of the codebase.\n\n [07/12]: pseudo-merge: fix disk reads from find_pseudo_merge()\n\n This one is...spicy. I think there are probably actual bugs here,\n but my hope is that this takes us in the right direction (and shuts\n up the warning).\n\n [08/12]: skip_prefix(): check const match between in and out params\n\n And here is where we might get controversial. It introduces some\n macro hackery that makes it safe and easy to use skip_prefix() with\n const or non-const strings. I _think_ it should just work\n everywhere, but I won't be surprised if some compiler somewhere\n complains about the construct. Coverity does, but it is so full of\n false positives that adding more is not a big deal.\n\n [09/12]: pkt-line: make packet_reader.line non-const\n [10/12]: range-diff: drop const to fix strstr() warnings\n [11/12]: http: drop const to fix strstr() warning\n [12/12]: refs/files-backend: drop const to fix strchr() warning\n\n And then these are all obvious fixes that are only made possible by\n the skip_prefix() magic above. Well, possible without extra ugly\n casts everywhere.\n\n builtin/config.c | 7 ++++---\n builtin/rev-parse.c | 40 ++++++++++++++++++++--------------------\n convert.c | 3 ++-\n git-compat-util.h | 23 ++++++++++++++++++-----\n http-push.c | 2 +-\n pager.c | 3 ++-\n pseudo-merge.c | 32 +++++++++++++++++++-------------\n revision.c | 25 +++++++++++++++----------\n run-command.c | 4 ++--\n transport-helper.c | 3 ++-\n 10 files changed, 85 insertions(+), 57 deletions(-)\n\n-Peff\n"},{"id":"540586","messageId":"20260331234115.GA2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 01/12] convert: add const to fix strchr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:41:15Z","receivedAt":"2026-03-31T23:41:16Z","isPatch":true,"body":"C23 versions of libc (like recent glibc) may provide generic versions of\nstrchr() that match constness between the input and return value. The\nidea being that the compiler can detect when it implicitly converts a\nconst pointer into a non-const one (which then emits a warning).\n\nThere are a few cases here where the result pointer does not need to be\nnon-const at all, and we should mark it as such. That silences the\nwarning (and avoids any potential problems with trying to write via\nthose pointers).\n\nSigned-off-by: Jeff King \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.1136.gd760fbd4a0\n\n"},{"id":"540587","messageId":"20260331234127.GB2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 02/12] http: add const to fix strchr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:41:27Z","receivedAt":"2026-03-31T23:41:28Z","isPatch":true,"body":"The \"path\" field of a \"struct repo\" (a custom http-push struct, not to\nbe confused with \"struct repository) is a pointer into a const argv\nstring, and is never written to.\n\nThe compiler does not traditionally complain about assigning from a\nconst pointer because it happens via strchr(). But with some C23 libc\nversions (notably recent glibc), it has started to do so. Let's mark the\nfield as const to silence the warnings.\n\nSigned-off-by: Jeff King \n---\n http-push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 9ae6062198..96df6344ee 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -99,7 +99,7 @@ static struct object_list *objects;\n \n struct repo {\n \tchar *url;\n-\tchar *path;\n+\tconst char *path;\n \tint path_len;\n \tint has_info_refs;\n \tint can_update_info_refs;\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540588","messageId":"20260331234148.GC2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 03/12] transport-helper: drop const to fix strchr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:41:48Z","receivedAt":"2026-03-31T23:41:50Z","isPatch":true,"body":"We implicitly drop the const from our \"key\" variable when we do:\n\n char *p = strchr(key, ' ');\n\nwhich causes compilation with some C23 versions of libc (notably recent\nglibc) to complain.\n\nWe need \"p\" to remain writable, since we assign NULL over the space we\nfound. We can solve this by also making \"key\" writable. This works\nbecause it comes from a strbuf, which is itself a writable string.\n\nSigned-off-by: Jeff King \n---\n transport-helper.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 4d95d84f9e..4614036c99 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -781,7 +781,8 @@ static int push_update_ref_status(struct strbuf *buf,\n \n \tif (starts_with(buf->buf, \"option \")) {\n \t\tstruct object_id old_oid, new_oid;\n-\t\tconst char *key, *val;\n+\t\tchar *key;\n+\t\tconst char *val;\n \t\tchar *p;\n \n \t\tif (!state->hint || !(state->report || state->new_report))\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540589","messageId":"20260331234220.GD2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 04/12] pager: explicitly cast away strchr() constness","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:42:20Z","receivedAt":"2026-03-31T23:42:22Z","isPatch":true,"body":"When we do:\n\n char *cp = strchr(argv[i], '=');\n\nit implicitly removes the constness from argv[i]. We need \"cp\" to remain\nwritable (since we overwrite it with a NUL). In theory we should be able\nto drop the const from argv[i], because it is a sub-pointer into our\nduplicated pager_env variable.\n\nBut we get it from split_cmdline(), which uses the traditional \"const\nchar **\" type for argv. This is overly limiting, but changing it would\nbe awkward for all the other callers of split_cmdline().\n\nLet's do an explicit cast with a note about why it is OK. This is enough\nto silence compiler warnings about the implicit const problems.\n\nSigned-off-by: Jeff King \n---\n pager.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/pager.c b/pager.c\nindex 5531fff50e..801ba392f2 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -118,7 +118,8 @@ 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\t/* we know this is writable because it was split from pager_env */\n+\t\tchar *cp = strchr((char *)argv[i], '=');\n \n \t\tif (!cp)\n \t\t\tdie(\"malformed build-time PAGER_ENV\");\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540590","messageId":"20260331234251.GE2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 05/12] run-command: explicitly cast away constness when assigning to void","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:42:51Z","receivedAt":"2026-03-31T23:42:53Z","isPatch":true,"body":"We do this:\n\n char *equals = strchr(*e, '=');\n\nwhich implicitly removes the constness from \"*e\" and cause the compiler\nto complain. We never write to \"equals\", but later assign it to a\nstring_list util field, which is defined as non-const \"void *\".\n\nWe have to cast somewhere, but doing so at the assignment to util is the\nleast-bad place, since that is the source of the confusion. Sadly we are\nstill open to accidentally writing to the string via the util pointer,\nbut that is the cost of using void pointers, which lose all type\ninformation.\n\nSigned-off-by: Jeff King \n---\n run-command.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 32c290ee6a..d6980c79b3 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -604,11 +604,11 @@ 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\tconst char *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\tstring_list_insert(&envs, key.buf)->util = (void *)(equals + 1);\n \t\t} else {\n \t\t\tstring_list_insert(&envs, *e)->util = NULL;\n \t\t}\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540591","messageId":"20260331234415.GF2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 06/12] find_last_dir_sep(): convert inline function to macro","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:44:15Z","receivedAt":"2026-03-31T23:44:16Z","isPatch":true,"body":"The find_last_dir_sep() function is implemented as an inline function\nwhich takes in a \"const char *\" and returns a \"char *\" via strrchr().\nThat means that just like strrchr(), it quietly removes the const from\nour pointer, which could lead to accidentally writing to the resulting\nstring.\n\nBut C23 versions of libc (including recent glibc) annotate strrchr()\nsuch that the compiler can detect when const is implicitly lost, and it\nnow complains about the call in this inline function.\n\nWe can't just switch the return type of the function to \"const char *\",\nthough. Some callers really do want a non-const string to be returned\n(and are OK because they are feeding a non-const string into the\nfunction).\n\nThe most general solution is for us to annotate find_last_dir_sep() in\nthe same way that is done for strrchr(). But doing so relies on using\nC23 generics, which we do not otherwise require.\n\nSince this inline function is wrapping a single call to strrchr(), we\ncan take a shortcut. If we implement it as a macro, then the original\ntype information is still available to strrchr(), and it does the check\nfor us.\n\nNote that this is just one implementation of find_last_dir_sep(). There\nis an alternate implementation in compat/win32/path-utils.h. It doesn't\nsuffer from the same warning, as it does not use strrchr() and just\ncasts away const explicitly. That's not ideal, and eventually we may\nwant to conditionally teach it the same C23 generic trick that strrchr()\nuses. But it has been that way forever, and our goal here is just\nquieting new warnings, not improving const-checking.\n\nSigned-off-by: Jeff King \n---\n git-compat-util.h | 6 +-----\n 1 file changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 4b4ea2498f..4bb59b3101 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -335,11 +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-{\n-\treturn strrchr(path, '/');\n-}\n-#define find_last_dir_sep git_find_last_dir_sep\n+#define find_last_dir_sep(path) strrchr((path), '/')\n #endif\n \n #ifndef has_dir_sep\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540593","messageId":"20260331234622.GG2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 07/12] pseudo-merge: fix disk reads from find_pseudo_merge()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:46:22Z","receivedAt":"2026-03-31T23:46:24Z","isPatch":true,"body":"The goal of this commit was to fix a const warning when compiling\nwith new versions of glibc, but ended up untangling a much deeper\nproblem.\n\nThe find_pseudo_merge() function does a bsearch() on the \"commits\"\npointer of a pseudo_merge_map. This pointer ultimately comes from memory\nmapped from the on-disk bitmap file, and is thus not writable.\n\nThe \"commits\" array is correctly marked const, but the result from\nbsearch() is returned directly as a non-const pseudo_merge_commit\nstruct. Since new versions of glibc annotate bsearch() in a way that\ndetects the implicit loss of const, the compiler now warns.\n\nMy first instinct was that we should be returning a const struct. That\nrequires apply_pseudo_merges_for_commit() to mark its local pointer as\nconst. But that doesn't work! If the offset field has the high-bit set,\nwe look it up in the extended table via nth_pseudo_merge_ext(). And that\nfunction then feeds our const struct to read_pseudo_merge_commit_at(),\nwhich writes into it by byte-swapping from the on-disk mmap.\n\nBut I think this points to a larger problem with find_pseudo_merge(). It\nis not just that the return value is missing const, but it is missing\nthat byte-swapping! And we know that byte-swapping is needed here,\nbecause the comparator we use for bsearch() also calls our\nread_pseudo_merge_commit_at() helper.\n\nSo I think the interface is all wrong here. We should not be returning a\npointer to a struct which was cast from on-disk data. We should be\nfilling in a caller-provided struct using the bytes we found,\nbyte-swapping the values.\n\nThat of course raises the dual question: how did this ever work, and\ndoes it work now? The answer to the first part is: this code does not\nseem to be triggered in the test suite at all. If we insert a BUG(\"foo\")\ncall into apply_pseudo_merges_for_commit(), it never triggers.\n\nSo I think there is something wrong or missing from the test setup, and\nthis bears further investigation. Sadly the answer to the second part\n(\"does it work now\") is still \"no idea\". I _think_ this takes us in a\npositive direction, but my goal here is mainly to quiet the compiler\nwarning. Further bug-hunting on this experimental feature can be done\nseparately.\n\nSigned-off-by: Jeff King \n---\n+cc Taylor\n\n pseudo-merge.c | 32 +++++++++++++++++++-------------\n 1 file changed, 19 insertions(+), 13 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex a2d5bd85f9..ff18b6c364 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -638,31 +638,37 @@ static int pseudo_merge_commit_cmp(const void *va, const void *vb)\n \treturn 0;\n }\n \n-static struct pseudo_merge_commit *find_pseudo_merge(const struct pseudo_merge_map *pm,\n-\t\t\t\t\t\t uint32_t pos)\n+static int find_pseudo_merge(const struct pseudo_merge_map *pm, uint32_t pos,\n+\t\t\t struct pseudo_merge_commit *out)\n {\n+\tconst unsigned char *at;\n+\n \tif (!pm->commits_nr)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n-\treturn bsearch(&pos, pm->commits, pm->commits_nr,\n-\t\t PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp);\n+\tat = bsearch(&pos, pm->commits, pm->commits_nr,\n+\t\t PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp);\n+\tif (!at)\n+\t\treturn 0;\n+\n+\tread_pseudo_merge_commit_at(out, at);\n+\treturn 1;\n }\n \n int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\t\t\t struct bitmap *result,\n \t\t\t\t struct commit *commit, uint32_t commit_pos)\n {\n \tstruct pseudo_merge *merge;\n-\tstruct pseudo_merge_commit *merge_commit;\n+\tstruct pseudo_merge_commit merge_commit;\n \tint ret = 0;\n \n-\tmerge_commit = find_pseudo_merge(pm, commit_pos);\n-\tif (!merge_commit)\n+\tif (!find_pseudo_merge(pm, commit_pos, &merge_commit))\n \t\treturn 0;\n \n-\tif (merge_commit->pseudo_merge_ofs & ((uint64_t)1<<63)) {\n+\tif (merge_commit.pseudo_merge_ofs & ((uint64_t)1<<63)) {\n \t\tstruct pseudo_merge_commit_ext ext = { 0 };\n-\t\toff_t ofs = merge_commit->pseudo_merge_ofs & ~((uint64_t)1<<63);\n+\t\toff_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63);\n \t\tuint32_t i;\n \n \t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < -1) {\n@@ -673,11 +679,11 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\t}\n \n \t\tfor (i = 0; i < ext.nr; i++) {\n-\t\t\tif (nth_pseudo_merge_ext(pm, &ext, merge_commit, i) < 0)\n+\t\t\tif (nth_pseudo_merge_ext(pm, &ext, &merge_commit, i) < 0)\n \t\t\t\treturn ret;\n \n \t\t\tmerge = pseudo_merge_at(pm, &commit->object.oid,\n-\t\t\t\t\t\tmerge_commit->pseudo_merge_ofs);\n+\t\t\t\t\t\tmerge_commit.pseudo_merge_ofs);\n \n \t\t\tif (!merge)\n \t\t\t\treturn ret;\n@@ -687,7 +693,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\t}\n \t} else {\n \t\tmerge = pseudo_merge_at(pm, &commit->object.oid,\n-\t\t\t\t\tmerge_commit->pseudo_merge_ofs);\n+\t\t\t\t\tmerge_commit.pseudo_merge_ofs);\n \n \t\tif (!merge)\n \t\t\treturn ret;\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540594","messageId":"20260331235017.GH2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 08/12] skip_prefix(): check const match between in and out params","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:50:17Z","receivedAt":"2026-03-31T23:50:18Z","isPatch":true,"body":"The skip_prefix() function takes in a \"const char *\" string, and returns\nvia a \"const char **\" out-parameter that points somewhere in that\nstring. This is fine if you are operating on a const string, like:\n\n const char *in = ...;\n const char *out;\n if (skip_prefix(in, \"foo\", &out))\n\t...look at out...\n\nIt is also OK if \"in\" is not const but \"out\" is, as we add an implicit\nconst when we pass \"in\" to the function. But there's another case where\nthis is limiting. If we want both fields to be non-const, like:\n\n char *in = ...;\n char *out;\n if (skip_prefix(in, \"foo\", &out))\n\t*out = '\\0';\n\nit doesn't work. The compiler will complain about the type mismatch in\npassing \"&out\" to a parameter which expects \"const char **\". So to make\nthis work, we have to do an explicit cast.\n\nBut such a cast is ugly, and also means that we run afoul of making this\nmistake:\n\n const char *in = ...;\n char *out;\n if (skip_prefix(in, \"foo\", (const char **)&out))\n\t*out = '\\0';\n\nwhich causes us to write to the memory pointed by \"in\", which was const.\n\nWe can imagine these four cases as:\n\n (1) const in, const out\n (2) non-const in, const out\n (3) non-const in, non-const out\n (4) const in, non-const out\n\nCases (1) and (2) work now. We would like case (3) to work but it\ndoesn't. But we would like to catch case (4) as a compile error.\n\nSo ideally the rule is \"the out-parameter must be at least as const as\nthe in-parameter\". We can do this with some macro trickery. We wrap\nskip_prefix() in a macro so that it has access to the real types of\nin/out. And then we pass those parameters through another macro which:\n\n 1. Fails if the \"at least as const\" rule is not filled.\n\n 2. Casts to match the signature of the real skip_prefix().\n\nThere are a lot of ways to implement the \"fails\" part. You can use\n__builtin_types_compatible_p() to check, and then either our\nBUILD_ASSERT macros or _Static_assert to fail. But that requires some\nconditional compilation based on compiler feature. That's probably OK\n(the fallback would be to just cast without catching case 4). But we can\ndo better.\n\nThe macro I have here uses a ternary with a dead branch that tries to\nassign \"in\" to \"out\", which should work everywhere and lets the compiler\ncatch the problem in the usual way. With an input like this:\n\n int foo(const char *x, const char **y);\n #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out)))\n\n void ok_const(const char *x, const char **y)\n {\n foo(x, y);\n }\n\n void ok_nonconst(char *x, char **y)\n {\n foo(x, y);\n }\n\n void ok_add_const(char *x, const char **y)\n {\n foo(x, y);\n }\n\n void bad_drop_const(const char *x, char **y)\n {\n foo(x, y);\n }\n\ngcc reports:\n\n foo.c: In function ‘bad_drop_const’:\n foo.c:2:35: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]\n 2 | ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n | ^\n foo.c:4:31: note: in expansion of macro ‘CONST_OUTPARAM’\n 4 | #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out)))\n | ^~~~~~~~~~~~~~\n foo.c:23:9: note: in expansion of macro ‘foo’\n 23 | foo(x, y);\n | ^~~\n\nIt's a bit verbose, but I think makes it reasonably clear what's going\non. Using BUILD_ASSERT_OR_ZERO() ends up much worse. Using\n_Static_assert you can be a bit more informative, but that's not\nsomething we use at all yet in our code-base (it's an old gnu-ism later\nstandardized in C11).\n\nOur generic macro only works for \"const char **\", which is something we\ncould improve by using typeof(in). But that introduces more portability\nquestions, and also some weird corner cases (e.g., around implicit void\nconversion).\n\nThis patch just introduces the concept. We'll make use of it in future\npatches.\n\nSigned-off-by: Jeff King \n---\n git-compat-util.h | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 4bb59b3101..58e494e037 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -491,6 +491,23 @@ static inline bool skip_prefix(const char *str, const char *prefix,\n \treturn false;\n }\n \n+/*\n+ * Check that an out-parameter that is \"at least as const as\" a matching\n+ * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n+ * of \"str\". So:\n+ *\n+ * const str, const out: ok\n+ * non-const str, const out: ok\n+ * non-const str, non-const out: ok\n+ * const str, non-const out: compile error\n+ *\n+ * See the skip_prefix macro below for an example of use.\n+ */\n+#define CONST_OUTPARAM(in, out) \\\n+ ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n+#define skip_prefix(str, prefix, out) \\\n+\tskip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n+\n /*\n * Like skip_prefix, but promises never to read past \"len\" bytes of the input\n * buffer, and returns the remaining number of bytes in \"out\" via \"outlen\".\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540595","messageId":"20260331235136.GI2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 09/12] pkt-line: make packet_reader.line non-const","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:51:36Z","receivedAt":"2026-03-31T23:51:37Z","isPatch":true,"body":"The \"line\" member of a packet_reader struct is marked as const. This\nkind of makes sense, because it's not its own allocated buffer that\nshould be freed, and we often use const to indicate that. But it is\nalways writable, because it points into the non-const \"buffer\" member.\n\nAnd we rely on this writability in places like send-pack and\nreceive-pack, where we parse incoming packet contents by writing NULs\nover delimiters. This has traditionally worked because we implicitly\ncast away the constness with strchr() like:\n\n const char *head;\n char *p;\n\n head = reader->line;\n p = strchr(head, ' ');\n\nSince C23 libc provides a generic strchr() to detect this implicit\nconst removal, this now generate a compiler warning on some platforms\n(like recent glibc).\n\nWe can fix it by marking \"line\" as non-const, as well as a few\nintermediate variables (like \"head\" in the above example). Note that by\nitself, switching to a non-const variable would cause problems with this\nline in send-pack.c:\n\n if (!skip_prefix(reader->line, \"unpack \", &reader->line))\n\nBut due to our skip_prefix() magic introduced in the previous commit,\nthis compiles fine (both the in and out-parameters are non-const, so we\nknow it is safe).\n\nSigned-off-by: Jeff King \n---\n builtin/receive-pack.c | 7 ++++---\n pkt-line.h | 2 +-\n send-pack.c | 7 ++++---\n 3 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e34edff406..a6af16c4e7 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1025,8 +1025,8 @@ static int read_proc_receive_report(struct packet_reader *reader,\n \n \tfor (;;) {\n \t\tstruct object_id old_oid, new_oid;\n-\t\tconst char *head;\n-\t\tconst char *refname;\n+\t\tchar *head;\n+\t\tchar *refname;\n \t\tchar *p;\n \t\tenum packet_read_status status;\n \n@@ -1050,7 +1050,8 @@ static int read_proc_receive_report(struct packet_reader *reader,\n \t\t}\n \t\t*p++ = '\\0';\n \t\tif (!strcmp(head, \"option\")) {\n-\t\t\tconst char *key, *val;\n+\t\t\tchar *key;\n+\t\t\tconst char *val;\n \n \t\t\tif (!hint || !(report || new_report)) {\n \t\t\t\tif (!once++)\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 3b33cc64f3..e6cf85e34e 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -184,7 +184,7 @@ struct packet_reader {\n \tint pktlen;\n \n \t/* the last line read */\n-\tconst char *line;\n+\tchar *line;\n \n \t/* indicates if a line has been peeked */\n \tint line_peeked;\ndiff --git a/send-pack.c b/send-pack.c\nindex 07ecfae4de..b4361d5610 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -175,8 +175,8 @@ static int receive_status(struct repository *r,\n \tret = receive_unpack_status(reader);\n \twhile (1) {\n \t\tstruct object_id old_oid, new_oid;\n-\t\tconst char *head;\n-\t\tconst char *refname;\n+\t\tchar *head;\n+\t\tchar *refname;\n \t\tchar *p;\n \t\tif (packet_reader_read(reader) != PACKET_READ_NORMAL)\n \t\t\tbreak;\n@@ -190,7 +190,8 @@ static int receive_status(struct repository *r,\n \t\t*p++ = '\\0';\n \n \t\tif (!strcmp(head, \"option\")) {\n-\t\t\tconst char *key, *val;\n+\t\t\tchar *key;\n+\t\t\tconst char *val;\n \n \t\t\tif (!hint || !(report || new_report)) {\n \t\t\t\tif (!once++)\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540596","messageId":"20260331235201.GJ2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 10/12] range-diff: drop const to fix strstr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:52:01Z","receivedAt":"2026-03-31T23:52:02Z","isPatch":true,"body":"This is another case where we implicitly drop the \"const\" from a pointer\nby feeding it to strstr() and assigning the result to a non-const\npointer. This is OK in practice, since the const pointer originally\ncomes from a writable source (a strbuf), but C23 libc implementations\nhave started to complain about it.\n\nWe do write to the output pointer, so it needs to remain non-const. We\ncan just switch the input pointer to also be non-const in this case. By\nitself that would run into problems with calls to skip_prefix(), but\nsince that function has now been taught to match in/out constness\nautomatically, it just works without us doing anything further.\n\nSigned-off-by: Jeff King \n---\n range-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 2712a9a107..8e2dd2eb19 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -88,7 +88,7 @@ static int read_patches(const char *range, struct string_list *list,\n \tline = contents.buf;\n \tsize = contents.len;\n \tfor (; size > 0; size -= len, line += len) {\n-\t\tconst char *p;\n+\t\tchar *p;\n \t\tchar *eol;\n \n \t\teol = memchr(line, '\\n', size);\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540597","messageId":"20260331235240.GK2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 11/12] http: drop const to fix strstr() warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:52:40Z","receivedAt":"2026-03-31T23:52:42Z","isPatch":true,"body":"In redact_sensitive_header(), a C23 implementation of libc will complain\nthat strstr() assigns the result from \"const char *cookie\" to \"char\n*semicolon\".\n\nUltimately the memory is writable. We're fed a strbuf, generate a const\npointer \"sensitive_header\" within it using skip_iprefix(), and then\nassign the result to \"cookie\". So we can solve this by dropping the\nconst from \"cookie\" and \"sensitive_header\".\n\nHowever, this runs afoul of skip_iprefix(), which wants a \"const char\n**\" for its out-parameter. We can solve that by teaching skip_iprefix()\nthe same \"make sure out is at least as const as in\" magic that we\nrecently taught to skip_prefix().\n\nSigned-off-by: Jeff King \n---\n git-compat-util.h | 3 +++\n http.c | 4 ++--\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 58e494e037..f60793fc36 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -914,6 +914,9 @@ static inline bool skip_iprefix(const char *str, const char *prefix,\n \treturn false;\n }\n \n+#define skip_iprefix(str, prefix, out) \\\n+\tskip_iprefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n+\n /*\n * Like skip_prefix_mem, but compare case-insensitively. Note that the\n * comparison is done via tolower(), so it is strictly ASCII (no multi-byte\ndiff --git a/http.c b/http.c\nindex 8ea1b9d1f6..8801bd22fe 100644\n--- a/http.c\n+++ b/http.c\n@@ -726,7 +726,7 @@ static int has_proxy_cert_password(void)\n static int redact_sensitive_header(struct strbuf *header, size_t offset)\n {\n \tint ret = 0;\n-\tconst char *sensitive_header;\n+\tchar *sensitive_header;\n \n \tif (trace_curl_redact &&\n \t (skip_iprefix(header->buf + offset, \"Authorization:\", &sensitive_header) ||\n@@ -743,7 +743,7 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset)\n \t} else if (trace_curl_redact &&\n \t\t skip_iprefix(header->buf + offset, \"Cookie:\", &sensitive_header)) {\n \t\tstruct strbuf redacted_header = STRBUF_INIT;\n-\t\tconst char *cookie;\n+\t\tchar *cookie;\n \n \t\twhile (isspace(*sensitive_header))\n \t\t\tsensitive_header++;\n-- \n2.53.0.1136.gd760fbd4a0\n\n"},{"id":"540598","messageId":"20260331235341.GL2328529@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH 12/12] refs/files-backend: drop const to fix strchr() warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:53:41Z","receivedAt":"2026-03-31T23:53:42Z","isPatch":true,"body":"In show_one_reflog_ent(), we're fed a writable strbuf buffer, which we\nparse into the various reflog components. We write a NUL over email_end\nto tie off one of the fields, and thus email_end must be non-const.\n\nBut with a C23 implementation of libc, strchr() will now complain when\nassigning the result to a non-const pointer from a const one. So we can\nfix this by making the source pointer non-const.\n\nBut there's a catch. We derive that source pointer by parsing the line\nwith parse_oid_hex_algop(), which requires a const pointer for its\nout-parameter. We can work around that by teaching it to use our\nCONST_OUTPARAM() trick, just like skip_prefix(). Note that unlike\nskip_prefix(), the function is not inline, so we can't just wrap it\nusing the same name (otherwise the actual definition would expand the\nmacro, which breaks compilation). So we rename the actual function with\nan \"_impl\" suffix, and callers will all use the macro.\n\nSigned-off-by: Jeff King \n---\n hex.c | 6 +++---\n hex.h | 6 ++++--\n refs/files-backend.c | 2 +-\n 3 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/hex.c b/hex.c\nindex 865a232167..bc756722ca 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -54,9 +54,9 @@ int get_oid_hex(const char *hex, struct object_id *oid)\n \treturn get_oid_hex_algop(hex, oid, the_hash_algo);\n }\n \n-int parse_oid_hex_algop(const char *hex, struct object_id *oid,\n-\t\t\tconst char **end,\n-\t\t\tconst struct git_hash_algo *algop)\n+int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid,\n+\t\t\t const char **end,\n+\t\t\t const struct git_hash_algo *algop)\n {\n \tint ret = get_oid_hex_algop(hex, oid, algop);\n \tif (!ret)\ndiff --git a/hex.h b/hex.h\nindex e9ccb54065..db7882bd17 100644\n--- a/hex.h\n+++ b/hex.h\n@@ -40,8 +40,10 @@ char *oid_to_hex(const struct object_id *oid);\t\t\t\t\t\t/* same static buffer */\n * other invalid character. end is only updated on success; otherwise, it is\n * unmodified.\n */\n-int parse_oid_hex_algop(const char *hex, struct object_id *oid, const char **end,\n-\t\t\tconst struct git_hash_algo *algo);\n+int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, const char **end,\n+\t\t\t const struct git_hash_algo *algo);\n+#define parse_oid_hex_algop(hex, oid, end, algo) \\\n+\tparse_oid_hex_algop_impl((hex), (oid), CONST_OUTPARAM((hex), (end)), (algo))\n \n /*\n * These functions work like get_oid_hex and parse_oid_hex, but they will parse\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 7ce0d57478..ad543ad751 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2190,7 +2190,7 @@ static int show_one_reflog_ent(struct files_ref_store *refs,\n \tchar *email_end, *message;\n \ttimestamp_t timestamp;\n \tint tz;\n-\tconst char *p = sb->buf;\n+\tchar *p = sb->buf;\n \n \t/* old SP new SP name SP time TAB msg LF */\n \tif (!sb->len || sb->buf[sb->len - 1] != '\\n' ||\n-- \n2.53.0.1136.gd760fbd4a0\n"},{"id":"540599","messageId":"20260331235637.GA2328851@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331234622.GG2328529@coredump.intra.peff.net","subject":"Re: [PATCH 07/12] pseudo-merge: fix disk reads from find_pseudo_merge()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-31T23:56:37Z","receivedAt":"2026-03-31T23:56:39Z","isPatch":true,"body":"On Tue, Mar 31, 2026 at 07:46:23PM -0400, Jeff King wrote:\n\n> So I think there is something wrong or missing from the test setup, and\n> this bears further investigation. Sadly the answer to the second part\n> (\"does it work now\") is still \"no idea\". I _think_ this takes us in a\n> positive direction, but my goal here is mainly to quiet the compiler\n> warning. Further bug-hunting on this experimental feature can be done\n> separately.\n\nIf this is the wrong direction or if we just want to keep things minimal\nin this patch series, the absolute smallest fix is probably to cast away\nthe constness explicitly in find_pseudo_merge(), along with a comment\nthat the fix is almost certainly wrong. ;)\n\n-Peff\n"},{"id":"540640","messageId":"14a417c6-fc80-4a7e-993d-57fff10896f8@gmail.com","threadId":"65400","inReplyTo":"20260331235017.GH2328529@coredump.intra.peff.net","subject":"Re: [PATCH 08/12] skip_prefix(): check const match between in and out params","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-01T13:17:20Z","receivedAt":"2026-04-01T13:17:23Z","isPatch":true,"body":"Hi peff\n\nOn 01/04/2026 00:50, Jeff King wrote:\n> \n> +/*\n> + * Check that an out-parameter that is \"at least as const as\" a matching\n> + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n> + * of \"str\". So:\n> + *\n> + * const str, const out: ok\n> + * non-const str, const out: ok\n> + * non-const str, non-const out: ok\n> + * const str, non-const out: compile error\n> + *\n> + * See the skip_prefix macro below for an example of use.\n> + */\n> +#define CONST_OUTPARAM(in, out) \\\n> + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n> +#define skip_prefix(str, prefix, out) \\\n> +\tskip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n\nThis is clever but it changes the behavior of skip_prefix() which is \ndocumented as not touching out if it returns false. That may not matter \nin practice but there are nearly 600 callers so auditing them all would \nbe quite an undertaking. Elsewhere there was some discussion about using \ntype generic macros to fix the warnings. That would be more complex as \nwe need to check if they were supported by the compiler but it would \navoid changing the behavior.\n\nThanks for working on fixing these warnings, your approach of trying to \nfix the underlying problem rather than casting away the warning is very \nwelcome.\n\nPhillip\n\n"},{"id":"540641","messageId":"ac0hyTqMvHriS_yf@pks.im","threadId":"65400","inReplyTo":"20260331234148.GC2328529@coredump.intra.peff.net","subject":"Re: [PATCH 03/12] transport-helper: drop const to fix strchr() warnings","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-01T13:46:49Z","receivedAt":"2026-04-01T13:46:56Z","isPatch":true,"body":"On Tue, Mar 31, 2026 at 07:41:48PM -0400, Jeff King wrote:\n> We implicitly drop the const from our \"key\" variable when we do:\n> \n> char *p = strchr(key, ' ');\n> \n> which causes compilation with some C23 versions of libc (notably recent\n> glibc) to complain.\n> \n> We need \"p\" to remain writable, since we assign NULL over the space we\n\nYou probably mean NUL, not NULL.\n\nPatrick\n"},{"id":"540642","messageId":"ac0hzlDdqEEPPMkh@pks.im","threadId":"65400","inReplyTo":"20260331235017.GH2328529@coredump.intra.peff.net","subject":"Re: [PATCH 08/12] skip_prefix(): check const match between in and out params","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-01T13:46:54Z","receivedAt":"2026-04-01T13:46:59Z","isPatch":true,"body":"On Tue, Mar 31, 2026 at 07:50:17PM -0400, Jeff King wrote:\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 4bb59b3101..58e494e037 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -491,6 +491,23 @@ static inline bool skip_prefix(const char *str, const char *prefix,\n> \treturn false;\n> }\n> \n> +/*\n> + * Check that an out-parameter that is \"at least as const as\" a matching\n> + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n> + * of \"str\". So:\n> + *\n> + * const str, const out: ok\n> + * non-const str, const out: ok\n> + * non-const str, non-const out: ok\n> + * const str, non-const out: compile error\n> + *\n> + * See the skip_prefix macro below for an example of use.\n> + */\n> +#define CONST_OUTPARAM(in, out) \\\n> + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n> +#define skip_prefix(str, prefix, out) \\\n> +\tskip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n\nOkay. In theory this would cause us to evaluate both `out` and `str`\nmultiple times. But I guess because this is a dead branch we expect the\ncompiler to optimize these statements away so that we don't have to\nworry about this.\n\nConstructs like this always feel a bit dirty, but I guess this is going\nto be fine in practice.\n\nPatrick\n"},{"id":"540643","messageId":"ac0h0xwqLdX5u51v@pks.im","threadId":"65400","inReplyTo":"20260331235341.GL2328529@coredump.intra.peff.net","subject":"Re: [PATCH 12/12] refs/files-backend: drop const to fix strchr() warning","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-01T13:46:59Z","receivedAt":"2026-04-01T13:47:04Z","isPatch":true,"body":"On Tue, Mar 31, 2026 at 07:53:41PM -0400, Jeff King wrote:\n> In show_one_reflog_ent(), we're fed a writable strbuf buffer, which we\n> parse into the various reflog components. We write a NUL over email_end\n> to tie off one of the fields, and thus email_end must be non-const.\n> \n> But with a C23 implementation of libc, strchr() will now complain when\n> assigning the result to a non-const pointer from a const one. So we can\n> fix this by making the source pointer non-const.\n> \n> But there's a catch. We derive that source pointer by parsing the line\n> with parse_oid_hex_algop(), which requires a const pointer for its\n> out-parameter. We can work around that by teaching it to use our\n> CONST_OUTPARAM() trick, just like skip_prefix(). Note that unlike\n> skip_prefix(), the function is not inline, so we can't just wrap it\n> using the same name (otherwise the actual definition would expand the\n> macro, which breaks compilation). So we rename the actual function with\n> an \"_impl\" suffix, and callers will all use the macro.\n\nFair. In fact, I was a bit torn with the other commits whether it's nice\nto reuse the same name. I guess what it buys us is that you cannot\naccidentally call the wrong function without the guardrails. Even though\nthat's quite unlikely with the `_impl` suffix.\n\nThanks!\n\nPatrick\n"},{"id":"540646","messageId":"b77be594-83b4-4d9b-9f89-569f1f335d61@gmail.com","threadId":"65400","inReplyTo":"14a417c6-fc80-4a7e-993d-57fff10896f8@gmail.com","subject":"Re: [PATCH 08/12] skip_prefix(): check const match between in and out params","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-01T14:04:10Z","receivedAt":"2026-04-01T14:04:15Z","isPatch":true,"body":"On 01/04/2026 14:17, Phillip Wood wrote:\n> On 01/04/2026 00:50, Jeff King wrote:\n>>\n>> +/*\n>> + * Check that an out-parameter that is \"at least as const as\" a matching\n>> + * in-parameter. For example, skip_prefix() will return \"out\" that is \n>> a subset\n>> + * of \"str\". So:\n>> + *\n>> + *  const str, const out: ok\n>> + *  non-const str, const out: ok\n>> + *  non-const str, non-const out: ok\n>> + *  const str, non-const out: compile error\n>> + *\n>> + *  See the skip_prefix macro below for an example of use.\n>> + */\n>> +#define CONST_OUTPARAM(in, out) \\\n>> +    ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n>> +#define skip_prefix(str, prefix, out) \\\n>> +    skip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n> \n> This is clever but it changes the behavior of skip_prefix() which is \n> documented as not touching out if it returns false.\n\nSorry, I've just realized we always take the other branch so this does \nnot change the behavior and is in fact a nice solution to the problem.\n\nThanks\n\nPhillip\n\n> That may not matter \n> in practice but there are nearly 600 callers so auditing them all would \n> be quite an undertaking. Elsewhere there was some discussion about using \n> type generic macros to fix the warnings. That would be more complex as \n> we need to check if they were supported by the compiler but it would \n> avoid changing the behavior.\n> \n> Thanks for working on fixing these warnings, your approach of trying to \n> fix the underlying problem rather than casting away the warning is very \n> welcome.\n> \n> Phillip\n> \n> \n\n"},{"id":"540665","messageId":"20260401192423.GA2905896@coredump.intra.peff.net","threadId":"65400","inReplyTo":"b77be594-83b4-4d9b-9f89-569f1f335d61@gmail.com","subject":"Re: [PATCH 08/12] skip_prefix(): check const match between in and out params","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-01T19:24:23Z","receivedAt":"2026-04-01T19:31:05Z","isPatch":true,"body":"On Wed, Apr 01, 2026 at 03:04:10PM +0100, Phillip Wood wrote:\n\n> On 01/04/2026 14:17, Phillip Wood wrote:\n> > On 01/04/2026 00:50, Jeff King wrote:\n> > > \n> > > +/*\n> > > + * Check that an out-parameter that is \"at least as const as\" a matching\n> > > + * in-parameter. For example, skip_prefix() will return \"out\" that\n> > > is a subset\n> > > + * of \"str\". So:\n> > > + *\n> > > + *  const str, const out: ok\n> > > + *  non-const str, const out: ok\n> > > + *  non-const str, non-const out: ok\n> > > + *  const str, non-const out: compile error\n> > > + *\n> > > + *  See the skip_prefix macro below for an example of use.\n> > > + */\n> > > +#define CONST_OUTPARAM(in, out) \\\n> > > +    ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n> > > +#define skip_prefix(str, prefix, out) \\\n> > > +    skip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n> > \n> > This is clever but it changes the behavior of skip_prefix() which is\n> > documented as not touching out if it returns false.\n> \n> Sorry, I've just realized we always take the other branch so this does not\n> change the behavior and is in fact a nice solution to the problem.\n\nYeah, exactly. I was curious if the dead branch would be left in place,\nbut gcc seems to prune it even at -O0.\n\nI also pondered whether:\n\n (*out = in,out)\n\nmight be a problem, but I think it is OK. The \",\" is a sequence point,\nso it is well defined (of course we would never run this code anyway,\nbut if we have undefined behavior in the code at all, it may cause\nconfusing effects).\n\nFor reference, this is the more complicated one I came up with:\n\n /*\n * Note that builtin_types_compatible_p() counts \"char\" and \"const\n * char\" as the same type. So we deref and construct our own pointer\n * with const to find out it \"x\" is const, and then either compare\n * x and y exactly (if it is const, they must both be) or dereferenced\n * (which lets y be either const or not).\n */\n #define CONST_COMPATIBLE(x, y) \\\n (__builtin_types_compatible_p(typeof(x), const typeof(*(x)) *) ? \\\n __builtin_types_compatible_p(typeof(x), typeof(y)) : \\\n __builtin_types_compatible_p(typeof(*(x)), typeof(*(y))))\n\nI also tried using a gcc statement-expression and _Static_assert to get\na nicer message, like this:\n\n #define CONST_OUTPARAM(in, out, in_name, out_name) ({ \\\n _Static_assert(CONST_COMPATIBLE((in),*(out)), \\\n in_name \" is not const-compatible with \" out_name); \\\n (const typeof(*(in)) **)(out); \\\n })\n #define skip_prefix(str, prefix, out) \\\n skip_prefix(str, prefix, CONST_OUTPARAM((str), (out), #str, #out))\n\nIt does produce slightly nicer output:\n\n foo.c: In function ‘bad’:\n foo.c:8:9: error: static assertion failed: \"my_in_var is not const-compatible with my_out_var\"\n 8 | _Static_assert(CONST_COMPATIBLE((in),*(out)), \\\n | ^~~~~~~~~~~~~~\n foo.c:13:34: note: in expansion of macro ‘CONST_OUTPARAM’\n 13 | skip_prefix(str, prefix, CONST_OUTPARAM((str), (out), #str, #out))\n | ^~~~~~~~~~~~~~\n foo.c:16:9: note: in expansion of macro ‘skip_prefix’\n 16 | skip_prefix(my_in_var, \"foo\", my_out_var);\n\nbut I don't think the extra complexity and portability headache is worth\nit.\n\n-Peff\n"},{"id":"540668","messageId":"xmqqh5puv20c.fsf@gitster.g","threadId":"65400","inReplyTo":"20260331234220.GD2328529@coredump.intra.peff.net","subject":"Re: [PATCH 04/12] pager: explicitly cast away strchr() constness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-01T20:50:27Z","receivedAt":"2026-04-01T20:50:30Z","isPatch":true,"body":"Jeff King writes:\n\n> When we do:\n>\n> char *cp = strchr(argv[i], '=');\n>\n> it implicitly removes the constness from argv[i]. We need \"cp\" to remain\n> writable (since we overwrite it with a NUL). In theory we should be able\n> to drop the const from argv[i], because it is a sub-pointer into our\n> duplicated pager_env variable.\n>\n> But we get it from split_cmdline(), which uses the traditional \"const\n> char **\" type for argv. This is overly limiting, but changing it would\n> be awkward for all the other callers of split_cmdline().\n\nYeah, it was the first thing that came to my mind that const char\n**argv is the source of the problem. We could cast the pointer we\ngive to split_cmdline() and drop const from argv[] instead, which\nmay make the in-code comment unnecessary but the patch we see here\nis good enough.\n\n\n\n>\n> Let's do an explicit cast with a note about why it is OK. This is enough\n> to silence compiler warnings about the implicit const problems.\n>\n> Signed-off-by: Jeff King \n> ---\n> pager.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/pager.c b/pager.c\n> index 5531fff50e..801ba392f2 100644\n> --- a/pager.c\n> +++ b/pager.c\n> @@ -118,7 +118,8 @@ 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\t/* we know this is writable because it was split from pager_env */\n> +\t\tchar *cp = strchr((char *)argv[i], '=');\n> \n> \t\tif (!cp)\n> \t\t\tdie(\"malformed build-time PAGER_ENV\");\n"},{"id":"540674","messageId":"xmqqcy0iuzop.fsf@gitster.g","threadId":"65400","inReplyTo":"20260331235637.GA2328851@coredump.intra.peff.net","subject":"Re: [PATCH 07/12] pseudo-merge: fix disk reads from find_pseudo_merge()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-01T21:40:38Z","receivedAt":"2026-04-01T21:40:41Z","isPatch":true,"body":"Jeff King writes:\n\n> On Tue, Mar 31, 2026 at 07:46:23PM -0400, Jeff King wrote:\n>\n>> So I think there is something wrong or missing from the test setup, and\n>> this bears further investigation. Sadly the answer to the second part\n>> (\"does it work now\") is still \"no idea\". I _think_ this takes us in a\n>> positive direction, but my goal here is mainly to quiet the compiler\n>> warning. Further bug-hunting on this experimental feature can be done\n>> separately.\n>\n> If this is the wrong direction or if we just want to keep things minimal\n> in this patch series, the absolute smallest fix is probably to cast away\n> the constness explicitly in find_pseudo_merge(), along with a comment\n> that the fix is almost certainly wrong. ;)\n\n;-) Together with the BUG(\"foo\") at the beginning of the function...\n"},{"id":"540675","messageId":"xmqq8qb6uy6g.fsf@gitster.g","threadId":"65400","inReplyTo":"20260401192423.GA2905896@coredump.intra.peff.net","subject":"Re: [PATCH 08/12] skip_prefix(): check const match between in and out params","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-01T22:13:11Z","receivedAt":"2026-04-01T22:13:14Z","isPatch":true,"body":"Jeff King writes:\n\n>> > > +#define CONST_OUTPARAM(in, out) \\\n>> > > +    ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n>> > > +#define skip_prefix(str, prefix, out) \\\n>> > > +    skip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n>> > \n>> > This is clever but it changes the behavior of skip_prefix() which is\n>> > documented as not touching out if it returns false.\n>> \n>> Sorry, I've just realized we always take the other branch so this does not\n>> change the behavior and is in fact a nice solution to the problem.\n>\n> Yeah, exactly. I was curious if the dead branch would be left in place,\n> but gcc seems to prune it even at -O0.\n>\n> I also pondered whether:\n>\n> (*out = in,out)\n>\n> might be a problem, but I think it is OK. The \",\" is a sequence point,\n> so it is well defined (of course we would never run this code anyway,\n> but if we have undefined behavior in the code at all, it may cause\n> confusing effects).\n\nYup, the part I like this the most is that this is still well\ndefined, and the never-taken side of the ternary will not cause us\ntrouble.\n\nVery nicely done.\n"},{"id":"540676","messageId":"xmqq4iluuxxg.fsf@gitster.g","threadId":"65400","inReplyTo":"20260331235136.GI2328529@coredump.intra.peff.net","subject":"Re: [PATCH 09/12] pkt-line: make packet_reader.line non-const","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-01T22:18:35Z","receivedAt":"2026-04-01T22:18:38Z","isPatch":true,"body":"Jeff King writes:\n\n> The \"line\" member of a packet_reader struct is marked as const. This\n> kind of makes sense, because it's not its own allocated buffer that\n> should be freed, and we often use const to indicate that.\n\nThis is interesting. Once we go down this path, will we rethink the\nuse of \"const\" as \"not ours\" hint (which I always found confusing)?\n\n> We can fix it by marking \"line\" as non-const, as well as a few\n> intermediate variables (like \"head\" in the above example). Note that by\n> itself, switching to a non-const variable would cause problems with this\n> line in send-pack.c:\n>\n> if (!skip_prefix(reader->line, \"unpack \", &reader->line))\n>\n> But due to our skip_prefix() magic introduced in the previous commit,\n> this compiles fine (both the in and out-parameters are non-const, so we\n> know it is safe).\n\nOK.\n\n"},{"id":"540677","messageId":"xmqqzf3mtj5z.fsf@gitster.g","threadId":"65400","inReplyTo":"ac0h0xwqLdX5u51v@pks.im","subject":"Re: [PATCH 12/12] refs/files-backend: drop const to fix strchr() warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-01T22:22:48Z","receivedAt":"2026-04-01T22:22:50Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> On Tue, Mar 31, 2026 at 07:53:41PM -0400, Jeff King wrote:\n>> In show_one_reflog_ent(), we're fed a writable strbuf buffer, which we\n>> parse into the various reflog components. We write a NUL over email_end\n>> to tie off one of the fields, and thus email_end must be non-const.\n>> \n>> But with a C23 implementation of libc, strchr() will now complain when\n>> assigning the result to a non-const pointer from a const one. So we can\n>> fix this by making the source pointer non-const.\n>> \n>> But there's a catch. We derive that source pointer by parsing the line\n>> with parse_oid_hex_algop(), which requires a const pointer for its\n>> out-parameter. We can work around that by teaching it to use our\n>> CONST_OUTPARAM() trick, just like skip_prefix(). Note that unlike\n>> skip_prefix(), the function is not inline, so we can't just wrap it\n>> using the same name (otherwise the actual definition would expand the\n>> macro, which breaks compilation). So we rename the actual function with\n>> an \"_impl\" suffix, and callers will all use the macro.\n>\n> Fair. In fact, I was a bit torn with the other commits whether it's nice\n> to reuse the same name. I guess what it buys us is that you cannot\n> accidentally call the wrong function without the guardrails. Even though\n> that's quite unlikely with the `_impl` suffix.\n\nI share the sentiment. If I were deciding the design, I'd even go\nforcing the _impl suffix to everything, including the inline ones,\nas I found the earlier \"strip_prefix()\" example already confusing.\n\n\n"},{"id":"540685","messageId":"20260402035409.GA3492642@coredump.intra.peff.net","threadId":"65400","inReplyTo":"xmqqh5puv20c.fsf@gitster.g","subject":"Re: [PATCH 04/12] pager: explicitly cast away strchr() constness","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T03:54:09Z","receivedAt":"2026-04-02T03:54:16Z","isPatch":true,"body":"On Wed, Apr 01, 2026 at 01:50:27PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > When we do:\n> >\n> > char *cp = strchr(argv[i], '=');\n> >\n> > it implicitly removes the constness from argv[i]. We need \"cp\" to remain\n> > writable (since we overwrite it with a NUL). In theory we should be able\n> > to drop the const from argv[i], because it is a sub-pointer into our\n> > duplicated pager_env variable.\n> >\n> > But we get it from split_cmdline(), which uses the traditional \"const\n> > char **\" type for argv. This is overly limiting, but changing it would\n> > be awkward for all the other callers of split_cmdline().\n> \n> Yeah, it was the first thing that came to my mind that const char\n> **argv is the source of the problem. We could cast the pointer we\n> give to split_cmdline() and drop const from argv[] instead, which\n> may make the in-code comment unnecessary but the patch we see here\n> is good enough.\n\nOoh, that is a good idea. It puts the cast closer to the actual root\ncause. I think there are one or two other touch-ups, so I'll include\nthat in a v2.\n\n-Peff\n"},{"id":"540686","messageId":"20260402035546.GB3492642@coredump.intra.peff.net","threadId":"65400","inReplyTo":"xmqq4iluuxxg.fsf@gitster.g","subject":"Re: [PATCH 09/12] pkt-line: make packet_reader.line non-const","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T03:55:46Z","receivedAt":"2026-04-02T03:55:48Z","isPatch":true,"body":"On Wed, Apr 01, 2026 at 03:18:35PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > The \"line\" member of a packet_reader struct is marked as const. This\n> > kind of makes sense, because it's not its own allocated buffer that\n> > should be freed, and we often use const to indicate that.\n> \n> This is interesting. Once we go down this path, will we rethink the\n> use of \"const\" as \"not ours\" hint (which I always found confusing)?\n\nMaybe. It is already a weak signal, so I consider it more of a hint than\na rule. I think it probably applies more consistently to the return\nvalue of functions (most things returning \"char *\" probably are passing\nback ownership).\n\n-Peff\n"},{"id":"540687","messageId":"20260402035640.GC3492642@coredump.intra.peff.net","threadId":"65400","inReplyTo":"xmqqzf3mtj5z.fsf@gitster.g","subject":"Re: [PATCH 12/12] refs/files-backend: drop const to fix strchr() warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T03:56:40Z","receivedAt":"2026-04-02T03:56:42Z","isPatch":true,"body":"On Wed, Apr 01, 2026 at 03:22:48PM -0700, Junio C Hamano wrote:\n\n> >> But there's a catch. We derive that source pointer by parsing the line\n> >> with parse_oid_hex_algop(), which requires a const pointer for its\n> >> out-parameter. We can work around that by teaching it to use our\n> >> CONST_OUTPARAM() trick, just like skip_prefix(). Note that unlike\n> >> skip_prefix(), the function is not inline, so we can't just wrap it\n> >> using the same name (otherwise the actual definition would expand the\n> >> macro, which breaks compilation). So we rename the actual function with\n> >> an \"_impl\" suffix, and callers will all use the macro.\n> >\n> > Fair. In fact, I was a bit torn with the other commits whether it's nice\n> > to reuse the same name. I guess what it buys us is that you cannot\n> > accidentally call the wrong function without the guardrails. Even though\n> > that's quite unlikely with the `_impl` suffix.\n> \n> I share the sentiment. If I were deciding the design, I'd even go\n> forcing the _impl suffix to everything, including the inline ones,\n> as I found the earlier \"strip_prefix()\" example already confusing.\n\nOK. I think the _impl() is ugly, but the idea is that you never have to\ntype it outside of the macro anyway. I'll switch to that for the others\nfor consistency.\n\n-Peff\n"},{"id":"540688","messageId":"20260402041433.GA3501120@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260331233856.GA2327197@coredump.intra.peff.net","subject":"[PATCH v2 0/12] fixing the remainder of the C23 strchr warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:14:33Z","receivedAt":"2026-04-02T04:14:35Z","isPatch":true,"body":"On Tue, Mar 31, 2026 at 07:38:57PM -0400, Jeff King wrote:\n\n> This series fixes the rest of the warnings you might see on recent glibc\n> or other C23 libc where:\n> [...]\n\nAnd here's a v2 with some minor changes based on review of round 1:\n\n - move cast in patch 4 to split_cmdline(); note that this makes the\n patch totally different from range-diff's perspective\n\n - fix NULL/NUL typo\n\n - use _impl() suffix consistently for wrapped functions. I also\n reordered the macro definitions a bit for readability. Patch 8 also\n does not show up in range-diff (unlike patch 4, you can convince it\n to show it with \"--creation-factor=80\", but the result is not very\n readable anyway).\n\n [01/12]: convert: add const to fix strchr() warnings\n [02/12]: http: add const to fix strchr() warnings\n [03/12]: transport-helper: drop const to fix strchr() warnings\n [04/12]: pager: explicitly cast away strchr() constness\n [05/12]: run-command: explicitly cast away constness when assigning to void\n [06/12]: find_last_dir_sep(): convert inline function to macro\n [07/12]: pseudo-merge: fix disk reads from find_pseudo_merge()\n [08/12]: skip_prefix(): check const match between in and out params\n [09/12]: pkt-line: make packet_reader.line non-const\n [10/12]: range-diff: drop const to fix strstr() warnings\n [11/12]: http: drop const to fix strstr() warning\n [12/12]: refs/files-backend: drop const to fix strchr() warning\n\n builtin/receive-pack.c | 7 ++++---\n convert.c | 3 ++-\n git-compat-util.h | 33 ++++++++++++++++++++++++---------\n hex.c | 6 +++---\n hex.h | 6 ++++--\n http-push.c | 2 +-\n http.c | 4 ++--\n pager.c | 5 +++--\n pkt-line.h | 2 +-\n pseudo-merge.c | 32 +++++++++++++++++++-------------\n range-diff.c | 2 +-\n refs/files-backend.c | 2 +-\n run-command.c | 4 ++--\n send-pack.c | 7 ++++---\n transport-helper.c | 3 ++-\n 15 files changed, 73 insertions(+), 45 deletions(-)\n\n 1: 4b149037ed = 1: 785b9cd5c0 convert: add const to fix strchr() warnings\n 2: 65eb42692e = 2: 6e4b0b2e50 http: add const to fix strchr() warnings\n 3: 659dde8201 ! 3: 42ee9437c2 transport-helper: drop const to fix strchr() warnings\n @@ Commit message\n which causes compilation with some C23 versions of libc (notably recent\n glibc) to complain.\n \n - We need \"p\" to remain writable, since we assign NULL over the space we\n + We need \"p\" to remain writable, since we assign NUL over the space we\n found. We can solve this by also making \"key\" writable. This works\n because it comes from a strbuf, which is itself a writable string.\n \n 4: 66018b5ccf < -: ---------- pager: explicitly cast away strchr() constness\n -: ---------- > 4: 856a72ded8 pager: explicitly cast away strchr() constness\n 5: a696a8a4d9 = 5: 8181dcb941 run-command: explicitly cast away constness when assigning to void\n 6: 32bba4d607 = 6: 2a0830a807 find_last_dir_sep(): convert inline function to macro\n 7: ddd0798d19 = 7: e5b8820cf0 pseudo-merge: fix disk reads from find_pseudo_merge()\n 8: d736d1e070 < -: ---------- skip_prefix(): check const match between in and out params\n -: ---------- > 8: e87d8b70e2 skip_prefix(): check const match between in and out params\n 9: 22c8d65229 = 9: ed84dc926b pkt-line: make packet_reader.line non-const\n10: 5cca7c109c = 10: ff425ef7c3 range-diff: drop const to fix strstr() warnings\n11: 30e7b35073 ! 11: f01ba0616f http: drop const to fix strstr() warning\n @@ Commit message\n Signed-off-by: Jeff King \n \n ## git-compat-util.h ##\n -@@ git-compat-util.h: static inline bool skip_iprefix(const char *str, const char *prefix,\n - \treturn false;\n - }\n - \n +@@ git-compat-util.h: static inline size_t xsize_t(off_t len)\n + * is done via tolower(), so it is strictly ASCII (no multi-byte characters or\n + * locale-specific conversions).\n + */\n +-static inline bool skip_iprefix(const char *str, const char *prefix,\n +-\t\t\t const char **out)\n +#define skip_iprefix(str, prefix, out) \\\n -+\tskip_iprefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n -+\n - /*\n - * Like skip_prefix_mem, but compare case-insensitively. Note that the\n - * comparison is done via tolower(), so it is strictly ASCII (no multi-byte\n ++\tskip_iprefix_impl((str), (prefix), CONST_OUTPARAM((str), (out)))\n ++static inline bool skip_iprefix_impl(const char *str, const char *prefix,\n ++\t\t\t\t const char **out)\n + {\n + \tdo {\n + \t\tif (!*prefix) {\n \n ## http.c ##\n @@ http.c: static int has_proxy_cert_password(void)\n12: 317436aca5 ! 12: d5c971cf26 refs/files-backend: drop const to fix strchr() warning\n @@ Commit message\n But there's a catch. We derive that source pointer by parsing the line\n with parse_oid_hex_algop(), which requires a const pointer for its\n out-parameter. We can work around that by teaching it to use our\n - CONST_OUTPARAM() trick, just like skip_prefix(). Note that unlike\n - skip_prefix(), the function is not inline, so we can't just wrap it\n - using the same name (otherwise the actual definition would expand the\n - macro, which breaks compilation). So we rename the actual function with\n - an \"_impl\" suffix, and callers will all use the macro.\n + CONST_OUTPARAM() trick, just like skip_prefix().\n \n Signed-off-by: Jeff King \n \n @@ hex.h: char *oid_to_hex(const struct object_id *oid);\t\t\t\t\t\t/* same static buffer\n */\n -int parse_oid_hex_algop(const char *hex, struct object_id *oid, const char **end,\n -\t\t\tconst struct git_hash_algo *algo);\n -+int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, const char **end,\n -+\t\t\t const struct git_hash_algo *algo);\n +#define parse_oid_hex_algop(hex, oid, end, algo) \\\n +\tparse_oid_hex_algop_impl((hex), (oid), CONST_OUTPARAM((hex), (end)), (algo))\n ++int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, const char **end,\n ++\t\t\t const struct git_hash_algo *algo);\n \n /*\n * These functions work like get_oid_hex and parse_oid_hex, but they will parse\n"},{"id":"540689","messageId":"20260402041449.GA3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 01/12] convert: add const to fix strchr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:14:49Z","receivedAt":"2026-04-02T04:14:51Z","isPatch":true,"body":"C23 versions of libc (like recent glibc) may provide generic versions of\nstrchr() that match constness between the input and return value. The\nidea being that the compiler can detect when it implicitly converts a\nconst pointer into a non-const one (which then emits a warning).\n\nThere are a few cases here where the result pointer does not need to be\nnon-const at all, and we should mark it as such. That silences the\nwarning (and avoids any potential problems with trying to write via\nthose pointers).\n\nSigned-off-by: Jeff King \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.1172.ge9e20b5838\n\n"},{"id":"540690","messageId":"20260402041451.GB3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 02/12] http: add const to fix strchr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:14:51Z","receivedAt":"2026-04-02T04:14:53Z","isPatch":true,"body":"The \"path\" field of a \"struct repo\" (a custom http-push struct, not to\nbe confused with \"struct repository) is a pointer into a const argv\nstring, and is never written to.\n\nThe compiler does not traditionally complain about assigning from a\nconst pointer because it happens via strchr(). But with some C23 libc\nversions (notably recent glibc), it has started to do so. Let's mark the\nfield as const to silence the warnings.\n\nSigned-off-by: Jeff King \n---\n http-push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 9ae6062198..96df6344ee 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -99,7 +99,7 @@ static struct object_list *objects;\n \n struct repo {\n \tchar *url;\n-\tchar *path;\n+\tconst char *path;\n \tint path_len;\n \tint has_info_refs;\n \tint can_update_info_refs;\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540691","messageId":"20260402041456.GC3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 03/12] transport-helper: drop const to fix strchr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:14:56Z","receivedAt":"2026-04-02T04:14:57Z","isPatch":true,"body":"We implicitly drop the const from our \"key\" variable when we do:\n\n char *p = strchr(key, ' ');\n\nwhich causes compilation with some C23 versions of libc (notably recent\nglibc) to complain.\n\nWe need \"p\" to remain writable, since we assign NUL over the space we\nfound. We can solve this by also making \"key\" writable. This works\nbecause it comes from a strbuf, which is itself a writable string.\n\nSigned-off-by: Jeff King \n---\n transport-helper.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 4d95d84f9e..4614036c99 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -781,7 +781,8 @@ static int push_update_ref_status(struct strbuf *buf,\n \n \tif (starts_with(buf->buf, \"option \")) {\n \t\tstruct object_id old_oid, new_oid;\n-\t\tconst char *key, *val;\n+\t\tchar *key;\n+\t\tconst char *val;\n \t\tchar *p;\n \n \t\tif (!state->hint || !(state->report || state->new_report))\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540692","messageId":"20260402041458.GD3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 04/12] pager: explicitly cast away strchr() constness","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:14:58Z","receivedAt":"2026-04-02T04:15:00Z","isPatch":true,"body":"When we do:\n\n char *cp = strchr(argv[i], '=');\n\nit implicitly removes the constness from argv[i]. We need \"cp\" to remain\nwritable (since we overwrite it with a NUL). In theory we should be able\nto drop the const from argv[i], because it is a sub-pointer into our\nduplicated pager_env variable.\n\nBut we get it from split_cmdline(), which uses the traditional \"const\nchar **\" type for argv. This is overly limiting, but changing it would\nbe awkward for all the other callers of split_cmdline().\n\nLet's do an explicit cast with a note about why it is OK. This is enough\nto silence compiler warnings about the implicit const problems.\n\nSigned-off-by: Jeff King \n---\n pager.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 5531fff50e..35b210e048 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -108,10 +108,11 @@ const char *git_pager(struct repository *r, int stdout_is_tty)\n \n static void setup_pager_env(struct strvec *env)\n {\n-\tconst char **argv;\n+\tchar **argv;\n \tint i;\n \tchar *pager_env = xstrdup(PAGER_ENV);\n-\tint n = split_cmdline(pager_env, &argv);\n+\t/* split_cmdline splits in place, so we know the result is writable */\n+\tint n = split_cmdline(pager_env, (const char ***)&argv);\n \n \tif (n < 0)\n \t\tdie(\"malformed build-time PAGER_ENV: %s\",\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540693","messageId":"20260402041501.GE3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 05/12] run-command: explicitly cast away constness when assigning to void","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:01Z","receivedAt":"2026-04-02T04:15:03Z","isPatch":true,"body":"We do this:\n\n char *equals = strchr(*e, '=');\n\nwhich implicitly removes the constness from \"*e\" and cause the compiler\nto complain. We never write to \"equals\", but later assign it to a\nstring_list util field, which is defined as non-const \"void *\".\n\nWe have to cast somewhere, but doing so at the assignment to util is the\nleast-bad place, since that is the source of the confusion. Sadly we are\nstill open to accidentally writing to the string via the util pointer,\nbut that is the cost of using void pointers, which lose all type\ninformation.\n\nSigned-off-by: Jeff King \n---\n run-command.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 32c290ee6a..d6980c79b3 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -604,11 +604,11 @@ 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\tconst char *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\tstring_list_insert(&envs, key.buf)->util = (void *)(equals + 1);\n \t\t} else {\n \t\t\tstring_list_insert(&envs, *e)->util = NULL;\n \t\t}\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540694","messageId":"20260402041503.GF3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 06/12] find_last_dir_sep(): convert inline function to macro","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:03Z","receivedAt":"2026-04-02T04:15:04Z","isPatch":true,"body":"The find_last_dir_sep() function is implemented as an inline function\nwhich takes in a \"const char *\" and returns a \"char *\" via strrchr().\nThat means that just like strrchr(), it quietly removes the const from\nour pointer, which could lead to accidentally writing to the resulting\nstring.\n\nBut C23 versions of libc (including recent glibc) annotate strrchr()\nsuch that the compiler can detect when const is implicitly lost, and it\nnow complains about the call in this inline function.\n\nWe can't just switch the return type of the function to \"const char *\",\nthough. Some callers really do want a non-const string to be returned\n(and are OK because they are feeding a non-const string into the\nfunction).\n\nThe most general solution is for us to annotate find_last_dir_sep() in\nthe same way that is done for strrchr(). But doing so relies on using\nC23 generics, which we do not otherwise require.\n\nSince this inline function is wrapping a single call to strrchr(), we\ncan take a shortcut. If we implement it as a macro, then the original\ntype information is still available to strrchr(), and it does the check\nfor us.\n\nNote that this is just one implementation of find_last_dir_sep(). There\nis an alternate implementation in compat/win32/path-utils.h. It doesn't\nsuffer from the same warning, as it does not use strrchr() and just\ncasts away const explicitly. That's not ideal, and eventually we may\nwant to conditionally teach it the same C23 generic trick that strrchr()\nuses. But it has been that way forever, and our goal here is just\nquieting new warnings, not improving const-checking.\n\nSigned-off-by: Jeff King \n---\n git-compat-util.h | 6 +-----\n 1 file changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 4b4ea2498f..4bb59b3101 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -335,11 +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-{\n-\treturn strrchr(path, '/');\n-}\n-#define find_last_dir_sep git_find_last_dir_sep\n+#define find_last_dir_sep(path) strrchr((path), '/')\n #endif\n \n #ifndef has_dir_sep\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540695","messageId":"20260402041505.GG3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 07/12] pseudo-merge: fix disk reads from find_pseudo_merge()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:05Z","receivedAt":"2026-04-02T04:15:07Z","isPatch":true,"body":"The goal of this commit was to fix a const warning when compiling\nwith new versions of glibc, but ended up untangling a much deeper\nproblem.\n\nThe find_pseudo_merge() function does a bsearch() on the \"commits\"\npointer of a pseudo_merge_map. This pointer ultimately comes from memory\nmapped from the on-disk bitmap file, and is thus not writable.\n\nThe \"commits\" array is correctly marked const, but the result from\nbsearch() is returned directly as a non-const pseudo_merge_commit\nstruct. Since new versions of glibc annotate bsearch() in a way that\ndetects the implicit loss of const, the compiler now warns.\n\nMy first instinct was that we should be returning a const struct. That\nrequires apply_pseudo_merges_for_commit() to mark its local pointer as\nconst. But that doesn't work! If the offset field has the high-bit set,\nwe look it up in the extended table via nth_pseudo_merge_ext(). And that\nfunction then feeds our const struct to read_pseudo_merge_commit_at(),\nwhich writes into it by byte-swapping from the on-disk mmap.\n\nBut I think this points to a larger problem with find_pseudo_merge(). It\nis not just that the return value is missing const, but it is missing\nthat byte-swapping! And we know that byte-swapping is needed here,\nbecause the comparator we use for bsearch() also calls our\nread_pseudo_merge_commit_at() helper.\n\nSo I think the interface is all wrong here. We should not be returning a\npointer to a struct which was cast from on-disk data. We should be\nfilling in a caller-provided struct using the bytes we found,\nbyte-swapping the values.\n\nThat of course raises the dual question: how did this ever work, and\ndoes it work now? The answer to the first part is: this code does not\nseem to be triggered in the test suite at all. If we insert a BUG(\"foo\")\ncall into apply_pseudo_merges_for_commit(), it never triggers.\n\nSo I think there is something wrong or missing from the test setup, and\nthis bears further investigation. Sadly the answer to the second part\n(\"does it work now\") is still \"no idea\". I _think_ this takes us in a\npositive direction, but my goal here is mainly to quiet the compiler\nwarning. Further bug-hunting on this experimental feature can be done\nseparately.\n\nSigned-off-by: Jeff King \n---\n pseudo-merge.c | 32 +++++++++++++++++++-------------\n 1 file changed, 19 insertions(+), 13 deletions(-)\n\ndiff --git a/pseudo-merge.c b/pseudo-merge.c\nindex a2d5bd85f9..ff18b6c364 100644\n--- a/pseudo-merge.c\n+++ b/pseudo-merge.c\n@@ -638,31 +638,37 @@ static int pseudo_merge_commit_cmp(const void *va, const void *vb)\n \treturn 0;\n }\n \n-static struct pseudo_merge_commit *find_pseudo_merge(const struct pseudo_merge_map *pm,\n-\t\t\t\t\t\t uint32_t pos)\n+static int find_pseudo_merge(const struct pseudo_merge_map *pm, uint32_t pos,\n+\t\t\t struct pseudo_merge_commit *out)\n {\n+\tconst unsigned char *at;\n+\n \tif (!pm->commits_nr)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n-\treturn bsearch(&pos, pm->commits, pm->commits_nr,\n-\t\t PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp);\n+\tat = bsearch(&pos, pm->commits, pm->commits_nr,\n+\t\t PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp);\n+\tif (!at)\n+\t\treturn 0;\n+\n+\tread_pseudo_merge_commit_at(out, at);\n+\treturn 1;\n }\n \n int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\t\t\t struct bitmap *result,\n \t\t\t\t struct commit *commit, uint32_t commit_pos)\n {\n \tstruct pseudo_merge *merge;\n-\tstruct pseudo_merge_commit *merge_commit;\n+\tstruct pseudo_merge_commit merge_commit;\n \tint ret = 0;\n \n-\tmerge_commit = find_pseudo_merge(pm, commit_pos);\n-\tif (!merge_commit)\n+\tif (!find_pseudo_merge(pm, commit_pos, &merge_commit))\n \t\treturn 0;\n \n-\tif (merge_commit->pseudo_merge_ofs & ((uint64_t)1<<63)) {\n+\tif (merge_commit.pseudo_merge_ofs & ((uint64_t)1<<63)) {\n \t\tstruct pseudo_merge_commit_ext ext = { 0 };\n-\t\toff_t ofs = merge_commit->pseudo_merge_ofs & ~((uint64_t)1<<63);\n+\t\toff_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63);\n \t\tuint32_t i;\n \n \t\tif (pseudo_merge_ext_at(pm, &ext, ofs) < -1) {\n@@ -673,11 +679,11 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\t}\n \n \t\tfor (i = 0; i < ext.nr; i++) {\n-\t\t\tif (nth_pseudo_merge_ext(pm, &ext, merge_commit, i) < 0)\n+\t\t\tif (nth_pseudo_merge_ext(pm, &ext, &merge_commit, i) < 0)\n \t\t\t\treturn ret;\n \n \t\t\tmerge = pseudo_merge_at(pm, &commit->object.oid,\n-\t\t\t\t\t\tmerge_commit->pseudo_merge_ofs);\n+\t\t\t\t\t\tmerge_commit.pseudo_merge_ofs);\n \n \t\t\tif (!merge)\n \t\t\t\treturn ret;\n@@ -687,7 +693,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm,\n \t\t}\n \t} else {\n \t\tmerge = pseudo_merge_at(pm, &commit->object.oid,\n-\t\t\t\t\tmerge_commit->pseudo_merge_ofs);\n+\t\t\t\t\tmerge_commit.pseudo_merge_ofs);\n \n \t\tif (!merge)\n \t\t\treturn ret;\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540696","messageId":"20260402041507.GH3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 08/12] skip_prefix(): check const match between in and out params","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:07Z","receivedAt":"2026-04-02T04:15:09Z","isPatch":true,"body":"The skip_prefix() function takes in a \"const char *\" string, and returns\nvia a \"const char **\" out-parameter that points somewhere in that\nstring. This is fine if you are operating on a const string, like:\n\n const char *in = ...;\n const char *out;\n if (skip_prefix(in, \"foo\", &out))\n\t...look at out...\n\nIt is also OK if \"in\" is not const but \"out\" is, as we add an implicit\nconst when we pass \"in\" to the function. But there's another case where\nthis is limiting. If we want both fields to be non-const, like:\n\n char *in = ...;\n char *out;\n if (skip_prefix(in, \"foo\", &out))\n\t*out = '\\0';\n\nit doesn't work. The compiler will complain about the type mismatch in\npassing \"&out\" to a parameter which expects \"const char **\". So to make\nthis work, we have to do an explicit cast.\n\nBut such a cast is ugly, and also means that we run afoul of making this\nmistake:\n\n const char *in = ...;\n char *out;\n if (skip_prefix(in, \"foo\", (const char **)&out))\n\t*out = '\\0';\n\nwhich causes us to write to the memory pointed by \"in\", which was const.\n\nWe can imagine these four cases as:\n\n (1) const in, const out\n (2) non-const in, const out\n (3) non-const in, non-const out\n (4) const in, non-const out\n\nCases (1) and (2) work now. We would like case (3) to work but it\ndoesn't. But we would like to catch case (4) as a compile error.\n\nSo ideally the rule is \"the out-parameter must be at least as const as\nthe in-parameter\". We can do this with some macro trickery. We wrap\nskip_prefix() in a macro so that it has access to the real types of\nin/out. And then we pass those parameters through another macro which:\n\n 1. Fails if the \"at least as const\" rule is not filled.\n\n 2. Casts to match the signature of the real skip_prefix().\n\nThere are a lot of ways to implement the \"fails\" part. You can use\n__builtin_types_compatible_p() to check, and then either our\nBUILD_ASSERT macros or _Static_assert to fail. But that requires some\nconditional compilation based on compiler feature. That's probably OK\n(the fallback would be to just cast without catching case 4). But we can\ndo better.\n\nThe macro I have here uses a ternary with a dead branch that tries to\nassign \"in\" to \"out\", which should work everywhere and lets the compiler\ncatch the problem in the usual way. With an input like this:\n\n int foo(const char *x, const char **y);\n #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out)))\n\n void ok_const(const char *x, const char **y)\n {\n foo(x, y);\n }\n\n void ok_nonconst(char *x, char **y)\n {\n foo(x, y);\n }\n\n void ok_add_const(char *x, const char **y)\n {\n foo(x, y);\n }\n\n void bad_drop_const(const char *x, char **y)\n {\n foo(x, y);\n }\n\ngcc reports:\n\n foo.c: In function ‘bad_drop_const’:\n foo.c:2:35: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]\n 2 | ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n | ^\n foo.c:4:31: note: in expansion of macro ‘CONST_OUTPARAM’\n 4 | #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out)))\n | ^~~~~~~~~~~~~~\n foo.c:23:9: note: in expansion of macro ‘foo’\n 23 | foo(x, y);\n | ^~~\n\nIt's a bit verbose, but I think makes it reasonably clear what's going\non. Using BUILD_ASSERT_OR_ZERO() ends up much worse. Using\n_Static_assert you can be a bit more informative, but that's not\nsomething we use at all yet in our code-base (it's an old gnu-ism later\nstandardized in C11).\n\nOur generic macro only works for \"const char **\", which is something we\ncould improve by using typeof(in). But that introduces more portability\nquestions, and also some weird corner cases (e.g., around implicit void\nconversion).\n\nThis patch just introduces the concept. We'll make use of it in future\npatches.\n\nNote that we rename skip_prefix() to skip_prefix_impl() here, to avoid\nexpanding the macro when defining the function. That's not strictly\nnecessary since we could just define the macro after defining the inline\nfunction. But that would not be the case for a non-inline function (and\nwe will apply this technique to them later, and should be consistent).\nIt also gives us more freedom about where to define the macro. I did so\nright above the definition here, which I think keeps the relevant bits\ntogether.\n\nSigned-off-by: Jeff King \n---\n git-compat-util.h | 21 +++++++++++++++++++--\n 1 file changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 4bb59b3101..e9629b2a9d 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -463,6 +463,21 @@ void set_warn_routine(report_fn routine);\n report_fn get_warn_routine(void);\n void set_die_is_recursing_routine(int (*routine)(void));\n \n+/*\n+ * Check that an out-parameter that is \"at least as const as\" a matching\n+ * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n+ * of \"str\". So:\n+ *\n+ * const str, const out: ok\n+ * non-const str, const out: ok\n+ * non-const str, non-const out: ok\n+ * const str, non-const out: compile error\n+ *\n+ * See the skip_prefix macro below for an example of use.\n+ */\n+#define CONST_OUTPARAM(in, out) \\\n+ ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n+\n /*\n * If the string \"str\" begins with the string found in \"prefix\", return true.\n * The \"out\" parameter is set to \"str + strlen(prefix)\" (i.e., to the point in\n@@ -479,8 +494,10 @@ void set_die_is_recursing_routine(int (*routine)(void));\n * [skip prefix if present, otherwise use whole string]\n * skip_prefix(name, \"refs/heads/\", &name);\n */\n-static inline bool skip_prefix(const char *str, const char *prefix,\n-\t\t\t const char **out)\n+#define skip_prefix(str, prefix, out) \\\n+\tskip_prefix_impl((str), (prefix), CONST_OUTPARAM((str), (out)))\n+static inline bool skip_prefix_impl(const char *str, const char *prefix,\n+\t\t\t\t const char **out)\n {\n \tdo {\n \t\tif (!*prefix) {\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540697","messageId":"20260402041510.GI3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 09/12] pkt-line: make packet_reader.line non-const","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:10Z","receivedAt":"2026-04-02T04:15:11Z","isPatch":true,"body":"The \"line\" member of a packet_reader struct is marked as const. This\nkind of makes sense, because it's not its own allocated buffer that\nshould be freed, and we often use const to indicate that. But it is\nalways writable, because it points into the non-const \"buffer\" member.\n\nAnd we rely on this writability in places like send-pack and\nreceive-pack, where we parse incoming packet contents by writing NULs\nover delimiters. This has traditionally worked because we implicitly\ncast away the constness with strchr() like:\n\n const char *head;\n char *p;\n\n head = reader->line;\n p = strchr(head, ' ');\n\nSince C23 libc provides a generic strchr() to detect this implicit\nconst removal, this now generate a compiler warning on some platforms\n(like recent glibc).\n\nWe can fix it by marking \"line\" as non-const, as well as a few\nintermediate variables (like \"head\" in the above example). Note that by\nitself, switching to a non-const variable would cause problems with this\nline in send-pack.c:\n\n if (!skip_prefix(reader->line, \"unpack \", &reader->line))\n\nBut due to our skip_prefix() magic introduced in the previous commit,\nthis compiles fine (both the in and out-parameters are non-const, so we\nknow it is safe).\n\nSigned-off-by: Jeff King \n---\n builtin/receive-pack.c | 7 ++++---\n pkt-line.h | 2 +-\n send-pack.c | 7 ++++---\n 3 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e34edff406..a6af16c4e7 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1025,8 +1025,8 @@ static int read_proc_receive_report(struct packet_reader *reader,\n \n \tfor (;;) {\n \t\tstruct object_id old_oid, new_oid;\n-\t\tconst char *head;\n-\t\tconst char *refname;\n+\t\tchar *head;\n+\t\tchar *refname;\n \t\tchar *p;\n \t\tenum packet_read_status status;\n \n@@ -1050,7 +1050,8 @@ static int read_proc_receive_report(struct packet_reader *reader,\n \t\t}\n \t\t*p++ = '\\0';\n \t\tif (!strcmp(head, \"option\")) {\n-\t\t\tconst char *key, *val;\n+\t\t\tchar *key;\n+\t\t\tconst char *val;\n \n \t\t\tif (!hint || !(report || new_report)) {\n \t\t\t\tif (!once++)\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 3b33cc64f3..e6cf85e34e 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -184,7 +184,7 @@ struct packet_reader {\n \tint pktlen;\n \n \t/* the last line read */\n-\tconst char *line;\n+\tchar *line;\n \n \t/* indicates if a line has been peeked */\n \tint line_peeked;\ndiff --git a/send-pack.c b/send-pack.c\nindex 07ecfae4de..b4361d5610 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -175,8 +175,8 @@ static int receive_status(struct repository *r,\n \tret = receive_unpack_status(reader);\n \twhile (1) {\n \t\tstruct object_id old_oid, new_oid;\n-\t\tconst char *head;\n-\t\tconst char *refname;\n+\t\tchar *head;\n+\t\tchar *refname;\n \t\tchar *p;\n \t\tif (packet_reader_read(reader) != PACKET_READ_NORMAL)\n \t\t\tbreak;\n@@ -190,7 +190,8 @@ static int receive_status(struct repository *r,\n \t\t*p++ = '\\0';\n \n \t\tif (!strcmp(head, \"option\")) {\n-\t\t\tconst char *key, *val;\n+\t\t\tchar *key;\n+\t\t\tconst char *val;\n \n \t\t\tif (!hint || !(report || new_report)) {\n \t\t\t\tif (!once++)\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540698","messageId":"20260402041512.GJ3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 10/12] range-diff: drop const to fix strstr() warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:12Z","receivedAt":"2026-04-02T04:15:13Z","isPatch":true,"body":"This is another case where we implicitly drop the \"const\" from a pointer\nby feeding it to strstr() and assigning the result to a non-const\npointer. This is OK in practice, since the const pointer originally\ncomes from a writable source (a strbuf), but C23 libc implementations\nhave started to complain about it.\n\nWe do write to the output pointer, so it needs to remain non-const. We\ncan just switch the input pointer to also be non-const in this case. By\nitself that would run into problems with calls to skip_prefix(), but\nsince that function has now been taught to match in/out constness\nautomatically, it just works without us doing anything further.\n\nSigned-off-by: Jeff King \n---\n range-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 2712a9a107..8e2dd2eb19 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -88,7 +88,7 @@ static int read_patches(const char *range, struct string_list *list,\n \tline = contents.buf;\n \tsize = contents.len;\n \tfor (; size > 0; size -= len, line += len) {\n-\t\tconst char *p;\n+\t\tchar *p;\n \t\tchar *eol;\n \n \t\teol = memchr(line, '\\n', size);\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540699","messageId":"20260402041514.GK3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 11/12] http: drop const to fix strstr() warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:14Z","receivedAt":"2026-04-02T04:15:15Z","isPatch":true,"body":"In redact_sensitive_header(), a C23 implementation of libc will complain\nthat strstr() assigns the result from \"const char *cookie\" to \"char\n*semicolon\".\n\nUltimately the memory is writable. We're fed a strbuf, generate a const\npointer \"sensitive_header\" within it using skip_iprefix(), and then\nassign the result to \"cookie\". So we can solve this by dropping the\nconst from \"cookie\" and \"sensitive_header\".\n\nHowever, this runs afoul of skip_iprefix(), which wants a \"const char\n**\" for its out-parameter. We can solve that by teaching skip_iprefix()\nthe same \"make sure out is at least as const as in\" magic that we\nrecently taught to skip_prefix().\n\nSigned-off-by: Jeff King \n---\n git-compat-util.h | 6 ++++--\n http.c | 4 ++--\n 2 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex e9629b2a9d..4ddac61992 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -902,8 +902,10 @@ static inline size_t xsize_t(off_t len)\n * is done via tolower(), so it is strictly ASCII (no multi-byte characters or\n * locale-specific conversions).\n */\n-static inline bool skip_iprefix(const char *str, const char *prefix,\n-\t\t\t const char **out)\n+#define skip_iprefix(str, prefix, out) \\\n+\tskip_iprefix_impl((str), (prefix), CONST_OUTPARAM((str), (out)))\n+static inline bool skip_iprefix_impl(const char *str, const char *prefix,\n+\t\t\t\t const char **out)\n {\n \tdo {\n \t\tif (!*prefix) {\ndiff --git a/http.c b/http.c\nindex d8d016891b..67c9c6fc60 100644\n--- a/http.c\n+++ b/http.c\n@@ -748,7 +748,7 @@ static int has_proxy_cert_password(void)\n static int redact_sensitive_header(struct strbuf *header, size_t offset)\n {\n \tint ret = 0;\n-\tconst char *sensitive_header;\n+\tchar *sensitive_header;\n \n \tif (trace_curl_redact &&\n \t (skip_iprefix(header->buf + offset, \"Authorization:\", &sensitive_header) ||\n@@ -765,7 +765,7 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset)\n \t} else if (trace_curl_redact &&\n \t\t skip_iprefix(header->buf + offset, \"Cookie:\", &sensitive_header)) {\n \t\tstruct strbuf redacted_header = STRBUF_INIT;\n-\t\tconst char *cookie;\n+\t\tchar *cookie;\n \n \t\twhile (isspace(*sensitive_header))\n \t\t\tsensitive_header++;\n-- \n2.53.0.1172.ge9e20b5838\n\n"},{"id":"540700","messageId":"20260402041516.GL3501239@coredump.intra.peff.net","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"[PATCH v2 12/12] refs/files-backend: drop const to fix strchr() warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T04:15:16Z","receivedAt":"2026-04-02T04:15:18Z","isPatch":true,"body":"In show_one_reflog_ent(), we're fed a writable strbuf buffer, which we\nparse into the various reflog components. We write a NUL over email_end\nto tie off one of the fields, and thus email_end must be non-const.\n\nBut with a C23 implementation of libc, strchr() will now complain when\nassigning the result to a non-const pointer from a const one. So we can\nfix this by making the source pointer non-const.\n\nBut there's a catch. We derive that source pointer by parsing the line\nwith parse_oid_hex_algop(), which requires a const pointer for its\nout-parameter. We can work around that by teaching it to use our\nCONST_OUTPARAM() trick, just like skip_prefix().\n\nSigned-off-by: Jeff King \n---\n hex.c | 6 +++---\n hex.h | 6 ++++--\n refs/files-backend.c | 2 +-\n 3 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/hex.c b/hex.c\nindex 865a232167..bc756722ca 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -54,9 +54,9 @@ int get_oid_hex(const char *hex, struct object_id *oid)\n \treturn get_oid_hex_algop(hex, oid, the_hash_algo);\n }\n \n-int parse_oid_hex_algop(const char *hex, struct object_id *oid,\n-\t\t\tconst char **end,\n-\t\t\tconst struct git_hash_algo *algop)\n+int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid,\n+\t\t\t const char **end,\n+\t\t\t const struct git_hash_algo *algop)\n {\n \tint ret = get_oid_hex_algop(hex, oid, algop);\n \tif (!ret)\ndiff --git a/hex.h b/hex.h\nindex e9ccb54065..1e9a65d83a 100644\n--- a/hex.h\n+++ b/hex.h\n@@ -40,8 +40,10 @@ char *oid_to_hex(const struct object_id *oid);\t\t\t\t\t\t/* same static buffer */\n * other invalid character. end is only updated on success; otherwise, it is\n * unmodified.\n */\n-int parse_oid_hex_algop(const char *hex, struct object_id *oid, const char **end,\n-\t\t\tconst struct git_hash_algo *algo);\n+#define parse_oid_hex_algop(hex, oid, end, algo) \\\n+\tparse_oid_hex_algop_impl((hex), (oid), CONST_OUTPARAM((hex), (end)), (algo))\n+int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, const char **end,\n+\t\t\t const struct git_hash_algo *algo);\n \n /*\n * These functions work like get_oid_hex and parse_oid_hex, but they will parse\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 0537a72b2a..b3b0c25f84 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2190,7 +2190,7 @@ static int show_one_reflog_ent(struct files_ref_store *refs,\n \tchar *email_end, *message;\n \ttimestamp_t timestamp;\n \tint tz;\n-\tconst char *p = sb->buf;\n+\tchar *p = sb->buf;\n \n \t/* old SP new SP name SP time TAB msg LF */\n \tif (!sb->len || sb->buf[sb->len - 1] != '\\n' ||\n-- \n2.53.0.1172.ge9e20b5838\n"},{"id":"540701","messageId":"xmqqeckyt08e.fsf@gitster.g","threadId":"65400","inReplyTo":"20260402041507.GH3501239@coredump.intra.peff.net","subject":"Re: [PATCH v2 08/12] skip_prefix(): check const match between in and out params","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-02T05:11:45Z","receivedAt":"2026-04-02T05:11:48Z","isPatch":true,"body":"Jeff King writes:\n\n> +/*\n> + * Check that an out-parameter that is \"at least as const as\" a matching\n> + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n> + * of \"str\". So:\n\nSorry for not mentioning earlier, but I couldn't quite parse the\nabove with \"that\" immediately after \"out-parameter\". I am guessing\nthat you wanted to say an equivalent of\n\n Check that an out-parameter \"out\" is at least as const as a\n matching in-parameter \"in\".\n\n> + *\n> + * const str, const out: ok\n> + * non-const str, const out: ok\n> + * non-const str, non-const out: ok\n> + * const str, non-const out: compile error\n> + *\n> + * See the skip_prefix macro below for an example of use.\n> + */\n> +#define CONST_OUTPARAM(in, out) \\\n> + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n> +\n> /*\n> * If the string \"str\" begins with the string found in \"prefix\", return true.\n> * The \"out\" parameter is set to \"str + strlen(prefix)\" (i.e., to the point in\n> @@ -479,8 +494,10 @@ void set_die_is_recursing_routine(int (*routine)(void));\n> * [skip prefix if present, otherwise use whole string]\n> * skip_prefix(name, \"refs/heads/\", &name);\n> */\n> -static inline bool skip_prefix(const char *str, const char *prefix,\n> -\t\t\t const char **out)\n> +#define skip_prefix(str, prefix, out) \\\n> +\tskip_prefix_impl((str), (prefix), CONST_OUTPARAM((str), (out)))\n> +static inline bool skip_prefix_impl(const char *str, const char *prefix,\n> +\t\t\t\t const char **out)\n> {\n> \tdo {\n> \t\tif (!*prefix) {\n"},{"id":"540704","messageId":"20260402060119.GA3504521@coredump.intra.peff.net","threadId":"65400","inReplyTo":"xmqqeckyt08e.fsf@gitster.g","subject":"Re: [PATCH v2 08/12] skip_prefix(): check const match between in and out params","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-02T06:01:19Z","receivedAt":"2026-04-02T06:01:21Z","isPatch":true,"body":"On Wed, Apr 01, 2026 at 10:11:45PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > +/*\n> > + * Check that an out-parameter that is \"at least as const as\" a matching\n> > + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n> > + * of \"str\". So:\n> \n> Sorry for not mentioning earlier, but I couldn't quite parse the\n> above with \"that\" immediately after \"out-parameter\". I am guessing\n> that you wanted to say an equivalent of\n> \n> Check that an out-parameter \"out\" is at least as const as a\n> matching in-parameter \"in\".\n\nYes, it's just a typo. What you wrote is what I meant. Can you fix it up\nwhile applying?\n\n-Peff\n"},{"id":"540767","messageId":"49834347-8049-4a79-8bdd-4da1ec1ebac0@gmail.com","threadId":"65400","inReplyTo":"20260401192423.GA2905896@coredump.intra.peff.net","subject":"Re: [PATCH 08/12] skip_prefix(): check const match between in and out params","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-02T15:05:49Z","receivedAt":"2026-04-02T15:05:52Z","isPatch":true,"body":"On 01/04/2026 20:24, Jeff King wrote:\n> On Wed, Apr 01, 2026 at 03:04:10PM +0100, Phillip Wood wrote:\n> \n>> On 01/04/2026 14:17, Phillip Wood wrote:\n>>> On 01/04/2026 00:50, Jeff King wrote:\n>>>>\n>>>> +/*\n>>>> + * Check that an out-parameter that is \"at least as const as\" a matching\n>>>> + * in-parameter. For example, skip_prefix() will return \"out\" that\n>>>> is a subset\n>>>> + * of \"str\". So:\n>>>> + *\n>>>> + *  const str, const out: ok\n>>>> + *  non-const str, const out: ok\n>>>> + *  non-const str, non-const out: ok\n>>>> + *  const str, non-const out: compile error\n>>>> + *\n>>>> + *  See the skip_prefix macro below for an example of use.\n>>>> + */\n>>>> +#define CONST_OUTPARAM(in, out) \\\n>>>> +    ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n>>>> +#define skip_prefix(str, prefix, out) \\\n>>>> +    skip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))\n>>>\n>>> This is clever but it changes the behavior of skip_prefix() which is\n>>> documented as not touching out if it returns false.\n>>\n>> Sorry, I've just realized we always take the other branch so this does not\n>> change the behavior and is in fact a nice solution to the problem.\n> \n> Yeah, exactly. I was curious if the dead branch would be left in place,\n> but gcc seems to prune it even at -O0.\n> \n> I also pondered whether:\n> \n> (*out = in,out)\n> \n> might be a problem, but I think it is OK. The \",\" is a sequence point,\n> so it is well defined (of course we would never run this code anyway,\n> but if we have undefined behavior in the code at all, it may cause\n> confusing effects).\n\nI agree it's well defined and because this code is never executed we \ndon't need to worry about evaluating \"out\" multiple times. While the \nmessage from using _Static_assert below is nicer I think having this \nwhich is simple and works with all compilers is a better trade off.\n\nThanks\n\nPhillip\n\n> For reference, this is the more complicated one I came up with:\n> \n> /*\n> * Note that builtin_types_compatible_p() counts \"char\" and \"const\n> * char\" as the same type. So we deref and construct our own pointer\n> * with const to find out it \"x\" is const, and then either compare\n> * x and y exactly (if it is const, they must both be) or dereferenced\n> * (which lets y be either const or not).\n> */\n> #define CONST_COMPATIBLE(x, y) \\\n> (__builtin_types_compatible_p(typeof(x), const typeof(*(x)) *) ? \\\n> __builtin_types_compatible_p(typeof(x), typeof(y)) : \\\n> __builtin_types_compatible_p(typeof(*(x)), typeof(*(y))))\n> \n> I also tried using a gcc statement-expression and _Static_assert to get\n> a nicer message, like this:\n> \n> #define CONST_OUTPARAM(in, out, in_name, out_name) ({ \\\n> _Static_assert(CONST_COMPATIBLE((in),*(out)), \\\n> in_name \" is not const-compatible with \" out_name); \\\n> (const typeof(*(in)) **)(out); \\\n> })\n> #define skip_prefix(str, prefix, out) \\\n> skip_prefix(str, prefix, CONST_OUTPARAM((str), (out), #str, #out))\n> \n> It does produce slightly nicer output:\n> \n> foo.c: In function ‘bad’:\n> foo.c:8:9: error: static assertion failed: \"my_in_var is not const-compatible with my_out_var\"\n> 8 | _Static_assert(CONST_COMPATIBLE((in),*(out)), \\\n> | ^~~~~~~~~~~~~~\n> foo.c:13:34: note: in expansion of macro ‘CONST_OUTPARAM’\n> 13 | skip_prefix(str, prefix, CONST_OUTPARAM((str), (out), #str, #out))\n> | ^~~~~~~~~~~~~~\n> foo.c:16:9: note: in expansion of macro ‘skip_prefix’\n> 16 | skip_prefix(my_in_var, \"foo\", my_out_var);\n> \n> but I don't think the extra complexity and portability headache is worth\n> it.\n> \n> -Peff\n\n"},{"id":"540770","messageId":"xmqq1pgxtlnm.fsf@gitster.g","threadId":"65400","inReplyTo":"xmqqeckyt08e.fsf@gitster.g","subject":"Re: [PATCH v2 08/12] skip_prefix(): check const match between in and out params","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-02T15:41:17Z","receivedAt":"2026-04-02T15:41:21Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> +/*\n>> + * Check that an out-parameter that is \"at least as const as\" a matching\n>> + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n>> + * of \"str\". So:\n>\n> Sorry for not mentioning earlier, but I couldn't quite parse the\n> above with \"that\" immediately after \"out-parameter\". I am guessing\n> that you wanted to say an equivalent of\n>\n> Check that an out-parameter \"out\" is at least as const as a\n> matching in-parameter \"in\".\n\nWhat I meant was that I'd understand if that \"that\" immediately\nafter the \"out-parameter\" is removed from the sentence.\n\nThanks.\n"},{"id":"540771","messageId":"xmqqqzoxs6ni.fsf@gitster.g","threadId":"65400","inReplyTo":"20260402060119.GA3504521@coredump.intra.peff.net","subject":"Re: [PATCH v2 08/12] skip_prefix(): check const match between in and out params","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-02T15:50:41Z","receivedAt":"2026-04-02T15:50:43Z","isPatch":true,"body":"Jeff King writes:\n\n> On Wed, Apr 01, 2026 at 10:11:45PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > +/*\n>> > + * Check that an out-parameter that is \"at least as const as\" a matching\n>> > + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n>> > + * of \"str\". So:\n>> \n>> Sorry for not mentioning earlier, but I couldn't quite parse the\n>> above with \"that\" immediately after \"out-parameter\". I am guessing\n>> that you wanted to say an equivalent of\n>> \n>> Check that an out-parameter \"out\" is at least as const as a\n>> matching in-parameter \"in\".\n>\n> Yes, it's just a typo. What you wrote is what I meant. Can you fix it up\n> while applying?\n\nWill do. Thanks.\n"},{"id":"540808","messageId":"ac8BE4StG2bJbFFc@nand.local","threadId":"65400","inReplyTo":"20260331235637.GA2328851@coredump.intra.peff.net","subject":"Re: [PATCH 07/12] pseudo-merge: fix disk reads from find_pseudo_merge()","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-04-02T23:51:47Z","receivedAt":"2026-04-02T23:51:50Z","isPatch":true,"body":"On Tue, Mar 31, 2026 at 07:56:37PM -0400, Jeff King wrote:\n> On Tue, Mar 31, 2026 at 07:46:23PM -0400, Jeff King wrote:\n>\n> > So I think there is something wrong or missing from the test setup, and\n> > this bears further investigation. Sadly the answer to the second part\n> > (\"does it work now\") is still \"no idea\". I _think_ this takes us in a\n> > positive direction, but my goal here is mainly to quiet the compiler\n> > warning. Further bug-hunting on this experimental feature can be done\n> > separately.\n>\n> If this is the wrong direction or if we just want to keep things minimal\n> in this patch series, the absolute smallest fix is probably to cast away\n> the constness explicitly in find_pseudo_merge(), along with a comment\n> that the fix is almost certainly wrong. ;)\n\nThis approach makes the most sense to me as a band-aid fix to squelch\nthe Coverity warnings.\n\nI pulled on this thread a little bit over the past couple of evenings\nand found a fair number of pseudo-merge related bugs / oddities that I'd\nlike to fix more comprehensively.\n\nBut this is makes sense as a first step to quiet the noise from Coverity\nwithout rushing the other fixes.\n\nThanks,\nTaylor\n"},{"id":"540849","messageId":"87h5ps5mbc.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set","threadId":"65400","inReplyTo":"20260402041507.GH3501239@coredump.intra.peff.net","subject":"Re: [PATCH v2 08/12] skip_prefix(): check const match between in and out params","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-04-03T11:13:11Z","receivedAt":"2026-04-03T11:13:23Z","isPatch":true,"body":"Jeff King writes:\n\n> There are a lot of ways to implement the \"fails\" part. You can use\n> __builtin_types_compatible_p() to check, and then either our\n> BUILD_ASSERT macros or _Static_assert to fail. But that requires some\n> conditional compilation based on compiler feature. That's probably OK\n> (the fallback would be to just cast without catching case 4). But we can\n> do better.\n>\n> The macro I have here uses a ternary with a dead branch that tries to\n> assign \"in\" to \"out\", \n\nI didn't know this pattern before, but is seems it's used in other\nplaces as well. Clever.\n\n> +/*\n> + * Check that an out-parameter that is \"at least as const as\" a matching\n> + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n> + * of \"str\". So:\n> + *\n> + * const str, const out: ok\n> + * non-const str, const out: ok\n> + * non-const str, non-const out: ok\n> + * const str, non-const out: compile error\n> + *\n> + * See the skip_prefix macro below for an example of use.\n> + */\n> +#define CONST_OUTPARAM(in, out) \\\n> + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n\nI'm not sure it matters, but this is indented with spaces\n\n-- \nCheers,\nToon\n"},{"id":"540850","messageId":"87cy0g5m8s.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set","threadId":"65400","inReplyTo":"20260402041433.GA3501120@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/12] fixing the remainder of the C23 strchr warnings","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-04-03T11:14:43Z","receivedAt":"2026-04-03T11:14:51Z","isPatch":true,"body":"Jeff King writes:\n\n> And here's a v2 with some minor changes based on review of round 1:\n\nThanks for all these fixes. On top of the fixes you've made that are\nalready in `next`, this makes my compiler and me happy.\n\nI've posted one nit about indenting, but other than that, all looks good\nto me.\n\n-- \nCheers,\nToon\n"},{"id":"540869","messageId":"20260404054211.GA1346444@coredump.intra.peff.net","threadId":"65400","inReplyTo":"87h5ps5mbc.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set","subject":"[PATCH v2 13/12] git-compat-util: fix CONST_OUTPARAM typo and indentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-04T05:42:11Z","receivedAt":"2026-04-04T05:42:19Z","isPatch":true,"body":"On Fri, Apr 03, 2026 at 01:13:11PM +0200, Toon Claes wrote:\n\n> > +/*\n> > + * Check that an out-parameter that is \"at least as const as\" a matching\n> > + * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n> > + * of \"str\". So:\n> > + *\n> > + * const str, const out: ok\n> > + * non-const str, const out: ok\n> > + * non-const str, non-const out: ok\n> > + * const str, non-const out: compile error\n> > + *\n> > + * See the skip_prefix macro below for an example of use.\n> > + */\n> > +#define CONST_OUTPARAM(in, out) \\\n> > + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n> \n> I'm not sure it matters, but this is indented with spaces\n\nThis is in 'next', so we'd need a patch on top. Maybe not worth it on\nits own, but we also missed the typo-fix that Junio pointed out earlier.\n\nSo maybe this on top:\n\n-- >8 --\nSubject: [PATCH] git-compat-util: fix CONST_OUTPARAM typo and indentation\n\nThere's a typo in the comment, making it hard to understand. And the\nmacro itself is indented with spaces rather than tab.\n\nSigned-off-by: Jeff King \n---\n git-compat-util.h | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 4ddac61992..ae1bdc90a4 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -464,7 +464,7 @@ report_fn get_warn_routine(void);\n void set_die_is_recursing_routine(int (*routine)(void));\n \n /*\n- * Check that an out-parameter that is \"at least as const as\" a matching\n+ * Check that an out-parameter is \"at least as const as\" a matching\n * in-parameter. For example, skip_prefix() will return \"out\" that is a subset\n * of \"str\". So:\n *\n@@ -476,7 +476,7 @@ void set_die_is_recursing_routine(int (*routine)(void));\n * See the skip_prefix macro below for an example of use.\n */\n #define CONST_OUTPARAM(in, out) \\\n- ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n+\t((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))\n \n /*\n * If the string \"str\" begins with the string found in \"prefix\", return true.\n-- \n2.54.0.rc0.409.g4c76eb20e4\n\n"}]}