From 4ac4705afa3ab660e206c2b870bfae2ddb647ffa Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Thu, 5 Feb 2026 17:46:09 -0800 Subject: [PATCH 01/34] global: constify some pointers that are not written to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent glibc 2.43 release had the following change listed in its NEWS file: For ISO C23, the functions bsearch, memchr, strchr, strpbrk, strrchr, strstr, wcschr, wcspbrk, wcsrchr, wcsstr and wmemchr that return pointers into their input arrays now have definitions as macros that return a pointer to a const-qualified type when the input argument is a pointer to a const-qualified type. When compiling with GCC 15, which defaults to -std=gnu23, this causes many warnings like this: merge-ort.c: In function ‘apply_directory_rename_modifications’: merge-ort.c:2734:36: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 2734 | char *last_slash = strrchr(cur_path, '/'); | ^~~~~~~ This patch fixes the more obvious ones by making them const when we do not write to the returned pointer. Signed-off-by: Collin Funk Signed-off-by: Junio C Hamano --- add-patch.c | 2 +- apply.c | 2 +- builtin/commit.c | 2 +- builtin/receive-pack.c | 2 +- builtin/remote.c | 2 +- builtin/shortlog.c | 2 +- config.c | 2 +- convert.c | 3 ++- diff.c | 4 ++-- diffcore-rename.c | 2 +- fmt-merge-msg.c | 3 ++- fsck.c | 2 +- gpg-interface.c | 2 +- help.c | 2 +- http-push.c | 2 +- mailinfo.c | 2 +- mem-pool.c | 2 +- merge-ort.c | 2 +- object-name.c | 2 +- pack-revindex.c | 2 +- pkt-line.c | 6 +++--- reflog-walk.c | 3 ++- scalar.c | 2 +- strbuf.c | 2 +- string-list.c | 2 +- t/unit-tests/clar/clar/print.h | 2 +- transport.c | 2 +- wrapper.c | 2 +- 28 files changed, 34 insertions(+), 31 deletions(-) diff --git a/add-patch.c b/add-patch.c index 173a53241ebf07..70242617ef01e5 100644 --- a/add-patch.c +++ b/add-patch.c @@ -342,7 +342,7 @@ static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk) { struct hunk_header *header = &hunk->header; const char *line = s->plain.buf + hunk->start, *p = line; - char *eol = memchr(p, '\n', s->plain.len - hunk->start); + const char *eol = memchr(p, '\n', s->plain.len - hunk->start); if (!eol) eol = s->plain.buf + s->plain.len; diff --git a/apply.c b/apply.c index 3de4aa4d2eaac5..9de2eb953e51f3 100644 --- a/apply.c +++ b/apply.c @@ -4144,7 +4144,7 @@ static int preimage_oid_in_gitlink_patch(struct patch *p, struct object_id *oid) */ struct fragment *hunk = p->fragments; static const char heading[] = "-Subproject commit "; - char *preimage; + const char *preimage; if (/* does the patch have only one hunk? */ hunk && !hunk->next && diff --git a/builtin/commit.c b/builtin/commit.c index 8e901fe8db7942..03265465480e7c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -816,7 +816,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, logfile); hook_arg1 = "message"; } else if (use_message) { - char *buffer; + const char *buffer; buffer = strstr(use_message_buffer, "\n\n"); if (buffer) strbuf_addstr(&sb, skip_blank_lines(buffer + 2)); diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9c491746168a6f..e8b6f960faea14 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -393,7 +393,7 @@ struct command { static void proc_receive_ref_append(const char *prefix) { struct proc_receive_ref *ref_pattern; - char *p; + const char *p; int len; CALLOC_ARRAY(ref_pattern, 1); diff --git a/builtin/remote.c b/builtin/remote.c index 7ffc14ba15743a..ace390c671d61c 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -332,7 +332,7 @@ static int config_read_branches(const char *key, const char *value, info->remote_name = xstrdup(value); break; case MERGE: { - char *space = strchr(value, ' '); + const char *space = strchr(value, ' '); value = abbrev_branch(value); while (space) { char *merge; diff --git a/builtin/shortlog.c b/builtin/shortlog.c index b91acf45c8fe59..d80bf1a7d055fc 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -76,7 +76,7 @@ static void insert_one_record(struct shortlog *log, if (!eol) eol = oneline + strlen(oneline); if (starts_with(oneline, "[PATCH")) { - char *eob = strchr(oneline, ']'); + const char *eob = strchr(oneline, ']'); if (eob && (!eol || eob < eol)) oneline = eob + 1; } diff --git a/config.c b/config.c index 7f6d53b4737cd8..156f2a24fa0027 100644 --- a/config.c +++ b/config.c @@ -160,7 +160,7 @@ static int handle_path_include(const struct key_value_info *kvi, * based on the including config file. */ if (!is_absolute_path(path)) { - char *slash; + const char *slash; if (!kvi || kvi->origin_type != CONFIG_ORIGIN_FILE) { ret = error(_("relative config includes must come from files")); diff --git a/convert.c b/convert.c index c7d6a85c226db7..a34ec6ecdc057e 100644 --- a/convert.c +++ b/convert.c @@ -1122,7 +1122,8 @@ static int count_ident(const char *cp, unsigned long size) static int ident_to_git(const char *src, size_t len, struct strbuf *buf, int ident) { - char *dst, *dollar; + char *dst; + const char *dollar; if (!ident || (src && !count_ident(src, len))) return 0; diff --git a/diff.c b/diff.c index a68ddd2168ba1c..2d92665159d540 100644 --- a/diff.c +++ b/diff.c @@ -1961,7 +1961,7 @@ static int fn_out_diff_words_write_helper(struct diff_options *o, struct strbuf sb = STRBUF_INIT; while (count) { - char *p = memchr(buf, '\n', count); + const char *p = memchr(buf, '\n', count); if (print) strbuf_addstr(&sb, diff_line_prefix(o)); @@ -3049,7 +3049,7 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir, struct dirstat_file *f = dir->files; int namelen = strlen(f->name); unsigned long changes; - char *slash; + const char *slash; if (namelen < baselen) break; diff --git a/diffcore-rename.c b/diffcore-rename.c index 7723bc3334e084..d9476db35acbf7 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -379,7 +379,7 @@ struct dir_rename_info { static char *get_dirname(const char *filename) { - char *slash = strrchr(filename, '/'); + const char *slash = strrchr(filename, '/'); return slash ? xstrndup(filename, slash - filename) : xstrdup(""); } diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index c9085edc40e934..1626667c0dc5c6 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -246,7 +246,8 @@ static void add_branch_desc(struct strbuf *out, const char *name) static void record_person_from_buf(int which, struct string_list *people, const char *buffer) { - char *name_buf, *name, *name_end; + char *name_buf; + const char *name, *name_end; struct string_list_item *elem; const char *field; diff --git a/fsck.c b/fsck.c index 3afec0d0d3828c..0f02cf8f77bf1a 100644 --- a/fsck.c +++ b/fsck.c @@ -1026,7 +1026,7 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer, int *tagged_type) { int ret = 0; - char *eol; + const char *eol; struct strbuf sb = STRBUF_INIT; const char *buffer_end = buffer + size; const char *p; diff --git a/gpg-interface.c b/gpg-interface.c index 47222bf31b6ef2..377c0cf49f83ad 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -398,7 +398,7 @@ static void parse_ssh_output(struct signature_check *sigc) { const char *line, *principal, *search; char *to_free; - char *key = NULL; + const char *key = NULL; /* * ssh-keygen output should be: diff --git a/help.c b/help.c index 3c36d9c218f824..be334d764284a7 100644 --- a/help.c +++ b/help.c @@ -857,7 +857,7 @@ struct similar_ref_cb { static int append_similar_ref(const struct reference *ref, void *cb_data) { struct similar_ref_cb *cb = (struct similar_ref_cb *)(cb_data); - char *branch = strrchr(ref->name, '/') + 1; + const char *branch = strrchr(ref->name, '/') + 1; /* A remote branch of the same name is deemed similar */ if (starts_with(ref->name, "refs/remotes/") && diff --git a/http-push.c b/http-push.c index cc0f80934615ba..9ae6062198e14f 100644 --- a/http-push.c +++ b/http-push.c @@ -1768,7 +1768,7 @@ int cmd_main(int argc, const char **argv) usage(http_push_usage); } if (!repo->url) { - char *path = strstr(arg, "//"); + const char *path = strstr(arg, "//"); str_end_url_with_slash(arg, &repo->url); repo->path_len = strlen(repo->url); if (path) { diff --git a/mailinfo.c b/mailinfo.c index 99ac596e096e70..a2f06dbd96ff6f 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -1141,7 +1141,7 @@ static void output_header_lines(FILE *fout, const char *hdr, const struct strbuf { const char *sp = data->buf; while (1) { - char *ep = strchr(sp, '\n'); + const char *ep = strchr(sp, '\n'); int len; if (!ep) len = strlen(sp); diff --git a/mem-pool.c b/mem-pool.c index 62441dcc71968f..8bc77cb0e80a35 100644 --- a/mem-pool.c +++ b/mem-pool.c @@ -169,7 +169,7 @@ char *mem_pool_strdup(struct mem_pool *pool, const char *str) char *mem_pool_strndup(struct mem_pool *pool, const char *str, size_t len) { - char *p = memchr(str, '\0', len); + const char *p = memchr(str, '\0', len); size_t actual_len = (p ? p - str : len); char *ret = mem_pool_alloc(pool, actual_len+1); diff --git a/merge-ort.c b/merge-ort.c index e80e4f735a60f0..6f30471b49f1ea 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2731,7 +2731,7 @@ static void apply_directory_rename_modifications(struct merge_options *opt, while (1) { /* Find the parent directory of cur_path */ - char *last_slash = strrchr(cur_path, '/'); + const char *last_slash = strrchr(cur_path, '/'); if (last_slash) { parent_name = mem_pool_strndup(&opt->priv->pool, cur_path, diff --git a/object-name.c b/object-name.c index 8b862c124e05a9..e1b09d823c2f80 100644 --- a/object-name.c +++ b/object-name.c @@ -1756,7 +1756,7 @@ int repo_interpret_branch_name(struct repository *r, struct strbuf *buf, const struct interpret_branch_name_options *options) { - char *at; + const char *at; const char *start; int len; diff --git a/pack-revindex.c b/pack-revindex.c index 8598b941c8c419..56cd803a6798d5 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -544,7 +544,7 @@ static int midx_key_to_pack_pos(struct multi_pack_index *m, struct midx_pack_key *key, uint32_t *pos) { - uint32_t *found; + const uint32_t *found; if (key->pack >= m->num_packs + m->num_packs_in_base) BUG("MIDX pack lookup out of bounds (%"PRIu32" >= %"PRIu32")", diff --git a/pkt-line.c b/pkt-line.c index fc583feb26510d..3fc3e9ea7059be 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -384,10 +384,10 @@ int packet_length(const char lenbuf_hex[4], size_t size) hexval(lenbuf_hex[3]); } -static char *find_packfile_uri_path(const char *buffer) +static const char *find_packfile_uri_path(const char *buffer) { const char *URI_MARK = "://"; - char *path; + const char *path; int len; /* First char is sideband mark */ @@ -417,7 +417,7 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer, { int len; char linelen[4]; - char *uri_path_start; + const char *uri_path_start; if (get_packet_data(fd, src_buffer, src_len, linelen, 4, options) < 0) { *pktlen = -1; diff --git a/reflog-walk.c b/reflog-walk.c index 4f1ce047498116..4dbeaa93a7703f 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -157,7 +157,8 @@ int add_reflog_for_walk(struct reflog_walk_info *info, int recno = -1; struct string_list_item *item; struct complete_reflogs *reflogs; - char *branch, *at = strchr(name, '@'); + char *branch; + const char *at = strchr(name, '@'); struct commit_reflog *commit_reflog; enum selector_type selector = SELECTOR_NONE; diff --git a/scalar.c b/scalar.c index c9df9348ecba46..4efb6ac36d888e 100644 --- a/scalar.c +++ b/scalar.c @@ -393,7 +393,7 @@ static int delete_enlistment(struct strbuf *enlistment) { struct strbuf parent = STRBUF_INIT; size_t offset; - char *path_sep; + const char *path_sep; if (unregister_dir()) return error(_("failed to unregister repository")); diff --git a/strbuf.c b/strbuf.c index 59678bf5b03e0b..3939863cf31ffd 100644 --- a/strbuf.c +++ b/strbuf.c @@ -1119,6 +1119,6 @@ void strbuf_stripspace(struct strbuf *sb, const char *comment_prefix) void strbuf_strip_file_from_path(struct strbuf *sb) { - char *path_sep = find_last_dir_sep(sb->buf); + const char *path_sep = find_last_dir_sep(sb->buf); strbuf_setlen(sb, path_sep ? path_sep - sb->buf + 1 : 0); } diff --git a/string-list.c b/string-list.c index 08dc00984ccbd6..7c34a425da5d0a 100644 --- a/string-list.c +++ b/string-list.c @@ -327,7 +327,7 @@ static int split_string(struct string_list *list, const char *string, const char BUG("string_list_split() called without strdup_strings"); for (;;) { - char *end; + const char *end; if (flags & STRING_LIST_SPLIT_TRIM) { /* ltrim */ diff --git a/t/unit-tests/clar/clar/print.h b/t/unit-tests/clar/clar/print.h index 6a2321b399d192..59b7dc14a104a8 100644 --- a/t/unit-tests/clar/clar/print.h +++ b/t/unit-tests/clar/clar/print.h @@ -127,7 +127,7 @@ static void clar_print_tap_error(int num, const struct clar_report *report, cons static void print_escaped(const char *str) { - char *c; + const char *c; while ((c = strchr(str, '\'')) != NULL) { printf("%.*s", (int)(c - str), str); diff --git a/transport.c b/transport.c index c7f06a7382e605..845fd441bec7fa 100644 --- a/transport.c +++ b/transport.c @@ -1657,7 +1657,7 @@ int transport_disconnect(struct transport *transport) */ char *transport_anonymize_url(const char *url) { - char *scheme_prefix, *anon_part; + const char *scheme_prefix, *anon_part; size_t anon_len, prefix_len = 0; anon_part = strchr(url, '@'); diff --git a/wrapper.c b/wrapper.c index b794fb20e71879..16f5a63fbb614a 100644 --- a/wrapper.c +++ b/wrapper.c @@ -115,7 +115,7 @@ void *xmemdupz(const void *data, size_t len) char *xstrndup(const char *str, size_t len) { - char *p = memchr(str, '\0', len); + const char *p = memchr(str, '\0', len); return xmemdupz(str, p ? p - str : len); } From fc9fd8065c6049243f50e90f00a847054ca15e28 Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Thu, 5 Feb 2026 17:46:10 -0800 Subject: [PATCH 02/34] gpg-interface: remove an unnecessary NULL initialization We assign this variable unconditionally, so we do not need to initialize it to NULL where it is defined. Signed-off-by: Collin Funk Signed-off-by: Junio C Hamano --- gpg-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gpg-interface.c b/gpg-interface.c index 377c0cf49f83ad..87fb6605fba174 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -398,7 +398,7 @@ static void parse_ssh_output(struct signature_check *sigc) { const char *line, *principal, *search; char *to_free; - const char *key = NULL; + const char *key; /* * ssh-keygen output should be: From 5ec4c22e49839c2eeb319070cde14ddbea3d1adb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:00:52 -0500 Subject: [PATCH 03/34] ref-filter: factor out refname component counting The "lstrip" and "rstrip" options to the %(refname) placeholder both accept a negative length, which asks us to keep that many path components (rather than stripping that many). The code to count components and convert the negative value to a positive was copied from lstrip to rstrip in 1a34728e6b (ref-filter: add an 'rstrip=' option to atoms which deal with refnames, 2017-01-10). Let's factor it out into a separate function. This reduces duplication and also makes the lstrip/rstrip functions much easier to follow, since the bulk of their code is now the actual stripping. Note that the computed "remaining" value is currently stored as a "long", so in theory that's what our function should return. But this is purely historical. When the variable was added in 0571979bd6 (tag: do not show ambiguous tag names as "tags/foo", 2016-01-25), we parsed the value from strtol(), and thus used a long. But these days we take "len" as an int, and also use an int to count up components. So let's just consistently use int here. This value could only overflow in a pathological case (e.g., 4GB worth of "a/a/...") and even then will not result in out-of-bounds memory access (we keep stripping until we run out of string to parse). The minimal Myers diff here is a little hard to read; with --patience the code movement is shown much more clearly. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ref-filter.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index c318f9ca0ec8dd..ff14ac53de2ed2 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2173,12 +2173,8 @@ static inline char *copy_advance(char *dst, const char *src) return dst; } -static const char *lstrip_ref_components(const char *refname, int len) +static int normalize_component_count(const char *refname, int len) { - long remaining = len; - const char *start = xstrdup(refname); - const char *to_free = start; - if (len < 0) { int i; const char *p = refname; @@ -2192,8 +2188,16 @@ static const char *lstrip_ref_components(const char *refname, int len) * because we count the number of '/', but the number * of components is one more than the no of '/'). */ - remaining = i + len + 1; + len = i + len + 1; } + return len; +} + +static const char *lstrip_ref_components(const char *refname, int len) +{ + int remaining = normalize_component_count(refname, len); + const char *start = xstrdup(refname); + const char *to_free = start; while (remaining > 0) { switch (*start++) { @@ -2213,26 +2217,10 @@ static const char *lstrip_ref_components(const char *refname, int len) static const char *rstrip_ref_components(const char *refname, int len) { - long remaining = len; + int remaining = normalize_component_count(refname, len); const char *start = xstrdup(refname); const char *to_free = start; - if (len < 0) { - int i; - const char *p = refname; - - /* Find total no of '/' separated path-components */ - for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) - ; - /* - * The number of components we need to strip is now - * the total minus the components to be left (Plus one - * because we count the number of '/', but the number - * of components is one more than the no of '/'). - */ - remaining = i + len + 1; - } - while (remaining-- > 0) { char *p = strrchr(start, '/'); if (!p) { From 87cb6dc9b0832683a31d0c3126ecad4ad444489c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:02:23 -0500 Subject: [PATCH 04/34] ref-filter: simplify lstrip_ref_components() memory handling We're walking forward in the string, skipping path components from left-to-right. So when we've stripped as much as we want, the pointer we have is a complete NUL-terminated string and we can just return it (after duplicating it, of course). So there is no need for a temporary allocated string. But we do make an extra temporary copy due to f0062d3b74 (ref-filter: free item->value and item->value->s, 2018-10-18). This is probably from cargo-culting the technique used in rstrip_ref_components(), which _does_ need a separate string (since it is stripping from the end and ties off the temporary string with a NUL). Let's drop the extra allocation. This is slightly more efficient, but more importantly makes the code much simpler. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ref-filter.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index ff14ac53de2ed2..f5f0cb4ad69632 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2196,13 +2196,10 @@ static int normalize_component_count(const char *refname, int len) static const char *lstrip_ref_components(const char *refname, int len) { int remaining = normalize_component_count(refname, len); - const char *start = xstrdup(refname); - const char *to_free = start; while (remaining > 0) { - switch (*start++) { + switch (*refname++) { case '\0': - free((char *)to_free); return xstrdup(""); case '/': remaining--; @@ -2210,9 +2207,7 @@ static const char *lstrip_ref_components(const char *refname, int len) } } - start = xstrdup(start); - free((char *)to_free); - return start; + return xstrdup(refname); } static const char *rstrip_ref_components(const char *refname, int len) From 2ec30e71f44233b9afceda0f2992029674187674 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:05:34 -0500 Subject: [PATCH 05/34] ref-filter: simplify rstrip_ref_components() memory handling We're stripping path components from the end of a string, which we do by assigning a NUL as we parse each component, shortening the string. This requires an extra temporary buffer to avoid munging our input string. But the way that we allocate the buffer is unusual. We have an extra "to_free" variable. Usually this is used when the access variable is conceptually const, like: const char *foo; char *to_free = NULL; if (...) foo = to_free = xstrdup(...); else foo = some_const_string; ... free(to_free); But that's not what's happening here. Our "start" variable always points to the allocated buffer, and to_free is redundant. Worse, it is marked as const itself, requiring a cast when we free it. Let's drop to_free entirely, and mark "start" as non-const, making the memory handling more clear. As a bonus, this also silences a warning from glibc-2.43 that our call to strrchr() implicitly strips away the const-ness of "start". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ref-filter.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index f5f0cb4ad69632..9589418c256840 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2213,13 +2213,12 @@ static const char *lstrip_ref_components(const char *refname, int len) static const char *rstrip_ref_components(const char *refname, int len) { int remaining = normalize_component_count(refname, len); - const char *start = xstrdup(refname); - const char *to_free = start; + char *start = xstrdup(refname); while (remaining-- > 0) { char *p = strrchr(start, '/'); if (!p) { - free((char *)to_free); + free(start); return xstrdup(""); } else p[0] = '\0'; From fe732a8b9f1837189eb3533a262548a652c11e61 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:07:44 -0500 Subject: [PATCH 06/34] ref-filter: avoid strrchr() in rstrip_ref_components() To strip path components from our refname string, we repeatedly call strrchr() to find the trailing slash, shortening the string each time by assigning NUL over it. This has two downsides: 1. Calling strrchr() in a loop is quadratic, since each call has to call strlen() under the hood to find the end of the string (even though we know exactly where it is from the last loop iteration). 2. We need a temporary buffer, since we're munging the string with NUL as we shorten it (which we must do, because strrchr() has no other way of knowing what we consider the end of the string). Using memrchr() would let us fix both of these, but it isn't portable. So instead, let's just open-code the string traversal from back to front as we loop. I doubt that the quadratic nature is a serious concern. You can see it in practice with something like: git init git commit --allow-empty -m foo echo "$(git rev-parse HEAD) refs/heads$(perl -e 'print "/a" x 500_000')" >.git/packed-refs time git for-each-ref --format='%(refname:rstrip=-1)' That takes ~5.5s to run on my machine before this patch, and ~11ms after. But I don't think there's a reasonable way for somebody to infect you with such a garbage ref, as the wire protocol is limited to 64k pkt-lines. The difference is measurable for me for a 32k-component ref (about 19ms vs 7ms), so perhaps you could create some chaos by pushing a lot of them. But we also run into filesystem limits (if the loose backend is in use), and in practice it seems like there are probably simpler and more effective ways to waste CPU. Likewise the extra allocation probably isn't really measurable. In fact, since our goal is to return an allocated string, we end up having to make the same allocation anyway (though it is sized to the result, rather than the input). My main goal was simplicity in avoiding the need to handle cleaning it up in the early return path. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ref-filter.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 9589418c256840..01512d50bd6d38 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2213,17 +2213,15 @@ static const char *lstrip_ref_components(const char *refname, int len) static const char *rstrip_ref_components(const char *refname, int len) { int remaining = normalize_component_count(refname, len); - char *start = xstrdup(refname); + const char *end = refname + strlen(refname); - while (remaining-- > 0) { - char *p = strrchr(start, '/'); - if (!p) { - free(start); + while (remaining > 0) { + if (end == refname) return xstrdup(""); - } else - p[0] = '\0'; + if (*--end == '/') + remaining--; } - return start; + return xmemdupz(refname, end - refname); } static const char *show_ref(struct refname_atom *atom, const char *refname) From 8b0061b5c5008375ef0986b7aafedbd7d79da0f6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 20 Feb 2026 01:00:03 -0500 Subject: [PATCH 07/34] ref-filter: clarify lstrip/rstrip component counting When a strip option to the %(refname) placeholder is asked to leave N path components, we first count up the path components to know how many to remove. That happens with a loop like this: /* Find total no of '/' separated path-components */ for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) ; which is a little hard to understand for two reasons. First, the dereference in "*p++" is seemingly useless, since nobody looks at the result. And static analyzers like Coverity will complain about that. But removing the "*" will cause gcc to complain with -Wint-conversion, since the two sides of the ternary do not match (one is a pointer and the other an int). Second, it is not clear what the meaning of "p" is at each iteration of the loop, as its position with respect to our walk over the string depends on how many slashes we've seen. The answer is that by itself, it doesn't really mean anything: "p + i" represents the current state of our walk, with "i" counting up slashes, and "p" by itself essentially meaningless. None of this behaves incorrectly, but ultimately the loop is just counting the slashes in the refname. We can do that much more simply with a for-loop iterating over the string and a separate slash counter. We can also drop the comment, which is somewhat misleading. We are counting slashes, not components (and a comment later in the function makes it clear that we must add one to compensate). In the new code it is obvious that we are counting slashes here. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ref-filter.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 01512d50bd6d38..db83d3b32c3e20 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2176,19 +2176,20 @@ static inline char *copy_advance(char *dst, const char *src) static int normalize_component_count(const char *refname, int len) { if (len < 0) { - int i; - const char *p = refname; + int slashes = 0; + + for (const char *p = refname; *p; p++) { + if (*p == '/') + slashes++; + } - /* Find total no of '/' separated path-components */ - for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) - ; /* * The number of components we need to strip is now * the total minus the components to be left (Plus one * because we count the number of '/', but the number * of components is one more than the no of '/'). */ - len = i + len + 1; + len = slashes + len + 1; } return len; } From 1ac1d4e761c5f394526873b364ba23cf5b9b0da5 Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Sun, 8 Mar 2026 19:55:11 -0700 Subject: [PATCH 08/34] bloom: remove a misleading const qualifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with glibc-2.43 there is the following warning: bloom.c: In function ‘get_or_compute_bloom_filter’: bloom.c:515:52: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 515 | char *last_slash = strrchr(path, '/'); | ^~~~~~~ In this case, we always write through "path" through the "last_slash" pointer. Therefore, the const qualifier on "path" is misleading and we can just remove it. Signed-off-by: Collin Funk Signed-off-by: Junio C Hamano --- bloom.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bloom.c b/bloom.c index 2d7b951e5bf245..d604e8f07ab8d8 100644 --- a/bloom.c +++ b/bloom.c @@ -501,7 +501,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r, struct hashmap_iter iter; for (i = 0; i < diff_queued_diff.nr; i++) { - const char *path = diff_queued_diff.queue[i]->two->path; + char *path = diff_queued_diff.queue[i]->two->path; /* * Add each leading directory of the changed file, i.e. for @@ -523,7 +523,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r, free(e); if (!last_slash) - last_slash = (char*)path; + last_slash = path; *last_slash = '\0'; } while (*path); From 02cbae61df7b3493e7f76f8385fc9257de60755f Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Sun, 8 Mar 2026 20:23:06 -0700 Subject: [PATCH 09/34] dir: avoid -Wdiscarded-qualifiers in remove_path() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with glibc-2.43 there is the following warning: dir.c:3526:15: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 3526 | slash = strrchr(name, '/'); | ^ In this case we use a non-const pointer to get the last slash of the unwritable file name, and then use it again to write in the strdup'd file name. We can avoid this warning and make the code a bit more clear by using a separate variable to access the original argument and its strdup'd copy. Signed-off-by: Collin Funk Signed-off-by: Junio C Hamano --- dir.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dir.c b/dir.c index b00821f294fea2..046a8bbf325ac5 100644 --- a/dir.c +++ b/dir.c @@ -3516,15 +3516,15 @@ int get_sparse_checkout_patterns(struct pattern_list *pl) int remove_path(const char *name) { - char *slash; + const char *last; if (unlink(name) && !is_missing_file_error(errno)) return -1; - slash = strrchr(name, '/'); - if (slash) { + last = strrchr(name, '/'); + if (last) { char *dirs = xstrdup(name); - slash = dirs + (slash - name); + char *slash = dirs + (last - name); do { *slash = '\0'; if (startup_info->original_cwd && From 96ef2fa9d0696813f57c57986d622659fab79303 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:49 -0400 Subject: [PATCH 10/34] convert: add const to fix strchr() warnings C23 versions of libc (like recent glibc) may provide generic versions of strchr() that match constness between the input and return value. The idea being that the compiler can detect when it implicitly converts a const pointer into a non-const one (which then emits a warning). There are a few cases here where the result pointer does not need to be non-const at all, and we should mark it as such. That silences the warning (and avoids any potential problems with trying to write via those pointers). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 2b9665c4e82a1a62d93a64e9892574d6e03ec019) Signed-off-by: Johannes Schindelin --- convert.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/convert.c b/convert.c index 7bf10d3178d752..0a29a7b112eb66 100644 --- a/convert.c +++ b/convert.c @@ -1181,7 +1181,8 @@ static int ident_to_worktree(const char *src, size_t len, struct strbuf *buf, int ident) { struct object_id oid; - char *to_free = NULL, *dollar, *spc; + char *to_free = NULL; + const char *dollar, *spc; int cnt; if (!ident) From a090ae3d19d1d64ba715a439b646b489190be494 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:51 -0400 Subject: [PATCH 11/34] http: add const to fix strchr() warnings The "path" field of a "struct repo" (a custom http-push struct, not to be confused with "struct repository) is a pointer into a const argv string, and is never written to. The compiler does not traditionally complain about assigning from a const pointer because it happens via strchr(). But with some C23 libc versions (notably recent glibc), it has started to do so. Let's mark the field as const to silence the warnings. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 2fb6a18782ff8f2d97b44c8812f9027f3812f970) Signed-off-by: Johannes Schindelin --- http-push.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http-push.c b/http-push.c index cc0f80934615ba..c3b20631ecb319 100644 --- a/http-push.c +++ b/http-push.c @@ -99,7 +99,7 @@ static struct object_list *objects; struct repo { char *url; - char *path; + const char *path; int path_len; int has_info_refs; int can_update_info_refs; From c0b71532bddf6b2631a0e35e4b348c218dba72b2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:56 -0400 Subject: [PATCH 12/34] transport-helper: drop const to fix strchr() warnings We implicitly drop the const from our "key" variable when we do: char *p = strchr(key, ' '); which causes compilation with some C23 versions of libc (notably recent glibc) to complain. We need "p" to remain writable, since we assign NUL over the space we found. We can solve this by also making "key" writable. This works because it comes from a strbuf, which is itself a writable string. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit eedc7ecc66aefa085aae9bf51b56aa11eeb23950) Signed-off-by: Johannes Schindelin --- transport-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/transport-helper.c b/transport-helper.c index af0089ce9a8246..9e1793138c4bbf 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -807,7 +807,8 @@ static int push_update_ref_status(struct strbuf *buf, if (starts_with(buf->buf, "option ")) { struct object_id old_oid, new_oid; - const char *key, *val; + char *key; + const char *val; char *p; if (!state->hint || !(state->report || state->new_report)) From cb39177bafaf09ba3a119382cbf4033ab5290efd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:58 -0400 Subject: [PATCH 13/34] pager: explicitly cast away strchr() constness When we do: char *cp = strchr(argv[i], '='); it implicitly removes the constness from argv[i]. We need "cp" to remain writable (since we overwrite it with a NUL). In theory we should be able to drop the const from argv[i], because it is a sub-pointer into our duplicated pager_env variable. But we get it from split_cmdline(), which uses the traditional "const char **" type for argv. This is overly limiting, but changing it would be awkward for all the other callers of split_cmdline(). Let's do an explicit cast with a note about why it is OK. This is enough to silence compiler warnings about the implicit const problems. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 031d29d6fbf12284d391c23f04d15970c3bac11c) Signed-off-by: Johannes Schindelin --- pager.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pager.c b/pager.c index 5531fff50eb73f..35b210e0484f90 100644 --- a/pager.c +++ b/pager.c @@ -108,10 +108,11 @@ const char *git_pager(struct repository *r, int stdout_is_tty) static void setup_pager_env(struct strvec *env) { - const char **argv; + char **argv; int i; char *pager_env = xstrdup(PAGER_ENV); - int n = split_cmdline(pager_env, &argv); + /* split_cmdline splits in place, so we know the result is writable */ + int n = split_cmdline(pager_env, (const char ***)&argv); if (n < 0) die("malformed build-time PAGER_ENV: %s", From d059ec98df3dca06572fd0bfe0003e077f43b5f0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:01 -0400 Subject: [PATCH 14/34] run-command: explicitly cast away constness when assigning to void We do this: char *equals = strchr(*e, '='); which implicitly removes the constness from "*e" and cause the compiler to complain. We never write to "equals", but later assign it to a string_list util field, which is defined as non-const "void *". We have to cast somewhere, but doing so at the assignment to util is the least-bad place, since that is the source of the confusion. Sadly we are still open to accidentally writing to the string via the util pointer, but that is the cost of using void pointers, which lose all type information. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 21c57efc77ccdef2d6186874024593ded59ecc65) Signed-off-by: Johannes Schindelin --- run-command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/run-command.c b/run-command.c index 497883a7defd62..a6958e8885d046 100644 --- a/run-command.c +++ b/run-command.c @@ -606,11 +606,11 @@ static void trace_add_env(struct strbuf *dst, const char *const *deltaenv) /* Last one wins, see run-command.c:prep_childenv() for context */ for (e = deltaenv; e && *e; e++) { struct strbuf key = STRBUF_INIT; - char *equals = strchr(*e, '='); + const char *equals = strchr(*e, '='); if (equals) { strbuf_add(&key, *e, equals - *e); - string_list_insert(&envs, key.buf)->util = equals + 1; + string_list_insert(&envs, key.buf)->util = (void *)(equals + 1); } else { string_list_insert(&envs, *e)->util = NULL; } From b894876b6acc098c82b83683c91efdb3733fa30d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:03 -0400 Subject: [PATCH 15/34] find_last_dir_sep(): convert inline function to macro The find_last_dir_sep() function is implemented as an inline function which takes in a "const char *" and returns a "char *" via strrchr(). That means that just like strrchr(), it quietly removes the const from our pointer, which could lead to accidentally writing to the resulting string. But C23 versions of libc (including recent glibc) annotate strrchr() such that the compiler can detect when const is implicitly lost, and it now complains about the call in this inline function. We can't just switch the return type of the function to "const char *", though. Some callers really do want a non-const string to be returned (and are OK because they are feeding a non-const string into the function). The most general solution is for us to annotate find_last_dir_sep() in the same way that is done for strrchr(). But doing so relies on using C23 generics, which we do not otherwise require. Since this inline function is wrapping a single call to strrchr(), we can take a shortcut. If we implement it as a macro, then the original type information is still available to strrchr(), and it does the check for us. Note that this is just one implementation of find_last_dir_sep(). There is an alternate implementation in compat/win32/path-utils.h. It doesn't suffer from the same warning, as it does not use strrchr() and just casts away const explicitly. That's not ideal, and eventually we may want to conditionally teach it the same C23 generic trick that strrchr() uses. But it has been that way forever, and our goal here is just quieting new warnings, not improving const-checking. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit bc4fd55984ce8c0fb99c8672ef2702acbbd98521) Signed-off-by: Johannes Schindelin --- git-compat-util.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 44cd216c58e5da..c15017e52c247a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -348,11 +348,7 @@ static inline int is_path_owned_by_current_uid(const char *path, #endif #ifndef find_last_dir_sep -static inline char *git_find_last_dir_sep(const char *path) -{ - return strrchr(path, '/'); -} -#define find_last_dir_sep git_find_last_dir_sep +#define find_last_dir_sep(path) strrchr((path), '/') #endif #ifndef has_dir_sep From 153a3501ed45d656225bf1a613928455f308919e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:05 -0400 Subject: [PATCH 16/34] pseudo-merge: fix disk reads from find_pseudo_merge() The goal of this commit was to fix a const warning when compiling with new versions of glibc, but ended up untangling a much deeper problem. The find_pseudo_merge() function does a bsearch() on the "commits" pointer of a pseudo_merge_map. This pointer ultimately comes from memory mapped from the on-disk bitmap file, and is thus not writable. The "commits" array is correctly marked const, but the result from bsearch() is returned directly as a non-const pseudo_merge_commit struct. Since new versions of glibc annotate bsearch() in a way that detects the implicit loss of const, the compiler now warns. My first instinct was that we should be returning a const struct. That requires apply_pseudo_merges_for_commit() to mark its local pointer as const. But that doesn't work! If the offset field has the high-bit set, we look it up in the extended table via nth_pseudo_merge_ext(). And that function then feeds our const struct to read_pseudo_merge_commit_at(), which writes into it by byte-swapping from the on-disk mmap. But I think this points to a larger problem with find_pseudo_merge(). It is not just that the return value is missing const, but it is missing that byte-swapping! And we know that byte-swapping is needed here, because the comparator we use for bsearch() also calls our read_pseudo_merge_commit_at() helper. So I think the interface is all wrong here. We should not be returning a pointer to a struct which was cast from on-disk data. We should be filling in a caller-provided struct using the bytes we found, byte-swapping the values. That of course raises the dual question: how did this ever work, and does it work now? The answer to the first part is: this code does not seem to be triggered in the test suite at all. If we insert a BUG("foo") call into apply_pseudo_merges_for_commit(), it never triggers. So I think there is something wrong or missing from the test setup, and this bears further investigation. Sadly the answer to the second part ("does it work now") is still "no idea". I _think_ this takes us in a positive direction, but my goal here is mainly to quiet the compiler warning. Further bug-hunting on this experimental feature can be done separately. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 25e5ceb9ee21d8806c9a3651e4f10241155f6e14) Signed-off-by: Johannes Schindelin --- pseudo-merge.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/pseudo-merge.c b/pseudo-merge.c index a2d5bd85f95ebf..ff18b6c364245e 100644 --- a/pseudo-merge.c +++ b/pseudo-merge.c @@ -638,14 +638,21 @@ static int pseudo_merge_commit_cmp(const void *va, const void *vb) return 0; } -static struct pseudo_merge_commit *find_pseudo_merge(const struct pseudo_merge_map *pm, - uint32_t pos) +static int find_pseudo_merge(const struct pseudo_merge_map *pm, uint32_t pos, + struct pseudo_merge_commit *out) { + const unsigned char *at; + if (!pm->commits_nr) - return NULL; + return 0; - return bsearch(&pos, pm->commits, pm->commits_nr, - PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp); + at = bsearch(&pos, pm->commits, pm->commits_nr, + PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp); + if (!at) + return 0; + + read_pseudo_merge_commit_at(out, at); + return 1; } int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, @@ -653,16 +660,15 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, struct commit *commit, uint32_t commit_pos) { struct pseudo_merge *merge; - struct pseudo_merge_commit *merge_commit; + struct pseudo_merge_commit merge_commit; int ret = 0; - merge_commit = find_pseudo_merge(pm, commit_pos); - if (!merge_commit) + if (!find_pseudo_merge(pm, commit_pos, &merge_commit)) return 0; - if (merge_commit->pseudo_merge_ofs & ((uint64_t)1<<63)) { + if (merge_commit.pseudo_merge_ofs & ((uint64_t)1<<63)) { struct pseudo_merge_commit_ext ext = { 0 }; - off_t ofs = merge_commit->pseudo_merge_ofs & ~((uint64_t)1<<63); + off_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63); uint32_t i; if (pseudo_merge_ext_at(pm, &ext, ofs) < -1) { @@ -673,11 +679,11 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, } for (i = 0; i < ext.nr; i++) { - if (nth_pseudo_merge_ext(pm, &ext, merge_commit, i) < 0) + if (nth_pseudo_merge_ext(pm, &ext, &merge_commit, i) < 0) return ret; merge = pseudo_merge_at(pm, &commit->object.oid, - merge_commit->pseudo_merge_ofs); + merge_commit.pseudo_merge_ofs); if (!merge) return ret; @@ -687,7 +693,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, } } else { merge = pseudo_merge_at(pm, &commit->object.oid, - merge_commit->pseudo_merge_ofs); + merge_commit.pseudo_merge_ofs); if (!merge) return ret; From 232793ef4bbdc38eaec65d34944b882b9751c2c6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:07 -0400 Subject: [PATCH 17/34] skip_prefix(): check const match between in and out params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skip_prefix() function takes in a "const char *" string, and returns via a "const char **" out-parameter that points somewhere in that string. This is fine if you are operating on a const string, like: const char *in = ...; const char *out; if (skip_prefix(in, "foo", &out)) ...look at out... It is also OK if "in" is not const but "out" is, as we add an implicit const when we pass "in" to the function. But there's another case where this is limiting. If we want both fields to be non-const, like: char *in = ...; char *out; if (skip_prefix(in, "foo", &out)) *out = '\0'; it doesn't work. The compiler will complain about the type mismatch in passing "&out" to a parameter which expects "const char **". So to make this work, we have to do an explicit cast. But such a cast is ugly, and also means that we run afoul of making this mistake: const char *in = ...; char *out; if (skip_prefix(in, "foo", (const char **)&out)) *out = '\0'; which causes us to write to the memory pointed by "in", which was const. We can imagine these four cases as: (1) const in, const out (2) non-const in, const out (3) non-const in, non-const out (4) const in, non-const out Cases (1) and (2) work now. We would like case (3) to work but it doesn't. But we would like to catch case (4) as a compile error. So ideally the rule is "the out-parameter must be at least as const as the in-parameter". We can do this with some macro trickery. We wrap skip_prefix() in a macro so that it has access to the real types of in/out. And then we pass those parameters through another macro which: 1. Fails if the "at least as const" rule is not filled. 2. Casts to match the signature of the real skip_prefix(). There are a lot of ways to implement the "fails" part. You can use __builtin_types_compatible_p() to check, and then either our BUILD_ASSERT macros or _Static_assert to fail. But that requires some conditional compilation based on compiler feature. That's probably OK (the fallback would be to just cast without catching case 4). But we can do better. The macro I have here uses a ternary with a dead branch that tries to assign "in" to "out", which should work everywhere and lets the compiler catch the problem in the usual way. With an input like this: int foo(const char *x, const char **y); #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out))) void ok_const(const char *x, const char **y) { foo(x, y); } void ok_nonconst(char *x, char **y) { foo(x, y); } void ok_add_const(char *x, const char **y) { foo(x, y); } void bad_drop_const(const char *x, char **y) { foo(x, y); } gcc reports: foo.c: In function ‘bad_drop_const’: foo.c:2:35: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] 2 | ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) | ^ foo.c:4:31: note: in expansion of macro ‘CONST_OUTPARAM’ 4 | #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out))) | ^~~~~~~~~~~~~~ foo.c:23:9: note: in expansion of macro ‘foo’ 23 | foo(x, y); | ^~~ It's a bit verbose, but I think makes it reasonably clear what's going on. Using BUILD_ASSERT_OR_ZERO() ends up much worse. Using _Static_assert you can be a bit more informative, but that's not something we use at all yet in our code-base (it's an old gnu-ism later standardized in C11). Our generic macro only works for "const char **", which is something we could improve by using typeof(in). But that introduces more portability questions, and also some weird corner cases (e.g., around implicit void conversion). This patch just introduces the concept. We'll make use of it in future patches. Note that we rename skip_prefix() to skip_prefix_impl() here, to avoid expanding the macro when defining the function. That's not strictly necessary since we could just define the macro after defining the inline function. But that would not be the case for a non-inline function (and we will apply this technique to them later, and should be consistent). It also gives us more freedom about where to define the macro. I did so right above the definition here, which I think keeps the relevant bits together. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit cefb8b7b47415aff6ad3ecdc6f3c546fa7abfd16) Signed-off-by: Johannes Schindelin --- git-compat-util.h | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index c15017e52c247a..2d6ae490bdd972 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -494,6 +494,21 @@ void set_warn_routine(report_fn routine); report_fn get_warn_routine(void); void set_die_is_recursing_routine(int (*routine)(void)); +/* + * Check that an out-parameter that is "at least as const as" a matching + * in-parameter. For example, skip_prefix() will return "out" that is a subset + * of "str". So: + * + * const str, const out: ok + * non-const str, const out: ok + * non-const str, non-const out: ok + * const str, non-const out: compile error + * + * See the skip_prefix macro below for an example of use. + */ +#define CONST_OUTPARAM(in, out) \ + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) + /* * If the string "str" begins with the string found in "prefix", return true. * The "out" parameter is set to "str + strlen(prefix)" (i.e., to the point in @@ -510,8 +525,10 @@ void set_die_is_recursing_routine(int (*routine)(void)); * [skip prefix if present, otherwise use whole string] * skip_prefix(name, "refs/heads/", &name); */ -static inline bool skip_prefix(const char *str, const char *prefix, - const char **out) +#define skip_prefix(str, prefix, out) \ + skip_prefix_impl((str), (prefix), CONST_OUTPARAM((str), (out))) +static inline bool skip_prefix_impl(const char *str, const char *prefix, + const char **out) { do { if (!*prefix) { From bb09391d3fb500d122104da2ab57b24438b2ff0b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:10 -0400 Subject: [PATCH 18/34] pkt-line: make packet_reader.line non-const The "line" member of a packet_reader struct is marked as const. This kind of makes sense, because it's not its own allocated buffer that should be freed, and we often use const to indicate that. But it is always writable, because it points into the non-const "buffer" member. And we rely on this writability in places like send-pack and receive-pack, where we parse incoming packet contents by writing NULs over delimiters. This has traditionally worked because we implicitly cast away the constness with strchr() like: const char *head; char *p; head = reader->line; p = strchr(head, ' '); Since C23 libc provides a generic strchr() to detect this implicit const removal, this now generate a compiler warning on some platforms (like recent glibc). We can fix it by marking "line" as non-const, as well as a few intermediate variables (like "head" in the above example). Note that by itself, switching to a non-const variable would cause problems with this line in send-pack.c: if (!skip_prefix(reader->line, "unpack ", &reader->line)) But due to our skip_prefix() magic introduced in the previous commit, this compiles fine (both the in and out-parameters are non-const, so we know it is safe). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit d3cd819e8bb21189b7bf3b2718898b610b85b119) Signed-off-by: Johannes Schindelin --- builtin/receive-pack.c | 7 ++++--- pkt-line.h | 2 +- send-pack.c | 7 ++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9c491746168a6f..f97698473ab9f8 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -989,8 +989,8 @@ static int read_proc_receive_report(struct packet_reader *reader, for (;;) { struct object_id old_oid, new_oid; - const char *head; - const char *refname; + char *head; + char *refname; char *p; enum packet_read_status status; @@ -1014,7 +1014,8 @@ static int read_proc_receive_report(struct packet_reader *reader, } *p++ = '\0'; if (!strcmp(head, "option")) { - const char *key, *val; + char *key; + const char *val; if (!hint || !(report || new_report)) { if (!once++) diff --git a/pkt-line.h b/pkt-line.h index 10fd9a812e1935..5c43e253993983 100644 --- a/pkt-line.h +++ b/pkt-line.h @@ -185,7 +185,7 @@ struct packet_reader { int pktlen; /* the last line read */ - const char *line; + char *line; /* indicates if a line has been peeked */ int line_peeked; diff --git a/send-pack.c b/send-pack.c index a85d5695d75947..2bd7a8df3831df 100644 --- a/send-pack.c +++ b/send-pack.c @@ -178,8 +178,8 @@ static int receive_status(struct repository *r, ret = receive_unpack_status(reader); while (1) { struct object_id old_oid, new_oid; - const char *head; - const char *refname; + char *head; + char *refname; char *p; if (packet_reader_read(reader) != PACKET_READ_NORMAL) break; @@ -193,7 +193,8 @@ static int receive_status(struct repository *r, *p++ = '\0'; if (!strcmp(head, "option")) { - const char *key, *val; + char *key; + const char *val; if (!hint || !(report || new_report)) { if (!once++) From 8d79e1611b5918b6f6b2b48df62c4829f5e94d7b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:12 -0400 Subject: [PATCH 19/34] range-diff: drop const to fix strstr() warnings This is another case where we implicitly drop the "const" from a pointer by feeding it to strstr() and assigning the result to a non-const pointer. This is OK in practice, since the const pointer originally comes from a writable source (a strbuf), but C23 libc implementations have started to complain about it. We do write to the output pointer, so it needs to remain non-const. We can just switch the input pointer to also be non-const in this case. By itself that would run into problems with calls to skip_prefix(), but since that function has now been taught to match in/out constness automatically, it just works without us doing anything further. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit c39512600f85aa88f368dc6bd13baeb183ae52ad) Signed-off-by: Johannes Schindelin --- range-diff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/range-diff.c b/range-diff.c index 57edff40a85f24..155bf0f2531d7c 100644 --- a/range-diff.c +++ b/range-diff.c @@ -88,7 +88,7 @@ static int read_patches(const char *range, struct string_list *list, line = contents.buf; size = contents.len; for (; size > 0; size -= len, line += len) { - const char *p; + char *p; char *eol; eol = memchr(line, '\n', size); From b9f2b2f6315359d2aef79ec75a9173f99bfc9297 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:14 -0400 Subject: [PATCH 20/34] http: drop const to fix strstr() warning In redact_sensitive_header(), a C23 implementation of libc will complain that strstr() assigns the result from "const char *cookie" to "char *semicolon". Ultimately the memory is writable. We're fed a strbuf, generate a const pointer "sensitive_header" within it using skip_iprefix(), and then assign the result to "cookie". So we can solve this by dropping the const from "cookie" and "sensitive_header". However, this runs afoul of skip_iprefix(), which wants a "const char **" for its out-parameter. We can solve that by teaching skip_iprefix() the same "make sure out is at least as const as in" magic that we recently taught to skip_prefix(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 8a0566b42b133b73423c801a7ab6f356de69f51a) Signed-off-by: Johannes Schindelin --- git-compat-util.h | 6 ++++-- http.c | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 2d6ae490bdd972..5b236d30b03b06 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -939,8 +939,10 @@ static inline size_t xsize_t(off_t len) * is done via tolower(), so it is strictly ASCII (no multi-byte characters or * locale-specific conversions). */ -static inline bool skip_iprefix(const char *str, const char *prefix, - const char **out) +#define skip_iprefix(str, prefix, out) \ + skip_iprefix_impl((str), (prefix), CONST_OUTPARAM((str), (out))) +static inline bool skip_iprefix_impl(const char *str, const char *prefix, + const char **out) { do { if (!*prefix) { diff --git a/http.c b/http.c index be1fd6f41e8241..51722a21491af6 100644 --- a/http.c +++ b/http.c @@ -766,7 +766,7 @@ static int has_proxy_cert_password(void) static int redact_sensitive_header(struct strbuf *header, size_t offset) { int ret = 0; - const char *sensitive_header; + char *sensitive_header; if (trace_curl_redact && (skip_iprefix(header->buf + offset, "Authorization:", &sensitive_header) || @@ -783,7 +783,7 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset) } else if (trace_curl_redact && skip_iprefix(header->buf + offset, "Cookie:", &sensitive_header)) { struct strbuf redacted_header = STRBUF_INIT; - const char *cookie; + char *cookie; while (isspace(*sensitive_header)) sensitive_header++; From 6e9f1d05956cc8f9afb38bd93798bac2f7db1c33 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:16 -0400 Subject: [PATCH 21/34] refs/files-backend: drop const to fix strchr() warning In show_one_reflog_ent(), we're fed a writable strbuf buffer, which we parse into the various reflog components. We write a NUL over email_end to tie off one of the fields, and thus email_end must be non-const. But with a C23 implementation of libc, strchr() will now complain when assigning the result to a non-const pointer from a const one. So we can fix this by making the source pointer non-const. But there's a catch. We derive that source pointer by parsing the line with parse_oid_hex_algop(), which requires a const pointer for its out-parameter. We can work around that by teaching it to use our CONST_OUTPARAM() trick, just like skip_prefix(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit f1b8a4d10888922eacaa9552d9925d3330ed5d8b) Signed-off-by: Johannes Schindelin --- hex.c | 6 +++--- hex.h | 6 ++++-- refs/files-backend.c | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/hex.c b/hex.c index 865a232167553d..bc756722ca623b 100644 --- a/hex.c +++ b/hex.c @@ -54,9 +54,9 @@ int get_oid_hex(const char *hex, struct object_id *oid) return get_oid_hex_algop(hex, oid, the_hash_algo); } -int parse_oid_hex_algop(const char *hex, struct object_id *oid, - const char **end, - const struct git_hash_algo *algop) +int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, + const char **end, + const struct git_hash_algo *algop) { int ret = get_oid_hex_algop(hex, oid, algop); if (!ret) diff --git a/hex.h b/hex.h index e9ccb54065c0bc..1e9a65d83a4f6b 100644 --- a/hex.h +++ b/hex.h @@ -40,8 +40,10 @@ char *oid_to_hex(const struct object_id *oid); /* same static buffer */ * other invalid character. end is only updated on success; otherwise, it is * unmodified. */ -int parse_oid_hex_algop(const char *hex, struct object_id *oid, const char **end, - const struct git_hash_algo *algo); +#define parse_oid_hex_algop(hex, oid, end, algo) \ + parse_oid_hex_algop_impl((hex), (oid), CONST_OUTPARAM((hex), (end)), (algo)) +int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, const char **end, + const struct git_hash_algo *algo); /* * These functions work like get_oid_hex and parse_oid_hex, but they will parse diff --git a/refs/files-backend.c b/refs/files-backend.c index 80515d3e0833b1..664a62e0d7992f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2183,7 +2183,7 @@ static int show_one_reflog_ent(struct files_ref_store *refs, char *email_end, *message; timestamp_t timestamp; int tz; - const char *p = sb->buf; + char *p = sb->buf; /* old SP new SP name SP time TAB msg LF */ if (!sb->len || sb->buf[sb->len - 1] != '\n' || From addd5cc71226fea1e219ea82e0c13442f7377981 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 4 Apr 2026 01:42:11 -0400 Subject: [PATCH 22/34] git-compat-util: fix CONST_OUTPARAM typo and indentation There's a typo in the comment, making it hard to understand. And the macro itself is indented with spaces rather than tab. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 58589c20e555de187c8026ac721467919595543f) Signed-off-by: Johannes Schindelin --- git-compat-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 5b236d30b03b06..08bd78a16d3285 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -495,7 +495,7 @@ report_fn get_warn_routine(void); void set_die_is_recursing_routine(int (*routine)(void)); /* - * Check that an out-parameter that is "at least as const as" a matching + * Check that an out-parameter is "at least as const as" a matching * in-parameter. For example, skip_prefix() will return "out" that is a subset * of "str". So: * @@ -507,7 +507,7 @@ void set_die_is_recursing_routine(int (*routine)(void)); * See the skip_prefix macro below for an example of use. */ #define CONST_OUTPARAM(in, out) \ - ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) /* * If the string "str" begins with the string found in "prefix", return true. From 5db7be0afc9fd365fdfc89f6846c0ce70351c0d7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:00:52 -0500 Subject: [PATCH 23/34] ref-filter: factor out refname component counting The "lstrip" and "rstrip" options to the %(refname) placeholder both accept a negative length, which asks us to keep that many path components (rather than stripping that many). The code to count components and convert the negative value to a positive was copied from lstrip to rstrip in 1a34728e6b (ref-filter: add an 'rstrip=' option to atoms which deal with refnames, 2017-01-10). Let's factor it out into a separate function. This reduces duplication and also makes the lstrip/rstrip functions much easier to follow, since the bulk of their code is now the actual stripping. Note that the computed "remaining" value is currently stored as a "long", so in theory that's what our function should return. But this is purely historical. When the variable was added in 0571979bd6 (tag: do not show ambiguous tag names as "tags/foo", 2016-01-25), we parsed the value from strtol(), and thus used a long. But these days we take "len" as an int, and also use an int to count up components. So let's just consistently use int here. This value could only overflow in a pathological case (e.g., 4GB worth of "a/a/...") and even then will not result in out-of-bounds memory access (we keep stripping until we run out of string to parse). The minimal Myers diff here is a little hard to read; with --patience the code movement is shown much more clearly. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 5ec4c22e49839c2eeb319070cde14ddbea3d1adb) Signed-off-by: Johannes Schindelin --- ref-filter.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index c318f9ca0ec8dd..ff14ac53de2ed2 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2173,12 +2173,8 @@ static inline char *copy_advance(char *dst, const char *src) return dst; } -static const char *lstrip_ref_components(const char *refname, int len) +static int normalize_component_count(const char *refname, int len) { - long remaining = len; - const char *start = xstrdup(refname); - const char *to_free = start; - if (len < 0) { int i; const char *p = refname; @@ -2192,8 +2188,16 @@ static const char *lstrip_ref_components(const char *refname, int len) * because we count the number of '/', but the number * of components is one more than the no of '/'). */ - remaining = i + len + 1; + len = i + len + 1; } + return len; +} + +static const char *lstrip_ref_components(const char *refname, int len) +{ + int remaining = normalize_component_count(refname, len); + const char *start = xstrdup(refname); + const char *to_free = start; while (remaining > 0) { switch (*start++) { @@ -2213,26 +2217,10 @@ static const char *lstrip_ref_components(const char *refname, int len) static const char *rstrip_ref_components(const char *refname, int len) { - long remaining = len; + int remaining = normalize_component_count(refname, len); const char *start = xstrdup(refname); const char *to_free = start; - if (len < 0) { - int i; - const char *p = refname; - - /* Find total no of '/' separated path-components */ - for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) - ; - /* - * The number of components we need to strip is now - * the total minus the components to be left (Plus one - * because we count the number of '/', but the number - * of components is one more than the no of '/'). - */ - remaining = i + len + 1; - } - while (remaining-- > 0) { char *p = strrchr(start, '/'); if (!p) { From a1da89086f9f18abc947161c1a6319b06110550f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:02:23 -0500 Subject: [PATCH 24/34] ref-filter: simplify lstrip_ref_components() memory handling We're walking forward in the string, skipping path components from left-to-right. So when we've stripped as much as we want, the pointer we have is a complete NUL-terminated string and we can just return it (after duplicating it, of course). So there is no need for a temporary allocated string. But we do make an extra temporary copy due to f0062d3b74 (ref-filter: free item->value and item->value->s, 2018-10-18). This is probably from cargo-culting the technique used in rstrip_ref_components(), which _does_ need a separate string (since it is stripping from the end and ties off the temporary string with a NUL). Let's drop the extra allocation. This is slightly more efficient, but more importantly makes the code much simpler. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 87cb6dc9b0832683a31d0c3126ecad4ad444489c) Signed-off-by: Johannes Schindelin --- ref-filter.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index ff14ac53de2ed2..f5f0cb4ad69632 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2196,13 +2196,10 @@ static int normalize_component_count(const char *refname, int len) static const char *lstrip_ref_components(const char *refname, int len) { int remaining = normalize_component_count(refname, len); - const char *start = xstrdup(refname); - const char *to_free = start; while (remaining > 0) { - switch (*start++) { + switch (*refname++) { case '\0': - free((char *)to_free); return xstrdup(""); case '/': remaining--; @@ -2210,9 +2207,7 @@ static const char *lstrip_ref_components(const char *refname, int len) } } - start = xstrdup(start); - free((char *)to_free); - return start; + return xstrdup(refname); } static const char *rstrip_ref_components(const char *refname, int len) From 2614f1a0e538556fd5513079ee02e386937b3877 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:05:34 -0500 Subject: [PATCH 25/34] ref-filter: simplify rstrip_ref_components() memory handling We're stripping path components from the end of a string, which we do by assigning a NUL as we parse each component, shortening the string. This requires an extra temporary buffer to avoid munging our input string. But the way that we allocate the buffer is unusual. We have an extra "to_free" variable. Usually this is used when the access variable is conceptually const, like: const char *foo; char *to_free = NULL; if (...) foo = to_free = xstrdup(...); else foo = some_const_string; ... free(to_free); But that's not what's happening here. Our "start" variable always points to the allocated buffer, and to_free is redundant. Worse, it is marked as const itself, requiring a cast when we free it. Let's drop to_free entirely, and mark "start" as non-const, making the memory handling more clear. As a bonus, this also silences a warning from glibc-2.43 that our call to strrchr() implicitly strips away the const-ness of "start". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 2ec30e71f44233b9afceda0f2992029674187674) Signed-off-by: Johannes Schindelin --- ref-filter.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index f5f0cb4ad69632..9589418c256840 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2213,13 +2213,12 @@ static const char *lstrip_ref_components(const char *refname, int len) static const char *rstrip_ref_components(const char *refname, int len) { int remaining = normalize_component_count(refname, len); - const char *start = xstrdup(refname); - const char *to_free = start; + char *start = xstrdup(refname); while (remaining-- > 0) { char *p = strrchr(start, '/'); if (!p) { - free((char *)to_free); + free(start); return xstrdup(""); } else p[0] = '\0'; From dcb592c9a69c02f3c1143b4b29bb0627d4cdfae3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 15 Feb 2026 04:07:44 -0500 Subject: [PATCH 26/34] ref-filter: avoid strrchr() in rstrip_ref_components() To strip path components from our refname string, we repeatedly call strrchr() to find the trailing slash, shortening the string each time by assigning NUL over it. This has two downsides: 1. Calling strrchr() in a loop is quadratic, since each call has to call strlen() under the hood to find the end of the string (even though we know exactly where it is from the last loop iteration). 2. We need a temporary buffer, since we're munging the string with NUL as we shorten it (which we must do, because strrchr() has no other way of knowing what we consider the end of the string). Using memrchr() would let us fix both of these, but it isn't portable. So instead, let's just open-code the string traversal from back to front as we loop. I doubt that the quadratic nature is a serious concern. You can see it in practice with something like: git init git commit --allow-empty -m foo echo "$(git rev-parse HEAD) refs/heads$(perl -e 'print "/a" x 500_000')" >.git/packed-refs time git for-each-ref --format='%(refname:rstrip=-1)' That takes ~5.5s to run on my machine before this patch, and ~11ms after. But I don't think there's a reasonable way for somebody to infect you with such a garbage ref, as the wire protocol is limited to 64k pkt-lines. The difference is measurable for me for a 32k-component ref (about 19ms vs 7ms), so perhaps you could create some chaos by pushing a lot of them. But we also run into filesystem limits (if the loose backend is in use), and in practice it seems like there are probably simpler and more effective ways to waste CPU. Likewise the extra allocation probably isn't really measurable. In fact, since our goal is to return an allocated string, we end up having to make the same allocation anyway (though it is sized to the result, rather than the input). My main goal was simplicity in avoiding the need to handle cleaning it up in the early return path. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit fe732a8b9f1837189eb3533a262548a652c11e61) Signed-off-by: Johannes Schindelin --- ref-filter.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 9589418c256840..01512d50bd6d38 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2213,17 +2213,15 @@ static const char *lstrip_ref_components(const char *refname, int len) static const char *rstrip_ref_components(const char *refname, int len) { int remaining = normalize_component_count(refname, len); - char *start = xstrdup(refname); + const char *end = refname + strlen(refname); - while (remaining-- > 0) { - char *p = strrchr(start, '/'); - if (!p) { - free(start); + while (remaining > 0) { + if (end == refname) return xstrdup(""); - } else - p[0] = '\0'; + if (*--end == '/') + remaining--; } - return start; + return xmemdupz(refname, end - refname); } static const char *show_ref(struct refname_atom *atom, const char *refname) From 0694b501d9157eaf7d0fcda2ba4c58dc4338e00e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 Apr 2026 11:22:49 +0200 Subject: [PATCH 27/34] fixup! hooks: add custom post-command hook config Accommodate for C23 enforcing `const`-ness of `strchr()`'s return value. Signed-off-by: Johannes Schindelin --- hook.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/hook.c b/hook.c index 9adb7aba23df51..7419a96c035ec3 100644 --- a/hook.c +++ b/hook.c @@ -181,20 +181,14 @@ static char *get_post_index_change_sentinel_name(struct repository *r) { struct strbuf path = STRBUF_INIT; const char *sid = tr2_sid_get(); - char *slash = strchr(sid, '/'); - - /* - * Name is based on top-level SID, so children can indicate that - * the top-level process should run the post-command hook. - */ - if (slash) - *slash = 0; + const char *slash = strchrnul(sid, '/'); /* * Do not write to hooks directory, as it could be redirected * somewhere like the source tree. */ - repo_git_path_replace(r, &path, "info/index-change-%s.snt", sid); + repo_git_path_replace(r, &path, "info/index-change-%.*s.snt", + (int)(slash - sid), sid); return strbuf_detach(&path, NULL); } From 88487dd7aa999c5f3bbe5d626e887436fdd093a6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 20 Feb 2026 01:00:03 -0500 Subject: [PATCH 28/34] ref-filter: clarify lstrip/rstrip component counting When a strip option to the %(refname) placeholder is asked to leave N path components, we first count up the path components to know how many to remove. That happens with a loop like this: /* Find total no of '/' separated path-components */ for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) ; which is a little hard to understand for two reasons. First, the dereference in "*p++" is seemingly useless, since nobody looks at the result. And static analyzers like Coverity will complain about that. But removing the "*" will cause gcc to complain with -Wint-conversion, since the two sides of the ternary do not match (one is a pointer and the other an int). Second, it is not clear what the meaning of "p" is at each iteration of the loop, as its position with respect to our walk over the string depends on how many slashes we've seen. The answer is that by itself, it doesn't really mean anything: "p + i" represents the current state of our walk, with "i" counting up slashes, and "p" by itself essentially meaningless. None of this behaves incorrectly, but ultimately the loop is just counting the slashes in the refname. We can do that much more simply with a for-loop iterating over the string and a separate slash counter. We can also drop the comment, which is somewhat misleading. We are counting slashes, not components (and a comment later in the function makes it clear that we must add one to compensate). In the new code it is obvious that we are counting slashes here. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 8b0061b5c5008375ef0986b7aafedbd7d79da0f6) Signed-off-by: Johannes Schindelin --- ref-filter.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 01512d50bd6d38..db83d3b32c3e20 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2176,19 +2176,20 @@ static inline char *copy_advance(char *dst, const char *src) static int normalize_component_count(const char *refname, int len) { if (len < 0) { - int i; - const char *p = refname; + int slashes = 0; + + for (const char *p = refname; *p; p++) { + if (*p == '/') + slashes++; + } - /* Find total no of '/' separated path-components */ - for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) - ; /* * The number of components we need to strip is now * the total minus the components to be left (Plus one * because we count the number of '/', but the number * of components is one more than the no of '/'). */ - len = i + len + 1; + len = slashes + len + 1; } return len; } From bd1e75afa428605dd94a2df6c2ca4d20a4ecd764 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 Mar 2026 15:04:44 -0400 Subject: [PATCH 29/34] revision: make handle_dotdot() interface less confusing There are two very subtle bits to the way we parse ".." (and "...") range operators: 1. In handle_dotdot_1(), we assume that the incoming arguments "dotdot" and "arg" are part of the same string, with the first digit of the range-operator blanked to a NUL. Then when we want the full name (e.g., to report an error), we replace the NUL with a dot to restore the original string. 2. In handle_dotdot(), we take in a const string, but then we modify it by overwriting the range operator with a NUL. This has worked OK in practice since we tend to pass in buffers that are actually writeable (including argv), but segfaults with something like: handle_revision_arg("..HEAD", &revs, 0, 0); On top of that, building with recent versions of glibc causes the compiler to complain, because it notices when we use strchr() or strstr() to launder away constness (basically detecting the possibility of the segfault above via the type system). Instead of munging the buffer, let's instead make a temporary copy of the left-hand side of the range operator. That avoids any const violations, and lets us pass around the parsed elements independently: the left-hand side, the right-hand side, the number of dots (via the "symmetric" flag), and the original full string for error messages. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 4d5fb9377bba1f45a940e10b0b7354fe7db2b301) Signed-off-by: Johannes Schindelin --- revision.c | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/revision.c b/revision.c index 72750f973f1630..38a2635e15b6c7 100644 --- a/revision.c +++ b/revision.c @@ -2037,41 +2037,32 @@ static void prepare_show_merge(struct rev_info *revs) free(prune); } -static int dotdot_missing(const char *arg, char *dotdot, +static int dotdot_missing(const char *full_name, struct rev_info *revs, int symmetric) { if (revs->ignore_missing) return 0; - /* de-munge so we report the full argument */ - *dotdot = '.'; die(symmetric ? "Invalid symmetric difference expression %s" - : "Invalid revision range %s", arg); + : "Invalid revision range %s", full_name); } -static int handle_dotdot_1(const char *arg, char *dotdot, +static int handle_dotdot_1(const char *a_name, const char *b_name, + const char *full_name, int symmetric, struct rev_info *revs, int flags, int cant_be_filename, struct object_context *a_oc, struct object_context *b_oc) { - const char *a_name, *b_name; struct object_id a_oid, b_oid; struct object *a_obj, *b_obj; unsigned int a_flags, b_flags; - int symmetric = 0; unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM); unsigned int oc_flags = GET_OID_COMMITTISH | GET_OID_RECORD_PATH; - a_name = arg; if (!*a_name) a_name = "HEAD"; - b_name = dotdot + 2; - if (*b_name == '.') { - symmetric = 1; - b_name++; - } if (!*b_name) b_name = "HEAD"; @@ -2080,15 +2071,13 @@ static int handle_dotdot_1(const char *arg, char *dotdot, return -1; if (!cant_be_filename) { - *dotdot = '.'; - verify_non_filename(revs->prefix, arg); - *dotdot = '\0'; + verify_non_filename(revs->prefix, full_name); } a_obj = parse_object(revs->repo, &a_oid); b_obj = parse_object(revs->repo, &b_oid); if (!a_obj || !b_obj) - return dotdot_missing(arg, dotdot, revs, symmetric); + return dotdot_missing(full_name, revs, symmetric); if (!symmetric) { /* just A..B */ @@ -2102,7 +2091,7 @@ static int handle_dotdot_1(const char *arg, char *dotdot, a = lookup_commit_reference(revs->repo, &a_obj->oid); b = lookup_commit_reference(revs->repo, &b_obj->oid); if (!a || !b) - return dotdot_missing(arg, dotdot, revs, symmetric); + return dotdot_missing(full_name, revs, symmetric); if (repo_get_merge_bases(the_repository, a, b, &exclude) < 0) { free_commit_list(exclude); @@ -2131,16 +2120,23 @@ static int handle_dotdot(const char *arg, int cant_be_filename) { struct object_context a_oc = {0}, b_oc = {0}; - char *dotdot = strstr(arg, ".."); + const char *dotdot = strstr(arg, ".."); + char *tmp; + int symmetric = 0; int ret; if (!dotdot) return -1; - *dotdot = '\0'; - ret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename, - &a_oc, &b_oc); - *dotdot = '.'; + tmp = xmemdupz(arg, dotdot - arg); + dotdot += 2; + if (*dotdot == '.') { + symmetric = 1; + dotdot++; + } + ret = handle_dotdot_1(tmp, dotdot, arg, symmetric, revs, flags, + cant_be_filename, &a_oc, &b_oc); + free(tmp); object_context_release(&a_oc); object_context_release(&b_oc); From d9210bbfc0c56422e0f9f49c3164bd87e045f4da Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 Mar 2026 15:05:25 -0400 Subject: [PATCH 30/34] rev-parse: simplify dotdot parsing The previous commit simplified the way that revision.c parses ".." and "..." range operators. But there's roughly similar code in rev-parse. This is less likely to trigger a segfault, as there is no library function which we'd pass a string literal to, but it still causes the compiler to complain about laundering away constness via strstr(). Let's give it the same treatment, copying the left-hand side of the range operator into its own string. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 268a9caaf29f0269147dacbea2c8d439c505c5ee) Signed-off-by: Johannes Schindelin --- builtin/rev-parse.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 9032cc6327c004..b655b8f5c7f95b 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -267,21 +267,20 @@ static int show_file(const char *arg, int output_prefix) static int try_difference(const char *arg) { - char *dotdot; + const char *dotdot; struct object_id start_oid; struct object_id end_oid; const char *end; const char *start; + char *to_free; int symmetric; static const char head_by_default[] = "HEAD"; if (!(dotdot = strstr(arg, ".."))) return 0; + start = to_free = xmemdupz(arg, dotdot - arg); end = dotdot + 2; - start = arg; symmetric = (*end == '.'); - - *dotdot = 0; end += symmetric; if (!*end) @@ -295,7 +294,7 @@ static int try_difference(const char *arg) * Just ".."? That is not a range but the * pathspec for the parent directory. */ - *dotdot = '.'; + free(to_free); return 0; } @@ -308,7 +307,7 @@ static int try_difference(const char *arg) a = lookup_commit_reference(the_repository, &start_oid); b = lookup_commit_reference(the_repository, &end_oid); if (!a || !b) { - *dotdot = '.'; + free(to_free); return 0; } if (repo_get_merge_bases(the_repository, a, b, &exclude) < 0) @@ -318,10 +317,10 @@ static int try_difference(const char *arg) show_rev(REVERSED, &commit->object.oid, NULL); } } - *dotdot = '.'; + free(to_free); return 1; } - *dotdot = '.'; + free(to_free); return 0; } From eadc6d9f8c6169fa75d2fc4bcf505e30563d42ed Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 Mar 2026 15:13:18 -0400 Subject: [PATCH 31/34] revision: avoid writing to const string for parent marks We take in a "const char *", but may write a NUL into it when parsing parent marks like "foo^-", since we want to isolate "foo" as a string for further parsing. This is usually OK, as our "const" strings are often actually argv strings which are technically writeable, but we'd segfault with a string literal like: handle_revision_arg("HEAD^-", &revs, 0, 0); Similar to how we handled dotdot in a previous commit, we can avoid this by making a temporary copy of the left-hand side of the string. The cost should negligible compared to the rest of the parsing (like actually parsing commits to create their parent linked-lists). There is one slightly tricky thing, though. We parse some of the marks progressively, so that if we see "foo^!" for example, we'll strip that down to "foo" not just for calling add_parents_only(), but also for the rest of the function. That makes sense since we eventually want to pass "foo" to get_oid_with_context(). But it also means that we'll keep looking for other marks. In particular, "foo^-^!" is valid, though oddly "foo^!^-" would ignore the "^-". I'm not sure if this is a weird historical artifact of the implementation, or if there are important corner cases. So I've left the behavior unchanged. Each mark we find allocates a string with the mark stripped, which means we could allocate multiple times (and carry a free-able pointer for each to the end). But in practice we won't, because of the three marks, "^@" jumps immediately to the end without further parsing, and "^-^!" is nonsense that nobody would pass. So you'd get one allocation in general, and never more than two. Another obvious option would be to just copy "arg" up front and be OK with munging it. But that means we pay the cost even when we find no marks. We could make a single copy upon finding a mark and then munge, but that adds extra code to each site (checking whether somebody else allocated, and if not, adjusting our "mark" pointer to be relative to the copied string). I aimed for something that was clear and obvious, if a bit verbose. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 22b985ef193a18ec0e6602ea1838e3290a351dd6) Signed-off-by: Johannes Schindelin --- revision.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/revision.c b/revision.c index 38a2635e15b6c7..219b9b6b9dacd4 100644 --- a/revision.c +++ b/revision.c @@ -2146,7 +2146,10 @@ static int handle_dotdot(const char *arg, static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int flags, unsigned revarg_opt) { struct object_context oc = {0}; - char *mark; + const char *mark; + char *arg_minus_at = NULL; + char *arg_minus_excl = NULL; + char *arg_minus_dash = NULL; struct object *object; struct object_id oid; int local_flags; @@ -2173,18 +2176,17 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl mark = strstr(arg, "^@"); if (mark && !mark[2]) { - *mark = 0; - if (add_parents_only(revs, arg, flags, 0)) { + arg_minus_at = xmemdupz(arg, mark - arg); + if (add_parents_only(revs, arg_minus_at, flags, 0)) { ret = 0; goto out; } - *mark = '^'; } mark = strstr(arg, "^!"); if (mark && !mark[2]) { - *mark = 0; - if (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), 0)) - *mark = '^'; + arg_minus_excl = xmemdupz(arg, mark - arg); + if (add_parents_only(revs, arg_minus_excl, flags ^ (UNINTERESTING | BOTTOM), 0)) + arg = arg_minus_excl; } mark = strstr(arg, "^-"); if (mark) { @@ -2198,9 +2200,9 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl } } - *mark = 0; - if (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), exclude_parent)) - *mark = '^'; + arg_minus_dash = xmemdupz(arg, mark - arg); + if (add_parents_only(revs, arg_minus_dash, flags ^ (UNINTERESTING | BOTTOM), exclude_parent)) + arg = arg_minus_dash; } local_flags = 0; @@ -2235,6 +2237,9 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl out: object_context_release(&oc); + free(arg_minus_at); + free(arg_minus_excl); + free(arg_minus_dash); return ret; } From 023941748085ba3c9608f4a4f97adcfac87011cc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 Mar 2026 15:14:24 -0400 Subject: [PATCH 32/34] rev-parse: avoid writing to const string for parent marks The previous commit cleared up some const confusion in handling parent marks in revision.c, but we have roughly the same code duplicated in rev-parse. This one is much easier to fix, because the handling of the shortened string is all done in one place, after detecting any marks (but without shortening the string between marks). As a side note, I suspect this means that it behaves differently than the revision.c parser for weird stuff like "foo^!^@^-", but that is outside the scope of this patch. While we are here, let's also rename the variable "dotdot", which is totally misleading (and which we already fixed in revision.c long ago via f632dedd8d (handle_revision_arg: stop using "dotdot" as a generic pointer, 2017-05-19)). Doing that here makes the diff a little messier, but it also lets the compiler help us make sure we did not miss any stray mentions of the variable while we are changing its semantics. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit 213b2138770d820bc28fde839f3e4df90a5d5d81) Signed-off-by: Johannes Schindelin --- builtin/rev-parse.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index b655b8f5c7f95b..7fe7f1e9b5b5f7 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -326,7 +326,7 @@ static int try_difference(const char *arg) static int try_parent_shorthands(const char *arg) { - char *dotdot; + const char *mark; struct object_id oid; struct commit *commit; struct commit_list *parents; @@ -334,38 +334,39 @@ static int try_parent_shorthands(const char *arg) int include_rev = 0; int include_parents = 0; int exclude_parent = 0; + char *to_free; - if ((dotdot = strstr(arg, "^!"))) { + if ((mark = strstr(arg, "^!"))) { include_rev = 1; - if (dotdot[2]) + if (mark[2]) return 0; - } else if ((dotdot = strstr(arg, "^@"))) { + } else if ((mark = strstr(arg, "^@"))) { include_parents = 1; - if (dotdot[2]) + if (mark[2]) return 0; - } else if ((dotdot = strstr(arg, "^-"))) { + } else if ((mark = strstr(arg, "^-"))) { include_rev = 1; exclude_parent = 1; - if (dotdot[2]) { + if (mark[2]) { char *end; - exclude_parent = strtoul(dotdot + 2, &end, 10); + exclude_parent = strtoul(mark + 2, &end, 10); if (*end != '\0' || !exclude_parent) return 0; } } else return 0; - *dotdot = 0; + arg = to_free = xmemdupz(arg, mark - arg); if (repo_get_oid_committish(the_repository, arg, &oid) || !(commit = lookup_commit_reference(the_repository, &oid))) { - *dotdot = '^'; + free(to_free); return 0; } if (exclude_parent && exclude_parent > commit_list_count(commit->parents)) { - *dotdot = '^'; + free(to_free); return 0; } @@ -386,7 +387,7 @@ static int try_parent_shorthands(const char *arg) free(name); } - *dotdot = '^'; + free(to_free); return 1; } From fd0d5ceb537b0b09d92b6a86a4197f341122ebd0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 Mar 2026 15:23:20 -0400 Subject: [PATCH 33/34] config: store allocated string in non-const pointer When git-config matches a url, we copy the variable section name and store it in the "section" member of a urlmatch_config struct. That member is const, since the url-matcher will not touch it (and other callers really will have a const string). But that means that we have only a const pointer to our allocated string. We have to cast away the constness when we free it, and likewise when we assign NUL to tie off the "." separating the subsection and key. This latter happens implicitly via a strchr() call, but recent versions of glibc have added annotations that let the compiler detect that and complain. Let's keep our own "section" pointer for the non-const string, and then just point config.section at it. That avoids all of the casting, both explicit and implicit. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano (cherry picked from commit d385845d55e0e3a775fc47ac8d73a5ec41308db3) Signed-off-by: Johannes Schindelin --- builtin/config.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 7c4857be622904..cf4ba0f7cc6f22 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -838,6 +838,7 @@ static int get_urlmatch(const struct config_location_options *opts, const char *var, const char *url) { int ret; + char *section; char *section_tail; struct config_display_options display_opts = *_display_opts; struct string_list_item *item; @@ -851,8 +852,8 @@ static int get_urlmatch(const struct config_location_options *opts, if (!url_normalize(url, &config.url)) die("%s", config.url.err); - config.section = xstrdup_tolower(var); - section_tail = strchr(config.section, '.'); + config.section = section = xstrdup_tolower(var); + section_tail = strchr(section, '.'); if (section_tail) { *section_tail = '\0'; config.key = section_tail + 1; @@ -886,7 +887,7 @@ static int get_urlmatch(const struct config_location_options *opts, string_list_clear(&values, 1); free(config.url.url); - free((void *)config.section); + free(section); return ret; } From 4722b670878467d757873d1ee60559ee1351971b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 Apr 2026 09:56:13 +0000 Subject: [PATCH 34/34] fixup! Add virtual file system settings and hook proc We need to accommodate for C23 being very strict about the `const`-ness of the return value of the `strchr()` function. Signed-off-by: Johannes Schindelin --- virtualfilesystem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtualfilesystem.c b/virtualfilesystem.c index 269af2de1d969d..b3adf9a0894b35 100644 --- a/virtualfilesystem.c +++ b/virtualfilesystem.c @@ -148,7 +148,7 @@ int is_included_in_virtualfilesystem(const char *pathname, int pathlen) static void parent_directory_hashmap_add(struct hashmap *map, const char *pattern, const int patternlen) { - char *slash; + const char *slash; struct virtualfilesystem *vfs; /*