[libvirt] [PATCH] util: Fix build on s390
GCC on s390 complains util/virconf.c: In function 'virConfGetValueSizeT': util/virconf.c:1220:9: error: format '%zu' expects argument of type 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=] Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virconf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virconf.c b/src/util/virconf.c index 3e49f41..1372389 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -1219,7 +1219,7 @@ int virConfGetValueSizeT(virConfPtr conf, if (((unsigned long long)cval->l) > SIZE_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: value for '%s' parameter must be in range 0:%zu"), - conf->filename, setting, SIZE_MAX); + conf->filename, setting, (size_t) SIZE_MAX); return -1; } #endif -- 2.10.1
On 10/07/2016 12:43 PM, Jiri Denemark wrote:
GCC on s390 complains
util/virconf.c: In function 'virConfGetValueSizeT': util/virconf.c:1220:9: error: format '%zu' expects argument of type 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]
Interesting, we have never seen this. is this 31bit (s390) or 64bit(s390x)? What gcc? Can you maybe provide the virconf.i file to see how the SIZE_MAX define was resolved? Christian
Hi. On Fri, Oct 07, 2016 at 13:36:21 +0200, Christian Borntraeger wrote:
On 10/07/2016 12:43 PM, Jiri Denemark wrote:
GCC on s390 complains
util/virconf.c: In function 'virConfGetValueSizeT': util/virconf.c:1220:9: error: format '%zu' expects argument of type 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]
Interesting, we have never seen this. is this 31bit (s390) or 64bit(s390x)? What gcc?
I finally got the time to get the additional info for you (which was not exactly easy as it happened on a builder system): $ gcc --version gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-11) Copyright (C) 2015 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. $ arch s390 However, the host kernel is 2.6.32-573.22.1.el6.s390x. The build is done on an s390x RHEL-6 host, but in a mocked s390 RHEL-7 environment :-)
Can you maybe provide the virconf.i file to see how the SIZE_MAX define was resolved?
# 1199 "util/virconf.c" int virConfGetValueSizeT(virConfPtr conf, const char *setting, size_t *value) { virConfValuePtr cval = virConfGetValue(conf, setting); virLogMessage(&virLogSelf, VIR_LOG_DEBUG, "util/virconf.c", 1206, __func__, ((void *)0), "Get value size_t %p %d", cval, cval ? cval->type : VIR_CONF_NONE); if (!cval) return 0; if (cval->type != VIR_CONF_ULLONG) { virReportErrorHelper(VIR_FROM_CONF, VIR_ERR_INTERNAL_ERROR, "util/virconf.c" # 1212 "util/virconf.c" , __FUNCTION__, 1214 # 1212 "util/virconf.c" , dcgettext ("libvirt", "%s: expected an unsigned integer for '%s' parameter", 5), conf->filename, setting) ; return -1; } if (((unsigned long long)cval->l) > (4294967295U)) { virReportErrorHelper(VIR_FROM_CONF, VIR_ERR_INTERNAL_ERROR, "util/virconf.c" # 1220 "util/virconf.c" , __FUNCTION__, 1222 # 1220 "util/virconf.c" , dcgettext ("libvirt", "%s: value for '%s' parameter must be in range 0:%zu", 5), conf->filename, setting, (4294967295U)) ; return -1; } *value = (size_t)cval->l; return 1; } Which means SIZE_MAX is 4294967295U. Complete virconf.i can be seen at http://people.redhat.com/~jdenemar/libvirt/virconf.i Jirka
On 10/07/2016 05:43 AM, Jiri Denemark wrote:
GCC on s390 complains
util/virconf.c: In function 'virConfGetValueSizeT': util/virconf.c:1220:9: error: format '%zu' expects argument of type 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]
That's a bug in s390's system headers. Gnulib should be taught to work around it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virconf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c index 3e49f41..1372389 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -1219,7 +1219,7 @@ int virConfGetValueSizeT(virConfPtr conf, if (((unsigned long long)cval->l) > SIZE_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: value for '%s' parameter must be in range 0:%zu"), - conf->filename, setting, SIZE_MAX); + conf->filename, setting, (size_t) SIZE_MAX); return -1; } #endif
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/13/2016 06:36 AM, Eric Blake wrote:
That's a bug in s390's system headers. Gnulib should be taught to work around it.
Although this bug was reported fixed in glibc 2.20; see https://sourceware.org/bugzilla/show_bug.cgi?id=16712 the Gnulib manual says the bug is still in glibc 2.24; see gnulib/doc/posix-headers/stdint.texi. Is the Gnulib manual correct? If so, why didn't the earlier glibc patch fix the bug? Anyway, I installed into Gnulib the attached patch, which I hope works around the bug. I can't easily test this, so please give it a try. And if you can think of a way to test whether SIZE_MAX has the correct type on pre-C11 compilers that lack __typeof__, please let me know.
On 10/13/2016 01:23 PM, Paul Eggert wrote:
On 10/13/2016 06:36 AM, Eric Blake wrote:
That's a bug in s390's system headers. Gnulib should be taught to work around it.
Although this bug was reported fixed in glibc 2.20; see
https://sourceware.org/bugzilla/show_bug.cgi?id=16712
the Gnulib manual says the bug is still in glibc 2.24; see gnulib/doc/posix-headers/stdint.texi. Is the Gnulib manual correct? If so, why didn't the earlier glibc patch fix the bug?
Anyway, I installed into Gnulib the attached patch, which I hope works around the bug. I can't easily test this, so please give it a try. And if you can think of a way to test whether SIZE_MAX has the correct type on pre-C11 compilers that lack __typeof__, please let me know.
With gcc, -Werror=format flags a mismatch on printf("%zu",SIZE_MAX). With C++, you can check whether a correct overloaded function is called. But I don't have any off-hand way of testing it without using compiler extensions or a different language. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/13/2016 01:50 PM, Eric Blake wrote:
On 10/13/2016 01:23 PM, Paul Eggert wrote:
On 10/13/2016 06:36 AM, Eric Blake wrote:
That's a bug in s390's system headers. Gnulib should be taught to work around it.
Although this bug was reported fixed in glibc 2.20; see
https://sourceware.org/bugzilla/show_bug.cgi?id=16712
the Gnulib manual says the bug is still in glibc 2.24; see gnulib/doc/posix-headers/stdint.texi. Is the Gnulib manual correct? If so, why didn't the earlier glibc patch fix the bug?
Anyway, I installed into Gnulib the attached patch, which I hope works around the bug. I can't easily test this, so please give it a try. And if you can think of a way to test whether SIZE_MAX has the correct type on pre-C11 compilers that lack __typeof__, please let me know.
With gcc, -Werror=format flags a mismatch on printf("%zu",SIZE_MAX).
With C++, you can check whether a correct overloaded function is called.
But I don't have any off-hand way of testing it without using compiler extensions or a different language.
That said, I think a configure test based on -Werror=format would be sufficient - after all, our objective is only to fix the problem if it can be observed. If a platform is using pre-C11 gcc and the problem is observable, we want it fixed; if they are using some other compiler, then chances are -Wformat does not work, therefore the problem cannot be portably observed so there is nothing to work around, whether or not the problem is present. That's the beauty of testing features rather than compiler versions :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/13/2016 12:14 PM, Eric Blake wrote:
I think a configure test based on -Werror=format would be sufficient
What we have now should work for GCC 2.0 and later, whereas -Werror=format is a much-newer GCC feature and so would be less portable.
if they are using some other compiler, then chances are -Wformat does not work, therefore the problem cannot be portably observed so there is nothing to work around
I am worried that other compilers might warn about it even without -Werror=format. (I'm not worried about GCC, as nobody uses GCC 1 any more.)
On 10/07/2016 12:43 PM, Jiri Denemark wrote:
GCC on s390 complains
util/virconf.c: In function 'virConfGetValueSizeT': util/virconf.c:1220:9: error: format '%zu' expects argument of type 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virconf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c index 3e49f41..1372389 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -1219,7 +1219,7 @@ int virConfGetValueSizeT(virConfPtr conf, if (((unsigned long long)cval->l) > SIZE_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: value for '%s' parameter must be in range 0:%zu"), - conf->filename, setting, SIZE_MAX); + conf->filename, setting, (size_t) SIZE_MAX); return -1; } #endif
S/390 32 bit uses an "unsigned long" as size_t while every other 32 bit target uses "unsigned int". The __SIZE_MAX__ predefined macro generated by GCC itself handles this correctly by defining it as 4294967295UL. However, the stdint.h header defines it as 4294967295U. So in the end we probably have to fix the Glibc header. This workaround is ok I think. Directly using __SIZE_MAX__ instead of SIZE_MAX would not be portable. -Andreas-
On 10/07/2016 12:43 PM, Jiri Denemark wrote:
GCC on s390 complains
util/virconf.c: In function 'virConfGetValueSizeT': util/virconf.c:1220:9: error: format '%zu' expects argument of type 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virconf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c index 3e49f41..1372389 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -1219,7 +1219,7 @@ int virConfGetValueSizeT(virConfPtr conf, if (((unsigned long long)cval->l) > SIZE_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: value for '%s' parameter must be in range 0:%zu"), - conf->filename, setting, SIZE_MAX); + conf->filename, setting, (size_t) SIZE_MAX); return -1; } #endif
I've just noticed that a colleague fixed that already quite some time ago in Glibc. So please update: commit 26011b5cfa6a1a8d8005d65f11d97498444a4e95 Author: Stefan Liebler <stli@linux.vnet.ibm.com> Date: Mon Mar 24 16:46:51 2014 +0100 S390: Define SIZE_MAX as unsigned long (BZ #16712). diff --git a/ChangeLog b/ChangeLog index 4da1027..c0d13ab 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2014-03-24 Stefan Liebler <stli@linux.vnet.ibm.com> + [BZ #16712] + * sysdeps/s390/s390-32/bits/wordsize.h + (__WORDSIZE32_SIZE_ULONG): New define. + * sysdeps/s390/s390-64/bits/wordsize.h + (__WORDSIZE32_SIZE_ULONG): Likewise. + * sysdeps/generic/stdint.h (SIZE_MAX): + Define as UL if __WORDSIZE32_SIZE_ULONG.
participants (5)
-
Andreas Krebbel -
Christian Borntraeger -
Eric Blake -
Jiri Denemark -
Paul Eggert