[PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Collin Funk]

Unsure if this should be tagged [RFC], but this patch clears up lots
of warning spam with glibc 2.43 because of a change mentioned in the
commit message.

I plan to handle the rest of them and try to organize the changes by
subsystem, for lack of a better term. But I figured it was best to
submit just this one for review first.

-- 8< --

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:

        CC abspath.o
    In file included from abspath.c:1:
    git-compat-util.h: In function ‘git_find_last_dir_sep’:
    git-compat-util.h:344:16: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
      344 |         return strrchr(path, '/');
          |                ^~~~~~~

Most of the warnings are from git_find_last_dir_sep which calls strrchr
on a "const char *" but returns a "char *". This patch addresses them by
changing the return type to be const, since only one location needs the
qualifier casted away.

Signed-off-by: Collin Funk <collin.funk1@gmail.com>
---
 config.c          | 2 +-
 git-compat-util.h | 2 +-
 remote.c          | 2 +-
 scalar.c          | 8 ++++----
 strbuf.c          | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/config.c b/config.c
index 7f6d53b473..156f2a24fa 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/git-compat-util.h b/git-compat-util.h
index bebcf9f698..fb4251564a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -339,7 +339,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)
+static inline const char *git_find_last_dir_sep(const char *path)
 {
 	return strrchr(path, '/');
 }
diff --git a/remote.c b/remote.c
index b756ff6f15..8c1a0a0c15 100644
--- a/remote.c
+++ b/remote.c
@@ -2753,7 +2753,7 @@ void remote_state_clear(struct remote_state *remote_state)
  */
 static int chop_last_dir(char **remoteurl, int is_relative)
 {
-	char *rfind = find_last_dir_sep(*remoteurl);
+	char *rfind = (char *) find_last_dir_sep(*remoteurl);
 	if (rfind) {
 		*rfind = '\0';
 		return 0;
diff --git a/scalar.c b/scalar.c
index c9df9348ec..54a75ad971 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"));
@@ -479,11 +479,11 @@ static int cmd_clone(int argc, const char **argv)
 		/* Strip suffix `.git`, if any */
 		strbuf_strip_suffix(&buf, ".git");
 
-		enlistment = find_last_dir_sep(buf.buf);
-		if (!enlistment) {
+		const char *last = find_last_dir_sep(buf.buf);
+		if (!last) {
 			die(_("cannot deduce worktree name from '%s'"), url);
 		}
-		enlistment = xstrdup(enlistment + 1);
+		enlistment = xstrdup(last + 1);
 	} else {
 		usage_msg_opt(_("You must specify a repository to clone."),
 			      clone_usage, clone_options);
diff --git a/strbuf.c b/strbuf.c
index 59678bf5b0..3939863cf3 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);
 }
-- 
2.52.0

Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Jeff King]

On Mon, Feb 02, 2026 at 09:19:01PM -0800, Collin Funk wrote:

> Unsure if this should be tagged [RFC], but this patch clears up lots
> of warning spam with glibc 2.43 because of a change mentioned in the
> commit message.

Thanks for the heads-up. I can reproduce here by installing glibc 2.43
via "apt install -t experimental libc6" on my debian unstable machine.

> I plan to handle the rest of them and try to organize the changes by
> subsystem, for lack of a better term. But I figured it was best to
> submit just this one for review first.

Wow, there's...a lot of spots. Looks like ~65 of them based on my hacky
first-pass. Many of them are quite obvious "s/char/const char/" fixes in
variable declarations, that should have been const all along. I think
those can all go together in one patch, as the compiler can verify that
we never try to write to the result.

And then, yeah, I'd do the tricky ones system by system. Some of the
ones that do write to the resulting pointers are rather nasty, and seem
to fall into one of two camps:

  1. Some function interface takes a const pointer, even though we try
     to write to it under the hood (after laundering it through strchr()
     or similar). I think it would be worth refactoring these interfaces
     when we can, though some of them are pretty questionable. For
     instance, all of the rev-parse/revision.c "dotdot" parsing works on
     a "const char *arg". Surely we feed this from command line options
     in some cases? I guess argv is guaranteed to be writable by the
     standard, though we tend to treat is as const everywhere.

  2. We know we have a non-const pointer, but it is passed through a
     const pointer that is used as an out-parameter to a function like
     skip_prefix(). For instance, in http.c's redact_sensitive_header()
     we have something like this:

        const char *sensitive_header;
	if (skip_iprefix(header->buf, "Cookie:", &sensitive_header)) {
		const char *cookie = sensitive_header;
		char *semicolon = strchr(cookie, "; ");
		*semicolon = 0;
		...

     Our header->buf here is a strbuf, so we know we are working with a
     non-const buffer. We launder away constness with the strchr()
     assignment to "semicolon", which glibc now complains about. We
     should make "cookie" non-const, which is easy. But now we'll get a
     complaint about assigning the const "sensitive_header" to it. And
     that one should _also_ be non-const, because it comes from
     header->buf. But switching it will cause the compiler to complain
     about passing it to skip_iprefix().

     So we have the problem in reverse (instead of laundering a const
     string to a non-const, we've accidentally added constness where it
     is not needed). If we drop the const from skip_iprefix(), then that
     has fallout in all the other spots that do pass in a const haystack
     parameter.

     I don't know what the right solution is here. I guess the best we
     can do is probably adding casts with comments like "this is OK
     because it comes from...". But I'm not sure if we are better to
     cast away the constness in one spot, or to make all of the
     variables non-const and cast the out-parameter to skip_iprefix().

>  #ifndef find_last_dir_sep
> -static inline char *git_find_last_dir_sep(const char *path)
> +static inline const char *git_find_last_dir_sep(const char *path)
>  {
>  	return strrchr(path, '/');
>  }

This kind of recreates that reverse problem again, though: any caller
who really does have a non-const "path" will get "const" added back into
it. And that leads to casts like...

>  static int chop_last_dir(char **remoteurl, int is_relative)
>  {
> -	char *rfind = find_last_dir_sep(*remoteurl);
> +	char *rfind = (char *) find_last_dir_sep(*remoteurl);
>  	if (rfind) {
>  		*rfind = '\0';
>  		return 0;

...this one. Can we implement it as a macro? That lets the compiler do
the right thing, because we do not declare any type then. It used to be
a macro, but switched in bf7283465b (turn path macros into inline
function, 2014-08-16). There's also a level of macro indirection; on
Windows this expands to win32_find_last_dir_sep(), which of course casts
away the constness manually. ;)

I also wonder if we could do some gcc/glibc-specific magic to get the
best of both worlds. That is, could we get the same "the return value is
const if the input parameter was" type-checking that is happening with
strchr()?

Looking at strchr()'s declaration in string.h, which is defined like:

  #  define strchr(S, C)                                          \
    __glibc_const_generic (S, const char *, strchr (S, C))

I think the answer is probably "yes". But it also doesn't quite solve
our problem. That would give us type-checking of callers of our
function, but we still have to convince the compiler not to complain
about its implementation. For that we'd need to either cast away const
manually, I guess.

Yuck. What a mess. I do think that fixing these warnings will improve
most of the call-sites I looked at, but some of them get a bit hairy.

-Peff

Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Collin Funk]

Jeff King <peff@peff.net> writes:

> On Mon, Feb 02, 2026 at 09:19:01PM -0800, Collin Funk wrote:
>
>> Unsure if this should be tagged [RFC], but this patch clears up lots
>> of warning spam with glibc 2.43 because of a change mentioned in the
>> commit message.
>
> Thanks for the heads-up. I can reproduce here by installing glibc 2.43
> via "apt install -t experimental libc6" on my debian unstable machine.
>
>> I plan to handle the rest of them and try to organize the changes by
>> subsystem, for lack of a better term. But I figured it was best to
>> submit just this one for review first.
>
> Wow, there's...a lot of spots. Looks like ~65 of them based on my hacky
> first-pass. Many of them are quite obvious "s/char/const char/" fixes in
> variable declarations, that should have been const all along. I think
> those can all go together in one patch, as the compiler can verify that
> we never try to write to the result.

Yep, it is quite noisy.

And that plan makes sense to me. I'll create a seperate patch handling
the obvious 's/char/const char/' conversions that make sense regardless
of this glibc change.

> And then, yeah, I'd do the tricky ones system by system. Some of the
> ones that do write to the resulting pointers are rather nasty, and seem
> to fall into one of two camps:
>
>   1. Some function interface takes a const pointer, even though we try
>      to write to it under the hood (after laundering it through strchr()
>      or similar). I think it would be worth refactoring these interfaces
>      when we can, though some of them are pretty questionable. For
>      instance, all of the rev-parse/revision.c "dotdot" parsing works on
>      a "const char *arg". Surely we feed this from command line options
>      in some cases? I guess argv is guaranteed to be writable by the
>      standard, though we tend to treat is as const everywhere.

Yes, I see. I think the "arg" there is from the command line or from a
buffer read using fgets() in get_object_list from
builtin/pack-objects.c, so it is safe to write to there.

It's also called like this though:

    handle_revision_arg("HEAD", &revs, 0, 0);

We can't write to the string "HEAD", but it doesn't have a "dotdot" so
we don't. It could probably be cleaned up a bit.

FYI, that code would also be made much clearer if not all of the
declarations were at the top of the function. I guess it just hasn't
been touched in a long while.

>   2. We know we have a non-const pointer, but it is passed through a
>      const pointer that is used as an out-parameter to a function like
>      skip_prefix(). For instance, in http.c's redact_sensitive_header()
>      we have something like this:
>
>         const char *sensitive_header;
> 	if (skip_iprefix(header->buf, "Cookie:", &sensitive_header)) {
> 		const char *cookie = sensitive_header;
> 		char *semicolon = strchr(cookie, "; ");
> 		*semicolon = 0;
> 		...
>
>      Our header->buf here is a strbuf, so we know we are working with a
>      non-const buffer. We launder away constness with the strchr()
>      assignment to "semicolon", which glibc now complains about. We
>      should make "cookie" non-const, which is easy. But now we'll get a
>      complaint about assigning the const "sensitive_header" to it. And
>      that one should _also_ be non-const, because it comes from
>      header->buf. But switching it will cause the compiler to complain
>      about passing it to skip_iprefix().
>
>      So we have the problem in reverse (instead of laundering a const
>      string to a non-const, we've accidentally added constness where it
>      is not needed). If we drop the const from skip_iprefix(), then that
>      has fallout in all the other spots that do pass in a const haystack
>      parameter.
>
>      I don't know what the right solution is here. I guess the best we
>      can do is probably adding casts with comments like "this is OK
>      because it comes from...". But I'm not sure if we are better to
>      cast away the constness in one spot, or to make all of the
>      variables non-const and cast the out-parameter to skip_iprefix().

Makes sense, I'll try to handle the non-obviously ones separately.

>
>>  #ifndef find_last_dir_sep
>> -static inline char *git_find_last_dir_sep(const char *path)
>> +static inline const char *git_find_last_dir_sep(const char *path)
>>  {
>>  	return strrchr(path, '/');
>>  }
>
> This kind of recreates that reverse problem again, though: any caller
> who really does have a non-const "path" will get "const" added back into
> it. And that leads to casts like...

I figured it was okay since only one place casted the qualifier away.
But I agree it is probably worth cleaning that up later.

>>  static int chop_last_dir(char **remoteurl, int is_relative)
>>  {
>> -	char *rfind = find_last_dir_sep(*remoteurl);
>> +	char *rfind = (char *) find_last_dir_sep(*remoteurl);
>>  	if (rfind) {
>>  		*rfind = '\0';
>>  		return 0;
>
> ...this one. Can we implement it as a macro? That lets the compiler do
> the right thing, because we do not declare any type then. It used to be
> a macro, but switched in bf7283465b (turn path macros into inline
> function, 2014-08-16). There's also a level of macro indirection; on
> Windows this expands to win32_find_last_dir_sep(), which of course casts
> away the constness manually. ;)
>
> I also wonder if we could do some gcc/glibc-specific magic to get the
> best of both worlds. That is, could we get the same "the return value is
> const if the input parameter was" type-checking that is happening with
> strchr()?
>
> Looking at strchr()'s declaration in string.h, which is defined like:
>
>   #  define strchr(S, C)                                          \
>     __glibc_const_generic (S, const char *, strchr (S, C))
>
> I think the answer is probably "yes". But it also doesn't quite solve
> our problem. That would give us type-checking of callers of our
> function, but we still have to convince the compiler not to complain
> about its implementation. For that we'd need to either cast away const
> manually, I guess.

That macro depends on Generic selections from C11 [1]. I wasn't sure if
Git would like that, given it is conservative with other C features.

> Yuck. What a mess. I do think that fixing these warnings will improve
> most of the call-sites I looked at, but some of them get a bit hairy.

Thanks to C23. :)

Collin

[1] https://en.cppreference.com/w/c/language/generic.html

Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Jeff King]

On Tue, Feb 03, 2026 at 07:15:10PM -0800, Collin Funk wrote:

> And that plan makes sense to me. I'll create a seperate patch handling
> the obvious 's/char/const char/' conversions that make sense regardless
> of this glibc change.

Sounds good. BTW, thank you for working on this! I think some of it will
be a slog. :)

> Yes, I see. I think the "arg" there is from the command line or from a
> buffer read using fgets() in get_object_list from
> builtin/pack-objects.c, so it is safe to write to there.
> 
> It's also called like this though:
> 
>     handle_revision_arg("HEAD", &revs, 0, 0);
> 
> We can't write to the string "HEAD", but it doesn't have a "dotdot" so
> we don't. It could probably be cleaned up a bit.

Oof, yeah. So this is a trap waiting to happen, and any code like:

  handle_revision_arg("..HEAD", &revs, 0, 0);

would segfault. That's something we're unlikely to write, which is how
it's managed to hang around for so long. But I'm happy that the new
warning will help us find and fix such cases.

This is a case where I think the interface really should be a const
string, and we should just pay the cost to make a NUL-terminated
version. I.e., something like this:

diff --git a/revision.c b/revision.c
index ba0da18f26..289af7507c 100644
--- a/revision.c
+++ b/revision.c
@@ -2143,16 +2143,17 @@ 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 *lhs;
 	int ret;
 
 	if (!dotdot)
 		return -1;
 
-	*dotdot = '\0';
-	ret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename,
+	lhs = xmemdupz(arg, dotdot - arg);
+	ret = handle_dotdot_1(arg, lhs, revs, flags, cant_be_filename,
 			      &a_oc, &b_oc);
-	*dotdot = '.';
+	free(lhs);
 
 	object_context_release(&a_oc);
 	object_context_release(&b_oc);

This isn't a hot enough code path for the allocation to matter, and
simple and safe is the best approach (IMHO). I suspect many cases you'll
find are similar.

> FYI, that code would also be made much clearer if not all of the
> declarations were at the top of the function. I guess it just hasn't
> been touched in a long while.

Though mixed statements and declarations are allowed by modern C99
(which we require these days), I think we still prefer not to use it as
a point of style. And we enforce it via -Wdeclaration-after-statement,
at least with all of our developer warnings on.

Speaking of which: I noticed your original patch introduced one such
case. Make sure you're building with "make DEVELOPER=1" as you build and
test.

All that said, there are often cases where variable declarations could
be pushed down into the inner block where they are used, and those sorts
of cleanups are welcome. We also allow declaring variables in loop
initializers these days.

> > Looking at strchr()'s declaration in string.h, which is defined like:
> >
> >   #  define strchr(S, C)                                          \
> >     __glibc_const_generic (S, const char *, strchr (S, C))
> >
> > I think the answer is probably "yes". But it also doesn't quite solve
> > our problem. That would give us type-checking of callers of our
> > function, but we still have to convince the compiler not to complain
> > about its implementation. For that we'd need to either cast away const
> > manually, I guess.
> 
> That macro depends on Generic selections from C11 [1]. I wasn't sure if
> Git would like that, given it is conservative with other C features.

We definitely can't rely on it everywhere. But if there is a solution
that is conditionally compiled, and can kick in only when these extra
warnings also kick in, that would be OK. Assuming the result is not too
painful to look at, of course.

Probably the best path forward for most spots is just fixing the code to
make it more obvious about its use of const. We may find there are not
enough left for us to try to get too clever afterwards.

Even though I think the skip_iprefix() thing is a general problem with
constness in C (the same one faced by strchr() in the first place!), in
practice we can probably just rewrite the code in the few cases where it
matters. For instance, the "cookie" example I gave could probably just
do something like this:

diff --git a/http.c b/http.c
index 7815f144de..e6f0913691 100644
--- a/http.c
+++ b/http.c
@@ -749,15 +749,16 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset)
 			sensitive_header++;
 
 		cookie = sensitive_header;
 
 		while (cookie) {
-			char *equals;
-			char *semicolon = strstr(cookie, "; ");
-			if (semicolon)
-				*semicolon = 0;
-			equals = strchrnul(cookie, '=');
+			const char *equals;
+			const char *semicolon = strstr(cookie, "; ");
+
+			equals = semicolon ?
+				 memchr(cookie, '=', semicolon - cookie) :
+				 strchr(cookie, '=');
 			if (!equals) {
 				/* invalid cookie, just append and continue */
 				strbuf_addstr(&redacted_header, cookie);
 				continue;
 			}

Though note there is another bug lurking in this code! If we hit the
"!equals" case, we will continue the loop without advancing "cookie" at
all, and loop forever. But in the current version of the function, that
is dead code, because strchrnul() will never return NULL (you get either
the matched char or the end-of-string). Probably the "continue" should
be a "break", though perhaps we could keep parsing past the next
semicolon.

Not necessarily a problem we need to solve, but as I've written it
above, the dead code becomes live. So I wanted to give fair warning. ;)

-Peff

Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Toon Claes]

Jeff King <peff@peff.net> writes:

>> > Looking at strchr()'s declaration in string.h, which is defined like:
>> >
>> >   #  define strchr(S, C)                                          \
>> >     __glibc_const_generic (S, const char *, strchr (S, C))
>> >
>> > I think the answer is probably "yes". But it also doesn't quite solve
>> > our problem. That would give us type-checking of callers of our
>> > function, but we still have to convince the compiler not to complain
>> > about its implementation. For that we'd need to either cast away const
>> > manually, I guess.
>> 
>> That macro depends on Generic selections from C11 [1]. I wasn't sure if
>> Git would like that, given it is conservative with other C features.
>
> We definitely can't rely on it everywhere. But if there is a solution
> that is conditionally compiled, and can kick in only when these extra
> warnings also kick in, that would be OK. Assuming the result is not too
> painful to look at, of course.

So the Git project would be okay to conditionally compile with Generic
selections if the compiler supports it? Seems to me this is the easiest
way forward to silence the errors for users who see these warnings (that
includes me).

> Probably the best path forward for most spots is just fixing the code to
> make it more obvious about its use of const.

Yeah, that's in any case a good idea. I think using Generic selections
should make it easier to locate those.

This is what I've done in an experiment in git-compat-util.h:

    #ifndef find_last_dir_sep
    static inline char *git_find_last_dir_sep_nonconst(char *path)
    {
    	return strrchr(path, '/');
    }
    static inline const char *git_find_last_dir_sep_const(const char *path)
    {
    	return strrchr(path, '/');
    }
    #if __STDC_VERSION__ >= 201112L || (defined(__GNUC__) && __GNUC__ >= 5)
    #define find_last_dir_sep(path) \
    	_Generic((path), \
    		const char *: git_find_last_dir_sep_const((const char *)(path)), \
    		default:      git_find_last_dir_sep_nonconst((char *)(path)))
    #else
    #define find_last_dir_sep(path) git_find_last_dir_sep_nonconst((char *)(path))
    #endif
    #endif

This leads to 28 places where the warning happens:

    bloom.c:515:52: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/config.c:855:22: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/receive-pack.c:1045:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/receive-pack.c:1075:27: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/receive-pack.c:1098:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:278:22: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:339:21: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:343:28: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:347:28: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    convert.c:1189:24: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    convert.c:1212:32: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    convert.c:1223:29: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    dir.c:3526:15: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    http-push.c:1775:44: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    http.c:755:43: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    pager.c:121:28: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    pseudo-merge.c:647:16: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    range-diff.c:109:50: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    refs/files-backend.c:2199:25: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2135:24: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2179:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2188:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2194:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    run-command.c:608:32: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    send-pack.c:184:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    send-pack.c:215:27: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    send-pack.c:240:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    transport-helper.c:803:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

I did not look into any of them, but I think you (Collin) have sent out
patches for various of these? But they _should_ managable to address?

-- 
Cheers,
Toon

Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Jeff King]

On Thu, Mar 19, 2026 at 12:06:43PM +0100, Toon Claes wrote:

> Jeff King <peff@peff.net> writes:
> 
> >> > Looking at strchr()'s declaration in string.h, which is defined like:
> >> >
> >> >   #  define strchr(S, C)                                          \
> >> >     __glibc_const_generic (S, const char *, strchr (S, C))
> >> >
> >> > I think the answer is probably "yes". But it also doesn't quite solve
> >> > our problem. That would give us type-checking of callers of our
> >> > function, but we still have to convince the compiler not to complain
> >> > about its implementation. For that we'd need to either cast away const
> >> > manually, I guess.
> >> 
> >> That macro depends on Generic selections from C11 [1]. I wasn't sure if
> >> Git would like that, given it is conservative with other C features.
> >
> > We definitely can't rely on it everywhere. But if there is a solution
> > that is conditionally compiled, and can kick in only when these extra
> > warnings also kick in, that would be OK. Assuming the result is not too
> > painful to look at, of course.
> 
> So the Git project would be okay to conditionally compile with Generic
> selections if the compiler supports it? Seems to me this is the easiest
> way forward to silence the errors for users who see these warnings (that
> includes me).

Yes, though I think just turning it into a macro is enough to silence
this particular case (because macros don't have types, and so the
compiler sees the original types passed to strchr). And as you noted,
there are a ton of other cases that have to be looked at individually,
which I think is the real blocker.

> I did not look into any of them, but I think you (Collin) have sent out
> patches for various of these? But they _should_ managable to address?

I have quick-and-dirty fixes for these at:

  https://github.com/peff/git jk/hacky-strchr-fixes

I haven't been cleaning them up and sending in patches because I didn't
want to duplicate work Collin was doing. But Collin, let us know if we
can contribute. Dealing with the warnings is an occasional hassle during
other work.

If you're using gcc, you can solve it by just adding
-Wno-discarded-qualifiers to your CFLAGS. But clang doesn't know about
that warning. Worse, if you sometimes compile with -std=c99 (which is
necessary to build versions of Git older than e8b3bcf491) then glibc's
preprocessor conditionals don't kick in correctly and you get:

  ./git-compat-util.h:344:9: warning: returning 'const char *' from a function with result type 'char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
    344 |         return strrchr(path, '/');
        |                ^~~~~~~~~~~~~~~~~~
  /usr/include/string.h:296:3: note: expanded from macro 'strrchr'
    296 |   __glibc_const_generic (S, const char *, strrchr (S, C))
        |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/include/x86_64-linux-gnu/sys/cdefs.h:838:3: note: expanded from macro '__glibc_const_generic'
    838 |   _Generic (0 ? (PTR) : (void *) 1,                     \
        |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    839 |             const void *: (CTYPE) (CALL),               \
        |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    840 |             default: CALL)
        |             ~~~~~~~~~~~~~~

Yuck. That is not even specific to Git, and is hopefully something that
glibc and clang folks might figure out.

-Peff

Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Collin Funk]

Jeff King <peff@peff.net> writes:

> On Thu, Mar 19, 2026 at 12:06:43PM +0100, Toon Claes wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> >> > Looking at strchr()'s declaration in string.h, which is defined like:
>> >> >
>> >> >   #  define strchr(S, C)                                          \
>> >> >     __glibc_const_generic (S, const char *, strchr (S, C))
>> >> >
>> >> > I think the answer is probably "yes". But it also doesn't quite solve
>> >> > our problem. That would give us type-checking of callers of our
>> >> > function, but we still have to convince the compiler not to complain
>> >> > about its implementation. For that we'd need to either cast away const
>> >> > manually, I guess.
>> >> 
>> >> That macro depends on Generic selections from C11 [1]. I wasn't sure if
>> >> Git would like that, given it is conservative with other C features.
>> >
>> > We definitely can't rely on it everywhere. But if there is a solution
>> > that is conditionally compiled, and can kick in only when these extra
>> > warnings also kick in, that would be OK. Assuming the result is not too
>> > painful to look at, of course.
>> 
>> So the Git project would be okay to conditionally compile with Generic
>> selections if the compiler supports it? Seems to me this is the easiest
>> way forward to silence the errors for users who see these warnings (that
>> includes me).
>
> Yes, though I think just turning it into a macro is enough to silence
> this particular case (because macros don't have types, and so the
> compiler sees the original types passed to strchr). And as you noted,
> there are a ton of other cases that have to be looked at individually,
> which I think is the real blocker.
>
>> I did not look into any of them, but I think you (Collin) have sent out
>> patches for various of these? But they _should_ managable to address?
>
> I have quick-and-dirty fixes for these at:
>
>   https://github.com/peff/git jk/hacky-strchr-fixes
>
> I haven't been cleaning them up and sending in patches because I didn't
> want to duplicate work Collin was doing. But Collin, let us know if we
> can contribute. Dealing with the warnings is an occasional hassle during
> other work.

Please feel free to work on it. I fixed two more simple cases which are
in Junio's "next" branch. I also have one patch that that I sent on list
but is pending review [1].

I had planned to do more, but got busy working on other things. Sorry
about that.

> If you're using gcc, you can solve it by just adding
> -Wno-discarded-qualifiers to your CFLAGS. But clang doesn't know about
> that warning. Worse, if you sometimes compile with -std=c99 (which is
> necessary to build versions of Git older than e8b3bcf491) then glibc's
> preprocessor conditionals don't kick in correctly and you get:
>
>   ./git-compat-util.h:344:9: warning: returning 'const char *' from a function with result type 'char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
>     344 |         return strrchr(path, '/');
>         |                ^~~~~~~~~~~~~~~~~~
>   /usr/include/string.h:296:3: note: expanded from macro 'strrchr'
>     296 |   __glibc_const_generic (S, const char *, strrchr (S, C))
>         |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /usr/include/x86_64-linux-gnu/sys/cdefs.h:838:3: note: expanded from macro '__glibc_const_generic'
>     838 |   _Generic (0 ? (PTR) : (void *) 1,                     \
>         |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     839 |             const void *: (CTYPE) (CALL),               \
>         |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     840 |             default: CALL)
>         |             ~~~~~~~~~~~~~~
>
> Yuck. That is not even specific to Git, and is hopefully something that
> glibc and clang folks might figure out.

I think something not immediately obvious is happening here. Hopefully
my explanation helps you.

Here is how the macro is defined in string.h:

    # if __GLIBC_USE (ISOC23) && defined __glibc_const_generic && !defined _LIBC
    #  define strrchr(S, C)						\
      __glibc_const_generic (S, const char *, strrchr (S, C))
    # endif

There are two cases that __GLIBC_USE (ISOC23) evaluates to true. The
first is if you use '-std=c23'/'-std=gnu23' (or have a compiler that
uses that version by default). The second, which I expect is happening
in your case, is if _GNU_SOURCE is defined. That flag causes GNU
extensions to be declared along with the latest standards.

Here is an example:

    $ cat main.c
    #include <string.h>
    int
    main (void)
    {
      const char *s = "hello";
      char *p = strchr (s, 'h');
      return 0;
    }
    $ gcc -std=c99 main.c
    $ gcc -std=c99 -D_GNU_SOURCE main.c
    main.c: In function ‘main’:
    main.c:6:13: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
        6 |   char *p = strchr (s, 'h');
          |             ^~~~~~

I can modify the file like this:

    $ cat main.c 
    #include <sys/cdefs.h>
    #undef __glibc_const_generic
    #include <string.h>
    int
    main (void)
    {
      const char *s = "hello";
      char *p = strchr (s, 'h');
      return 0;
    }
    $ gcc -std=c99 main.c
    $ gcc -std=c99 -D_GNU_SOURCE main.c

For context, this change has caused myself and the other Gnulib
maintainers some trouble as well. It broke the builds of many GNU
packages that vendor Gnulib (coreutils, m4, hello, and the list goes
on). Patching things to include sys/cdefs.h and undefining
__glibc_const_generic was a workaround we considered, but luckily it
didn't take too much time to make new releases for the affected packages.

Collin

[1] https://lore.kernel.org/git/d3447c19d83c37bf2db84ae0bf75801ef7a36cea.1773115420.git.collin.funk1@gmail.com/