On 11/25/25 16:03, Daniel P. Berrangé wrote:
> On Tue, Nov 25, 2025 at 03:51:00PM +0100, Michal Prívozník wrote:
>> On 11/25/25 15:10, Daniel P. Berrangé via Devel wrote:
>>> On Tue, Nov 25, 2025 at 02:54:20PM +0100, Ján Tomko via Devel wrote:
>>>> On a Tuesday in 2025, Peter Krempa via Devel wrote:
>>>>> From: Peter Krempa <[email protected]>
>>>>>
>>>>> 'char *tmp' is assigned from calling 'strrchr' on a 'const char *'. New
>>>>> clang in fedora doesn't like it. Make 'tmp' const.
>>>>>
>>>>> Signed-off-by: Peter Krempa <[email protected]>
>>>>> ---
>>>>>
>>>>> https://gitlab.com/MichalPrivoznik/libvirt/-/jobs/12208300313
>>>>>
>>>>
>>>> I was hoping the link would show a fixed pipeline :)
>>>
>>> I'm rather curious how clang decides to trigger that warning given
>>> the libc header file declares the return value non-const
>>>
>>> extern char *strchr (const char *__s, int __c)
>>> __THROW __attribute_pure__ __nonnull ((1));
>>>
>>> It seems like clang has special-cased strchr/strrchr to enforce
>>> the const return for const input.
>>
>> Well, it also triggers in places like:
>>
>> ../src/rpc/virnetsshsession.c:223:18: error: assigning to 'char *' from
>> 'const char *' discards qualifiers
>> [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
>> 223 | if ((tmp = strrchr(askcred[i].prompt, ':')))
>> | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>>
>> And just to give you context around the line:
>>
>> if ((tmp = strrchr(askcred[i].prompt, ':')))
>> *tmp = '\0';
>>
>> So I'd rather this patch is NOT merged and CLang is fixed instead.
>
> I'm on the fence about whether clang is "broken" or not.
>
> It is in response to -Wincompatible-pointer-types-discards-qualifiers.
> Since warnings aren't standardized, the compiler author has free
> choice over semantics.
>
> In C++ the string.h header actually provides 2 overloaded definitions
>
> /* Find the first occurrence of C in S. */
> #ifdef __CORRECT_ISO_CPP_STRING_H_PROTO
> extern "C++"
> {
> extern char *strchr (char *__s, int __c)
> __THROW __asm ("strchr") __attribute_pure__ __nonnull ((1));
> extern const char *strchr (const char *__s, int __c)
> __THROW __asm ("strchr") __attribute_pure__ __nonnull ((1));
> ...snip...
>
>
> IMHO it is not unreasonable for CLang to want to bring an equivalent
> level of const-correctness to C through warning flags.
>
> I figure we're probably got a choice of fixing all the non-const correct
> usage across the codebase, or turning off this warning with clang builds.
Alright, after some investigation, I think I know what's happening. In
fact, it's glibc that changed [1]. The strrchr() function is now
declared in string.h as the following:
extern char *strrchr (const char *__s, int __c)
__THROW __attribute_pure__ __nonnull ((1));
# if __GLIBC_USE (ISOC23) && defined __glibc_const_generic && !defined _LIBC
# define strrchr(S, C) \
__glibc_const_generic (S, const char *, strrchr (S, C))
# endif
where __glibc_const_generic macro is then declared as:
# define __glibc_const_generic(PTR, CTYPE, CALL) \
_Generic (0 ? (PTR) : (void *) 1, \
const void *: (CTYPE) (CALL), \
default: CALL)
Long story short, the call to strrchr() from virCgroupSetValueRaw()
expands to:
tmp = _Generic (0 ? (path) : (void *) 1, const void *: (const char *) (strrchr
(path, '/')), default: strrchr (path, '/'))
Which indeed means that whenever strrchr() is given a (const void *) arg
its retval is typecasted to (const char *). And it's the same story with
strchr() and others. So I agree, this is something we ought to fix. What
I don't quite understand is why we're not hitting the same warning with
GCC since its preprocessor expands the call to the very same line.
Michal
1:
https://sourceware.org/git/?p=glibc.git;a=commit;h=cd748a63ab1a7ae846175c532a3daab341c62690
