public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff King <peff@peff•net>
To: Toon Claes <toon@iotcl•com>
Cc: Collin Funk <collin.funk1@gmail•com>,
	git@vger•kernel.org, Junio C Hamano <gitster@pobox•com>,
	Johannes Schindelin <Johannes.Schindelin@gmx•de>,
	Phillip Wood <phillip.wood@dunelm•org.uk>,
	Matthew John Cheetham <mjcheetham@outlook•com>,
	Victoria Dye <vdye@github•com>, Derrick Stolee <stolee@gmail•com>,
	Justin Tobler <jltobler@gmail•com>
Subject: Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer
Date: Fri, 20 Mar 2026 00:39:43 -0400	[thread overview]
Message-ID: <20260320043943.GB18125@coredump.intra.peff.net> (raw)
In-Reply-To: <87a4w42i4c.fsf@iotcl.com>

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

  reply	other threads:[~2026-03-20  4:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-03  5:19 [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer Collin Funk
2026-02-03  6:25 ` Jeff King
2026-02-04  3:15   ` Collin Funk
2026-02-04  5:32     ` Jeff King
2026-03-19 11:06       ` Toon Claes
2026-03-20  4:39         ` Jeff King [this message]
2026-03-20  5:20           ` Collin Funk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260320043943.GB18125@coredump.intra.peff.net \
    --to=peff@peff$(echo .)net \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=collin.funk1@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=jltobler@gmail$(echo .)com \
    --cc=mjcheetham@outlook$(echo .)com \
    --cc=phillip.wood@dunelm$(echo .)org.uk \
    --cc=stolee@gmail$(echo .)com \
    --cc=toon@iotcl$(echo .)com \
    --cc=vdye@github$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox