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

2017-05-15 Thread Christophe de Dinechin 

> On 15 May 2017, at 11:58, Frediano Ziglio  wrote:
> 
> 
> On 12 May 2017, at 15:29, Frediano Ziglio  > wrote:
> 
> 
> On 11 May 2017, at 12:56, Daniel P. Berrange  > wrote:
> 
> On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
> From: Christophe de Dinechin  >
> 
> Signed-off-by: Christophe de Dinechin  >
> ---
> 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.
> 
> I would go then with 3 macros instead of two
> 
> SPICE_ALIGNED_CAST, SPICE_UNALIGNED_CAST. We know if is aligned or not and we 
> just make the cast, no additional checks.
> SPICE_ALIGNED_CHECK_CAST. We are not sure and we do the check giving warning

What are the cases where you are sure enough that it’s aligned to not even 
check? If the compiler does its inlining job properly, the test is a bitmask 
test witih a constant, so typically one instruction unless it fails. I don’t 
think it’s really worth having a non-checking variant. Maybe I could add a 
G_UNLIKELY?

Christophe

> 
> How does it sound?
> 
> Frediano
> 
> Christophe
> 
> 
> 
> Would that be better in your opinion?
> 
> 
> Christophe
> 
> 
> Regards,
> Daniel
> 
> Frediano
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/spice-devel 
> 
> 
> 
> ___
> Spice-devel mailing list
>

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

2017-05-15 Thread Frediano Ziglio 
> > On 12 May 2017, at 15:29, Frediano Ziglio < fzig...@redhat.com > wrote:
> 

> > > > On 11 May 2017, at 12:56, Daniel P. Berrange < berra...@redhat.com >
> > > > wrote:
> > > 
> > 
> 

> > > > On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
> > > 
> > 
> 

> > > > > From: Christophe de Dinechin < dinec...@redhat.com >
> > > > 
> > > 
> > 
> 

> > > > > Signed-off-by: Christophe de Dinechin < dinec...@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

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

2017-05-12 Thread Christophe de Dinechin 

> On 12 May 2017, at 15:29, Frediano Ziglio  wrote:
> 
>>> 
>>> On 11 May 2017, at 12:56, Daniel P. Berrange  wrote:
>>> 
>>> On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
 From: Christophe de Dinechin 
 
 Signed-off-by: Christophe de Dinechin 
 ---
 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@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/spice-devel 
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2017-05-12 Thread Frediano Ziglio 
> 
> > On 11 May 2017, at 12:56, Daniel P. Berrange  wrote:
> > 
> > On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
> >> From: Christophe de Dinechin 
> >> 
> >> Signed-off-by: Christophe de Dinechin 
> >> ---
> >> 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
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2017-05-11 Thread Christophe de Dinechin 

> On 11 May 2017, at 12:56, Daniel P. Berrange  wrote:
> 
> On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin 
>> 
>> Signed-off-by: Christophe de Dinechin 
>> ---
>> 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
 /*

Would that be better in your opinion?


Christophe

> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2017-05-11 Thread Daniel P. Berrange 
On Thu, May 11, 2017 at 07:08:13AM -0400, Frediano Ziglio wrote:
> > 
> > On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
> > > From: Christophe de Dinechin 
> > > 
> > > Signed-off-by: Christophe de Dinechin 
> > > ---
> > >  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.
> > 
> > Regards,
> > Daniel
> 
> But Christophe reported that this is not compiling at all.
> Did you manage to compile under OsX with ucontext ?

Is that with or without -Werror though ?  I understand if -Werror makes
it fail due to the deprecation warnings, but AFAIK, the functionality
still exists & should be compatible.

> Why ucontext is better?

Better performance essentially. 

> According to 
> http://duriansoftware.com/joe/PSA:-avoiding-the-%22ucontext-routines-are-deprecated%22-error-on-Mac-OS-X-Snow-Leopard.html
> seems that defining _XOPEN_SOURCE should remove at least the error.

Better is to just use a GCC pragma to silence the compile warning for that
piece of code only.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2017-05-11 Thread Frediano Ziglio 
> 
> On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
> > From: Christophe de Dinechin 
> > 
> > Signed-off-by: Christophe de Dinechin 
> > ---
> >  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.
> 
> Regards,
> Daniel

But Christophe reported that this is not compiling at all.
Did you manage to compile under OsX with ucontext ?
Why ucontext is better?

According to 
http://duriansoftware.com/joe/PSA:-avoiding-the-%22ucontext-routines-are-deprecated%22-error-on-Mac-OS-X-Snow-Leopard.html
seems that defining _XOPEN_SOURCE should remove at least the error.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2017-05-11 Thread Daniel P. Berrange 
On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:
> From: Christophe de Dinechin 
> 
> Signed-off-by: Christophe de Dinechin 
> ---
>  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.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2017-05-11 Thread Christophe de Dinechin 
From: Christophe de Dinechin 

Signed-off-by: Christophe de Dinechin 
---
 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
-- 
2.11.0 (Apple Git-81)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel