Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer
- From
Jeff King <peff@peff.net>
- Date
- Mar 20, 2026, 04:39 UTC
- Message-ID
- <20260320043943.GB18125@coredump.intra.peff.net>
- In-Reply-To
- <87a4w42i4c.fsf@iotcl.com>
On Thu, Mar 19, 2026 at 12:06:43PM +0100, Toon Claes wrote:
Show 25 quoted lines
> 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