Re: [Qemu-block] [Qemu-devel] [PATCH v5] build: Work around SIZE_MAX bug in OSX headers

[Programmingkid]

On Oct 11, 2016, at 2:12 PM, Eric Blake wrote:

> On 10/11/2016 01:03 PM, Programmingkid wrote:
> 
>>> +/* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
>>> + * the wrong type. Our replacement isn't usable in preprocessor
>>> + * expressions, but it is sufficient for our needs. */
>>> +#if defined(HAVE_BROKEN_SIZE_MAX) && HAVE_BROKEN_SIZE_MAX
>>> +#undef SIZE_MAX
>>> +#define SIZE_MAX ((size_t)-1)
>>> +#endif
>>> +
> 
>> I have applied your patch to the most recent git commit 
>> (627eae7d729277c84f8e0ac07a8caab39c92c38d) on Mac  OS 10.6.8. QEMU built 
>> without any problems using gcc 4.9.
> 
> Did you also tweak the code to make sure there was an instance of
> printf("%zu", SIZE_MAX) (or similar)? It's not enough that it compiles
> without complaint (although that helps), but also that the
> compiler-warning-on-printf goes away (which we currently don't have any
> in the tree, because we've been writing '"%zu", (size_t)SIZE_MAX' to
> work around the broken headers).

I saw no warnings when I added your printf code. The output was 
18446744073709551615.

Re: [Qemu-block] [Qemu-devel] [PATCH v5] build: Work around SIZE_MAX bug in OSX headers

[Peter Maydell]

On 11 October 2016 at 19:12, Eric Blake  wrote:
> On 10/11/2016 01:03 PM, Programmingkid wrote:
>
>>> +/* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
>>> + * the wrong type. Our replacement isn't usable in preprocessor
>>> + * expressions, but it is sufficient for our needs. */
>>> +#if defined(HAVE_BROKEN_SIZE_MAX) && HAVE_BROKEN_SIZE_MAX
>>> +#undef SIZE_MAX
>>> +#define SIZE_MAX ((size_t)-1)
>>> +#endif
>>> +
>
>> I have applied your patch to the most recent git commit 
>> (627eae7d729277c84f8e0ac07a8caab39c92c38d) on Mac  OS 10.6.8. QEMU built 
>> without any problems using gcc 4.9.
>
> Did you also tweak the code to make sure there was an instance of
> printf("%zu", SIZE_MAX) (or similar)? It's not enough that it compiles
> without complaint (although that helps), but also that the
> compiler-warning-on-printf goes away (which we currently don't have any
> in the tree, because we've been writing '"%zu", (size_t)SIZE_MAX' to
> work around the broken headers).

I have made that check, and tested that the patch causes the
resulting build failure to go away.

I'll apply this to master...

thanks
-- PMM

Re: [Qemu-block] [Qemu-devel] [PATCH v5] build: Work around SIZE_MAX bug in OSX headers

[Eric Blake]

On 10/11/2016 01:03 PM, Programmingkid wrote:

>> +/* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
>> + * the wrong type. Our replacement isn't usable in preprocessor
>> + * expressions, but it is sufficient for our needs. */
>> +#if defined(HAVE_BROKEN_SIZE_MAX) && HAVE_BROKEN_SIZE_MAX
>> +#undef SIZE_MAX
>> +#define SIZE_MAX ((size_t)-1)
>> +#endif
>> +

> I have applied your patch to the most recent git commit 
> (627eae7d729277c84f8e0ac07a8caab39c92c38d) on Mac  OS 10.6.8. QEMU built 
> without any problems using gcc 4.9.

Did you also tweak the code to make sure there was an instance of
printf("%zu", SIZE_MAX) (or similar)? It's not enough that it compiles
without complaint (although that helps), but also that the
compiler-warning-on-printf goes away (which we currently don't have any
in the tree, because we've been writing '"%zu", (size_t)SIZE_MAX' to
work around the broken headers).

> 
> Reviewed-by: John Arbuckle 
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [Qemu-devel] [PATCH v5] build: Work around SIZE_MAX bug in OSX headers

[Programmingkid]

On Oct 11, 2016, at 12:00 PM, qemu-devel-requ...@nongnu.org wrote:

> C99 requires SIZE_MAX to be declared with the same type as the
> integral promotion of size_t, but OSX mistakenly defines it as
> an 'unsigned long long' expression even though size_t is only
> 'unsigned long'.  Rather than futzing around with whether size_t
> is 32- or 64-bits wide (which would be needed if we cared about
> using SIZE_T in a #if expression), just hard-code it with a cast.
> This is not a strict C99-compliant definition, because it doesn't
> work in the preprocessor, but if we later need that, the build
> will break on Mac to inform us to improve our replacement at that
> time.
> 
> See also https://patchwork.ozlabs.org/patch/542327/ for an
> instance where the wrong type trips us up if we don't fix it
> for good in osdep.h.
> 
> Some versions of glibc make a similar mistake with SSIZE_MAX; the
> goal is that the approach of this patch could be copied to work
> around that problem if it ever becomes important to us.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v1 was here:
> https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg02520.html
> 
> The topic recently came up again, and I noticed this patch sitting
> on one of my older branches, so I've taken another shot at it.
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00950.html
> 
> v2: rewrite into a configure check (not sure if directly adding a
> -D to QEMU_CFLAGS is the best, so advice welcome)
> 
> v3: Use config-host.mak rather than direct -D [Peter]
> 
> v4: placate -Wunused builds
> 
> v5: use a simpler cast, rather than arithmetic promotion [Markus]
> 
> I lack easy access to a Mac box, so this is untested as to whether
> it actually solves the issue...
> ---
> include/qemu/osdep.h |  8 
> configure| 16 
> 2 files changed, 24 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 9e9fa61..c65dad7 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -141,6 +141,14 @@ extern int daemon(int, int);
> # error Unknown pointer size
> #endif
> 
> +/* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
> + * the wrong type. Our replacement isn't usable in preprocessor
> + * expressions, but it is sufficient for our needs. */
> +#if defined(HAVE_BROKEN_SIZE_MAX) && HAVE_BROKEN_SIZE_MAX
> +#undef SIZE_MAX
> +#define SIZE_MAX ((size_t)-1)
> +#endif
> +
> #ifndef MIN
> #define MIN(a, b) (((a) < (b)) ? (a) : (b))
> #endif
> diff --git a/configure b/configure
> index 5751d8e..dd9e679 100755
> --- a/configure
> +++ b/configure
> @@ -1725,6 +1725,19 @@ if test "$cocoa" = "yes"; then
> sdl=no
> fi
> 
> +# Some versions of Mac OS X incorrectly define SIZE_MAX
> +cat > $TMPC << EOF
> +#include 
> +#include 
> +int main(int argc, char *argv[]) {
> +return printf("%zu", SIZE_MAX);
> +}
> +EOF
> +have_broken_size_max=no
> +if ! compile_object -Werror ; then
> +have_broken_size_max=yes
> +fi
> +
> ##
> # L2TPV3 probe
> 
> @@ -5245,6 +5258,9 @@ fi
> if test "$have_ifaddrs_h" = "yes" ; then
> echo "HAVE_IFADDRS_H=y" >> $config_host_mak
> fi
> +if test "$have_broken_size_max" = "yes" ; then
> +echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
> +fi
> 
> # Work around a system header bug with some kernel/XFS header
> # versions where they both try to define 'struct fsxattr':
> -- 
> 2.7.4

I have applied your patch to the most recent git commit 
(627eae7d729277c84f8e0ac07a8caab39c92c38d) on Mac  OS 10.6.8. QEMU built 
without any problems using gcc 4.9.

Reviewed-by: John Arbuckle 

Re: [Qemu-block] [Qemu-devel] [PATCH v5] build: Work around SIZE_MAX bug in OSX headers

[Markus Armbruster]

Eric Blake  writes:

> C99 requires SIZE_MAX to be declared with the same type as the
> integral promotion of size_t, but OSX mistakenly defines it as
> an 'unsigned long long' expression even though size_t is only
> 'unsigned long'.  Rather than futzing around with whether size_t
> is 32- or 64-bits wide (which would be needed if we cared about
> using SIZE_T in a #if expression), just hard-code it with a cast.
> This is not a strict C99-compliant definition, because it doesn't
> work in the preprocessor, but if we later need that, the build
> will break on Mac to inform us to improve our replacement at that
> time.
>
> See also https://patchwork.ozlabs.org/patch/542327/ for an
> instance where the wrong type trips us up if we don't fix it
> for good in osdep.h.
>
> Some versions of glibc make a similar mistake with SSIZE_MAX; the
> goal is that the approach of this patch could be copied to work
> around that problem if it ever becomes important to us.
>
> Signed-off-by: Eric Blake 

Reviewed-by: Markus Armbruster