On Wed, Nov 26, 2025 at 09:15:58AM +0100, Michal Prívozník wrote:
> 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.

I'm actually seeing the opposite with a simple test program
GCC will warn without needing any args:

$ cat const.c
#include <string.h>
#include <stdbool.h>

bool foo(const char *wizz);

bool foo(const char *wizz)
{
  char *bang = strchr(wizz, '/');
  return bang != NULL;
}
$ gcc -c -o const const.c
const.c: In function 'foo':
const.c:8:16: warning: initialization discards 'const' qualifier from pointer 
target type [-Wdiscarded-qualifiers]
    8 |   char *bang = strchr(wizz, '/');
      |                ^~~~~~

clang meanwhile is completely quiet

$ clang -c -o const const.c
$ clang -Wincompatible-pointer-types-discards-qualifiers -Wall -c -o const 
const.c
$ clang -Wincompatible-pointer-types-discards-qualifiers -Wall -std=gnu99  -c 
-o const const.c
$ clang -Wincompatible-pointer-types-discards-qualifiers -Wall -std=gnu23  -c 
-o const const.c
const.c:8:9: warning: initializing 'char *' with an expression of type 'const 
char *' discards qualifiers
      [-Wincompatible-pointer-types-discards-qualifiers]
    8 |   char *bang = strchr(wizz, '/');
      |         ^      ~~~~~~~~~~~~~~~~~
1 warning generated.


which makes no sense, as libvirt builds with -std=gnu99

# rpm -q gcc glibc-devel clang
gcc-15.2.1-4.fc44.x86_64
glibc-devel-2.42.9000-13.fc44.x86_64
clang-21.1.6-1.fc44.x86_64


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Reply via email to