On Wed, Jun 14, 2017 at 04:30:49PM +0200, Martin Kletzander wrote:
> On Wed, Jun 14, 2017 at 02:32:41PM +0100, Daniel P. Berrange wrote:
> > On Wed, Jun 14, 2017 at 03:27:25PM +0200, Martin Kletzander wrote:
> > > On Wed, Jun 14, 2017 at 11:20:44AM +0100, Daniel P. Berrange wrote:
> > > > This fixes an incompatibility with glibc 2.25.90
> > > >
> > > > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > > > ---
> > > >
> > > > Pushed as a broken build fix to get CI back online
> > > >
> > >
> > > After this update the build fails for me with gcc-7.1.0 with the
> > > following error:
> > >
> > > In file included from util/virobject.c:28:0:
> > > util/virobject.c: In function 'virClassNew':
> > > util/viratomic.h:176:46: error: this condition has identical branches [-Werror=duplicated-branches]
> > > (void)(0 ? *(atomic) ^ *(atomic) : 0); \
> > > ^
> > > util/virobject.c:144:20: note: in expansion of macro 'virAtomicIntInc'
> > > klass->magic = virAtomicIntInc(&magicCounter);
> > > ^~~~~~~~~~~~~~~
> > >
> > > Does that mean that gcc does optimize our prefetch trick away
> > > (considering I understood what that line is trying to do)? Or should we
> > > just turn the warning off for that header file?
> >
> > Yep, "-Wduplicated-branches" appears to be a new warning flag in gcc 7.1
> > which gnulib turns on. There's a similar hit with mingw
> >
> > ../../src/util/vircommand.c: In function 'virCommandWait':
> > ../../src/util/vircommand.c:2562:51: error: this condition has identical branches [-Werror=duplicated-branches]
> > *exitstatus = cmd->rawStatus ? status : WEXITSTATUS(status);
> > ^
> > cc1: all warnings being treated as errors
> >
> >
> > because WEXITSTATUS(x) expands to 'x' on Win32.
> >
> > We could use a pragma to turn off selectively, but I'm more
> > inclined to just disable this new warning flag.
> >
>
> Well, I'm not sure how that affects the line where we actually use it
> (with the atomic variables) or whether that line is not needed anymore
> (if that was a fix for older compilers or something similar). But I
> can send a patch for removing that warning. How about the other
> warning we get when we turn off the first one? I just found out. I
> think that could be turned off as well, either for some particular
> places or for the whole build:
>
> util/virtime.c: In function 'virTimeStringThenRaw':
> util/virtime.c:215:9: error: '%02d' directive output may be truncated writing between 2 and 11 bytes into a region of size between 5 and 21 [-Werror=format-truncation=]
> if (snprintf(buf, VIR_TIME_STRING_BUFLEN,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> "%4d-%02d-%02d %02d:%02d:%02d.%03d+0000",
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> fields.tm_year, fields.tm_mon, fields.tm_mday,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> fields.tm_hour, fields.tm_min, fields.tm_sec,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> (int) (when % 1000)) >= VIR_TIME_STRING_BUFLEN) {
> ~~~~~~~~~~~~~~~~~~~~
> util/virtime.c:215:9: note: using the range [-2147483648, 2147483647] for directive argument
> In file included from /usr/include/stdio.h:936:0,
> from ../gnulib/lib/stdio.h:43,
> from util/virtime.c:36:
> /usr/include/bits/stdio2.h:64:10: note: '__builtin___snprintf_chk' output between 29 and 89 bytes into a destination of size 29
> return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> __bos (__s), __fmt, __va_arg_pack ());
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I think we might just want to switch to asprintf here instead of trying to
optimize into a fixed stack allocated buffer.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
