Re: [PULL 0/1] Quick fix patches

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Aug 30, 2023 at 08:31:15AM -0600, Warner Losh wrote:
> On Wed, Aug 30, 2023, 7:26 AM Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 30/08/2023 15.16, Stefan Hajnoczi wrote:
> > > Hi,
> > > The patch introduces the following build failure:
> > >
> > > cc -m64 -mcx16 -Isubprojects/libvhost-user/libvhost-user.a.p
> > > -Isubprojects/libvhost-user -I../subprojects/libvhost-user
> > > -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g
> > > -Wsign-compare -Wdeclaration-after-statement -Wstrict-aliasing
> > > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
> > > -fno-common -fwrapv -fPIE -pthread -D_GNU_SOURCE -MD -MQ
> > > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -MF
> > > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o.d -o
> > > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -c
> > > ../subprojects/libvhost-user/libvhost-user.c
> > > In file included from ../subprojects/libvhost-user/include/atomic.h:18,
> > > from ../subprojects/libvhost-user/libvhost-user.c:53:
> > > ../subprojects/libvhost-user/include/compiler.h:38:40: error: missing
> > > binary operator before token "("
> > > 38 | #if defined(__clang__) &&
> > __has_warning("-Waddress-of-packed-member")
> > > | ^
> > >
> > > https://gitlab.com/qemu-project/qemu/-/jobs/4981576093
> >
> > IIRC older versions of GCC do not have __has_warning() yet, so if you want
> > to use this in compiler.h, you have to do it below the line in compiler.h
> > that adds this:
> >
> > #ifndef __has_warning
> > #define __has_warning(x) 0 /* compatibility with non-clang compilers */
> > #endif
> >
> 
> This already works for linux-user. If there are gcc versions that break,
> our current CI jobs don't show it. Why add complexity for unsupported gcc
> versions? And how do I know I got it right?

IIUC, /no/ GCC version has __has_warning. The no-op stub we have works
because we merely need the preprocessor to be able to parse the
expression

  #if defined(__clang__) && __has_warning("....")

it'll never actually evaluate the __has_warning clause under GCC
because the defined(__clang__) will be false.

> I'm really starting to think the feedback 'move it to compilers.h' should
> have just been ignored... it's turning into a lot of my time to correct
> that I don't have when I'm also out of CI minutes to test with.

FWIW, if you have a Linux VM with docker/podman present it also possible
to run the CI environments locally with 'make docker-help' has more info

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 :|