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

[Christophe de Dinechin]

> On 12 May 2017, at 15:29, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>>> 
>>> 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?

It’s another case that triggers the “cast changes alignment” warning. The correct fix is a SPICE_ALIGNED_CAST.


Christophe


> 
>> Would that be better in your opinion?
>> 
>> 
>> Christophe
>> 
>>> 
>>> Regards,
>>> Daniel
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org <mailto:Spice-devel at lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/spice-devel <https://lists.freedesktop.org/mailman/listinfo/spice-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170512/1d7b9600/attachment-0001.html>