Subject: Re: [PATCH 12/12] refs/files-backend: drop const to fix strchr() warning

From: Junio C Hamano <[email-masked]>
Date: Wed, 01 Apr 2026 15:22:48 -0700

Patrick Steinhardt <[email-masked]> writes:

> On Tue, Mar 31, 2026 at 07:53:41PM -0400, Jeff King wrote:
>> 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(). Note that unlike
>> skip_prefix(), the function is not inline, so we can't just wrap it
>> using the same name (otherwise the actual definition would expand the
>> macro, which breaks compilation). So we rename the actual function with
>> an "_impl" suffix, and callers will all use the macro.
>
> Fair. In fact, I was a bit torn with the other commits whether it's nice
> to reuse the same name. I guess what it buys us is that you cannot
> accidentally call the wrong function without the guardrails. Even though
> that's quite unlikely with the `_impl` suffix.

I share the sentiment.  If I were deciding the design, I'd even go
forcing the _impl suffix to everything, including the inline ones,
as I found the earlier "strip_prefix()" example already confusing.