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

[Eric Blake]

On 10/11/2016 02:18 AM, Markus Armbruster wrote:

>> +#define SIZE_MAX ((sizeof(char)) * -1)
> 
> All right, let's see how this works.
> 

> 
> Cute, but what's wrong with straightforward
> 
> #define SIZE_MAX ((size_t)-1)

I was trying to make the macro usable even in situations where 'size_t'
itself is undefined (because sufficient headers were not pre-included).
But you're right:  pulls in , and therefore size_t
is always available.  And since neither my solution nor your shorter
direct cast is usable in a preprocessor expression, neither has that
advantage in its favor (a solution that works in ALL scenarios required
by C99, at the expense of more text, would have an advantage).  So I
guess going with the shorter direct cast is just fine.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] [PATCH v4] 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), let the compiler get the right
> type for us by virtue of integer promotion - if we later need it

By virtue of usual arithmetic conversions, actually.

> during the preprocessor, the build will break on Mac until we
> improve our replacement.
>
> 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
>
> v4: placate -Wunused builds
>
> 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..e06ac47 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 ((sizeof(char)) * -1)

All right, let's see how this works.

* -1 is of type int (C99 §6.4.4.1).

* sizeof(char) is a clever way to say (size_t)1.

* Integer promotion (§6.3.1.1) is a no-op for int, i.e. the left operand
  stays int.

* Integer promotion is a no-op for size_t, since size_t is surely at
  least as wide as unsigned int.  The right operand stays size_t.

* The type of the multiplication is determined by the usual arithmetic
  conversions (§6.3.1.6).  Since size_t's rank is surely greater or
  equal to int's rank, the int operand is converted to size_t, and the
  result is size_t.

* The value of the expression is therefore (size_t)-1 * (size_t)1,
  yielding (size_t)-1.

Cute, but what's wrong with straightforward

#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':