[Spice-devel] [PATCH spice-gtk v2 4/4] Add check for macOS, disable ucontext on macOS (deprecated)

[Frediano Ziglio]

> 
> > On 11 May 2017, at 12:56, Daniel P. Berrange <berrange at redhat.com> wrote:
> > 
> > On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
> >> From: Christophe de Dinechin <dinechin at redhat.com>
> >> 
> >> Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
> >> ---
> >> configure.ac | 14 ++++++++++++++
> >> 1 file changed, 14 insertions(+)
> >> 
> >> diff --git a/configure.ac b/configure.ac
> >> index 74b5811..ecab365 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -62,6 +62,18 @@ esac
> >> AC_MSG_RESULT([$os_win32])
> >> AM_CONDITIONAL([OS_WIN32],[test "$os_win32" = "yes"])
> >> 
> >> +AC_MSG_CHECKING([for native macOS])
> >> +case "$host_os" in
> >> +     *darwin*)
> >> +        os_mac=yes
> >> +        ;;
> >> +     *)
> >> +        os_mac=no
> >> +        ;;
> >> +esac
> >> +AC_MSG_RESULT([$os_mac])
> >> +AM_CONDITIONAL([OS_MAC],[test "$os_mac" = "yes"])
> >> +
> >> AC_CHECK_HEADERS([sys/socket.h netinet/in.h arpa/inet.h])
> >> AC_CHECK_HEADERS([termios.h])
> >> AC_CHECK_HEADERS([epoxy/egl.h],
> >> @@ -468,6 +480,8 @@ esac
> >> if test "$with_coroutine" = "auto"; then
> >>   if test "$os_win32" = "yes"; then
> >>     with_coroutine=winfiber
> >> +  elif test "$os_mac" = "yes"; then
> >> +    with_coroutine=gthread
> >>   else
> >>     with_coroutine=ucontext
> >>   fi
> > 
> > Despite ucontext being deprecated we are still better off using that &
> > ignoring the warnings, than using the gthread impl.
> 
> Yes, I remember you explained the benefits of keeping ucontext. But for the
> moment at least, on macOS, it is not a warning:
> 
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/ucontext.h:43:2:
> error:
>       The deprecated ucontext routines require _XOPEN_SOURCE to be defined
> #error The deprecated ucontext routines require _XOPEN_SOURCE to be defined
>  ^
> 1 error generated.
> 
> So I can:
> - Add the error in the patch description
> - Attempt to define _XOPEN_SOURCE in the configuration. I’m concerned about
> side effects.
> 
> The latter leads to another can of worms. Notably, the macro container_of
> triggers the alignment warning for container_of, so I have a set of
> alignment warnings, and a set of deprecation warnings. But it builds. The
> incremental patch would be something like:
> 
> 
> diff --git a/configure.ac b/configure.ac
> index ecab365..8b433ba 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -481,7 +481,8 @@ if test "$with_coroutine" = "auto"; then
>    if test "$os_win32" = "yes"; then
>      with_coroutine=winfiber
>    elif test "$os_mac" = "yes"; then
> -    with_coroutine=gthread
> +    with_coroutine=ucontext
> +    AC_DEFINE([_XOPEN_SOURCE], [1], [Define _XOPEN_SOURCE on macOS for
> ucontext])
>    else
>      with_coroutine=ucontext
>    fi
> diff --git a/src/continuation.h b/src/continuation.h
> index 675a257..cbca06e 100644
> --- a/src/continuation.h
> +++ b/src/continuation.h
> @@ -49,7 +49,7 @@ int cc_swap(struct continuation *from, struct continuation
> *to);
>  
>  #define offset_of(type, member) ((unsigned long)(&((type *)0)->member))
>  #define container_of(obj, type, member) \
> -        (type *)(((char *)obj) - offset_of(type, member))
> +        (type *)(void *)(((char *)obj) - offset_of(type, member))
>  
>  #endif
>  /*
> 

This change is ok. If it was a member of a structure surely the
structure must be aligned, if not the code that allocated the structure
is broken, not container_of.
Wondering why this change is needed. Maybe code using ucontext is
causing it?

> Would that be better in your opinion?
> 
> 
> Christophe
> 
> > 
> > Regards,
> > Daniel

Frediano