Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-14 Thread Andreas Krebbel 
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 
> ---
>  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-


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-14 Thread Andreas Krebbel 
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 
> ---
>  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 
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 

+   [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.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-13 Thread Paul Eggert 

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.)


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-13 Thread Eric Blake 
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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-13 Thread Eric Blake 
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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-13 Thread Paul Eggert 

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.


From df544bd79a83880cc3287d43c5be1719e1ca2ccf Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 13 Oct 2016 11:16:40 -0700
Subject: [PATCH] stdint: port SIZE_MAX to glibc s390

Problem reported by Eric Blake in:
http://lists.gnu.org/archive/html/bug-gnulib/2016-10/msg00031.html
* doc/posix-headers/stdint.texi (stdint.h): Document the fix.
* m4/stdint.m4 (gl_STDINT_H): Check that SIZE_MAX has the
correct type, if possible.
---
 ChangeLog |  9 +
 doc/posix-headers/stdint.texi |  8 
 m4/stdint.m4  | 11 ++-
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 070e498..89ba80f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2016-10-13  Paul Eggert  
+
+	stdint: port SIZE_MAX to glibc s390
+	Problem reported by Eric Blake in:
+	http://lists.gnu.org/archive/html/bug-gnulib/2016-10/msg00031.html
+	* doc/posix-headers/stdint.texi (stdint.h): Document the fix.
+	* m4/stdint.m4 (gl_STDINT_H): Check that SIZE_MAX has the
+	correct type, if possible.
+
 2016-10-13  Daniel Richard G.  
 
 	getprogname: port to IBM z/OS
diff --git a/doc/posix-headers/stdint.texi b/doc/posix-headers/stdint.texi
index 2cc083c..d5ca3c2 100644
--- a/doc/posix-headers/stdint.texi
+++ b/doc/posix-headers/stdint.texi
@@ -34,6 +34,10 @@ constant macros such as @code{INTMAX_C}, and one must define
 @code{__STDC_LIMIT_MACROS} to make visible the definitions of limit
 macros such as @code{INTMAX_MAX}.
 @item
+The macro @code{SIZE_MAX} has the wrong type,
+albeit with the correct value:
+32-bit glibc 2.24 (on s390 architecture), Mac OS X 10.7.
+@item
 Macros like @code{INTMAX_WIDTH} are not defined on some platforms:
 glibc 2.24, many others.
 @end itemize
@@ -48,10 +52,6 @@ so public header files should avoid these types.
 @item
 Macros are used instead of typedefs.
 @item
-The macro @code{SIZE_MAX} has the wrong type,
-albeit with the correct value:
-32-bit glibc 2.24 (on s390 architecture), Mac OS X 10.7.
-@item
 Some C preprocessors mishandle constants that do not fit in @code{long int}.
 For example, as of 2007, Sun C mishandles @code{#if LLONG_MIN < 0} on
 a platform with 32-bit @code{long int} and 64-bit @code{long long int}.
diff --git a/m4/stdint.m4 b/m4/stdint.m4
index fa6f103..05b6ab7 100644
--- a/m4/stdint.m4
+++ b/m4/stdint.m4
@@ -1,4 +1,4 @@
-# stdint.m4 serial 47
+# stdint.m4 serial 48
 dnl Copyright (C) 2001-2016 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -154,6 +154,15 @@ uintptr_t h = UINTPTR_MAX;
 intmax_t i = INTMAX_MAX;
 uintmax_t j = UINTMAX_MAX;
 
+/* Check that SIZE_MAX has the correct type, if possible.  */
+#if 201112 <= __STDC_VERSION__
+int k = _Generic (SIZE_MAX, size_t: 0);
+#elif (2 <= __GNUC__ || defined __IBM__TYPEOF__ \
+   || (0x5110 <= __SUNPRO_C && !__STDC__))
+extern size_t k;
+extern __typeof__ (SIZE_MAX) k;
+#endif
+
 #include  /* for CHAR_BIT */
 #define TYPE_MINIMUM(t) \
   ((t) ((t) 0 < (t) -1 ? (t) 0 : ~ TYPE_MAXIMUM (t)))
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-13 Thread Eric Blake 
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 
> ---
>  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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-13 Thread Jiri Denemark 
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(, 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix build on s390

2016-10-07 Thread Christian Borntraeger 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] util: Fix build on s390

2016-10-07 Thread Jiri Denemark 
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 
---
 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list