1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] maint: update to latest gnulib

Daniel P. Berrange posted 1 patch 8 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170614102044.672-1-berrange@redhat.com
.gnulib | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt] [PATCH] maint: update to latest gnulib
Posted by Daniel P. Berrange 8 years, 6 months ago 
This fixes an incompatibility with glibc 2.25.90

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---

Pushed as a broken build fix to get CI back online

 .gnulib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gnulib b/.gnulib
index da830b5..ce4ee4c 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit da830b5146cb553ac2a4bcfe76caeb57bda24cc3
+Subproject commit ce4ee4cbb596a9d7de2786cf8c48cf62a4edede7
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: update to latest gnulib
Posted by Martin Kletzander 8 years, 6 months ago 
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@redhat.com>
>---
>
>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?

> .gnulib | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/.gnulib b/.gnulib
>index da830b5..ce4ee4c 160000
>--- a/.gnulib
>+++ b/.gnulib
>@@ -1 +1 @@
>-Subproject commit da830b5146cb553ac2a4bcfe76caeb57bda24cc3
>+Subproject commit ce4ee4cbb596a9d7de2786cf8c48cf62a4edede7
>--
>2.9.3
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: update to latest gnulib
Posted by Daniel P. Berrange 8 years, 6 months ago 
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@redhat.com>
> > ---
> > 
> > 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. 




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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: update to latest gnulib
Posted by Martin Kletzander 8 years, 6 months ago 
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@redhat.com>
>> > ---
>> >
>> > 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 ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>
>
>
>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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: update to latest gnulib
Posted by Daniel P. Berrange 8 years, 6 months ago 
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@redhat.com>
> > > > ---
> > > >
> > > > 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: update to latest gnulib
Posted by Martin Kletzander 8 years, 6 months ago 
On Wed, Jun 14, 2017 at 03:35:39PM +0100, Daniel P. Berrange wrote:
>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@redhat.com>
>> > > > ---
>> > > >
>> > > > 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.
>

I wanted to do that and started rewriting it, but I found out we use
static buffers in lot of places and lot of them then use snprintf or
similar.  In some cases it doesn't even make much of a sense, e.g.:

  snprintf(buf, 3, "%d", var)

even this can fail.  I get the reason for the warning, but I don't
really agree that it's something that is necessary to avoid, so I'm
inclining to ignoring that warning as well.

>
>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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: update to latest gnulib
Posted by Daniel P. Berrange 8 years, 6 months ago 
On Thu, Jun 15, 2017 at 05:39:00PM +0200, Martin Kletzander wrote:
> On Wed, Jun 14, 2017 at 03:35:39PM +0100, Daniel P. Berrange wrote:
> > 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@redhat.com>
> > > > > > ---
> > > > > >
> > > > > > 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.
> > 
> 
> I wanted to do that and started rewriting it, but I found out we use
> static buffers in lot of places and lot of them then use snprintf or
> similar.  In some cases it doesn't even make much of a sense, e.g.:
> 
>  snprintf(buf, 3, "%d", var)
> 
> even this can fail.  I get the reason for the warning, but I don't
> really agree that it's something that is necessary to avoid, so I'm
> inclining to ignoring that warning as well.

In that kind of scenario we should be using  INT_BUFSIZE_BOUND(var)
to declare the 'buf' array, rather than hardcoding a fixed size
based on assumptions about likely max value of var.

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.