On 12 May 2017, at 15:29, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
On 11 May 2017, at 12:56, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote:
On Thu, May 11, 2017 at 12:47:08PM +0200, Christophe de Dinechin wrote:From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
---
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
How does it sound?
Frediano
ChristopheWould that be better in your opinion?
Christophe
Regards,
Daniel
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel