[Spice-commits] 3 commits - gtk/spice-channel.c m4/warnings.m4
Christophe Fergau
teuf at kemper.freedesktop.org
Thu Oct 10 09:02:51 PDT 2013
gtk/spice-channel.c | 26 ++++++----------
m4/warnings.m4 | 82 +++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 72 insertions(+), 36 deletions(-)
New commits:
commit e2b8da36101ebfb9a5fe46c59c7683c8adf9f6eb
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Thu Oct 10 16:57:30 2013 +0200
Fix switch to old SPICE protocol
After the previous commit, spice_channel_switch_protocol() is now
called when needed, but it's not doing anything. What happens is
that spice_channel_switch_protocol() triggers a channel disconnection
and then it queues an idle to reconnect (after having changed the
protocol version to be used).
When spice_channel_recv_link_hdr() returns, we need to jump out of
the coroutine to let the idle trigger and the new channel coroutine
be started. But jumping out of the coroutine will call channel_disconnect()
which calls channel_reset() which disables the idle switch_protocol()
just queued. This causes the connection attempt to be apparently
stuck with nothing happening.
Falling back to the older SPICE protocol is not the only situation
when we need to drop the current connection attempt and reconnect,
we also need to do that when the remote server returns
SPICE_LINK_ERR_NEED_SECURED to let the client know it needs to use
a secure port for this channel. This is handled by the 'switch_tls'
variable set in spice_channel_recv_link_msg and handled in
spice_channel_coroutine().
'switch_tls' does the same thing as spice_channel_switch_protocol(),
except that it calls spice_channel_connect() after channel_disconnect()
has been called, which means the idle queued by channel_connect()
won't get cleared.
This all that commit does, remove the spice_channel_switch_protocol()
method and use the same codepath as 'switch_tls' to handle the
reconnection.
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 709d7e0..2c4d44c 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1169,21 +1169,13 @@ static void spice_channel_send_link(SpiceChannel *channel)
free(buffer);
}
-static void spice_channel_switch_protocol(SpiceChannel *channel, gint version)
-{
- SpiceChannelPrivate *c = channel->priv;
-
- g_object_set(c->session, "protocol", version, NULL);
- SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
- spice_channel_connect(channel);
-}
-
/* coroutine context */
-static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel)
+static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel, gboolean *switch_protocol)
{
SpiceChannelPrivate *c = channel->priv;
int rc;
+ *switch_protocol = FALSE;
rc = spice_channel_read(channel, &c->peer_hdr, sizeof(c->peer_hdr));
if (rc != sizeof(c->peer_hdr)) {
g_warning("incomplete link header (%d/%" G_GSIZE_FORMAT ")",
@@ -1216,7 +1208,8 @@ error:
incompatible. Try with the oldest protocol in this case: */
if (c->link_hdr.major_version != 1) {
SPICE_DEBUG("%s: error, switching to protocol 1 (spice 0.4)", c->name);
- spice_channel_switch_protocol(channel, 1);
+ *switch_protocol = TRUE;
+ g_object_set(c->session, "protocol", 1, NULL);
return FALSE;
}
@@ -2210,6 +2203,7 @@ static void *spice_channel_coroutine(void *data)
guint verify;
int rc, delay_val = 1;
gboolean switch_tls = FALSE;
+ gboolean switch_protocol = FALSE;
CHANNEL_DEBUG(channel, "Started background coroutine %p", &c->coroutine);
@@ -2332,7 +2326,7 @@ connected:
}
spice_channel_send_link(channel);
- if (spice_channel_recv_link_hdr(channel) == FALSE)
+ if (spice_channel_recv_link_hdr(channel, &switch_protocol) == FALSE)
goto cleanup;
spice_channel_recv_link_msg(channel, &switch_tls);
if (switch_tls)
@@ -2347,8 +2341,8 @@ cleanup:
SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
- if (switch_tls && !c->tls) {
- c->tls = true;
+ if (switch_protocol || (switch_tls && !c->tls)) {
+ c->tls = switch_tls;
spice_channel_connect(channel);
g_object_unref(channel);
} else
commit e8393910e9aaf2610e1a587f41e8b17229cb71bd
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Thu Oct 10 16:28:04 2013 +0200
Reenable call to switch_protocol() on protocol version mismatches
This partially reverts b19acbc. This commit broke the fallback
to the old protocol as it added a check for c->peer_msg != NULL
before calling switch_protocol(), but mismatch between local
and remote protocol versions is detected before c->peer_msg is
allocated, so:
if (c->peer_msg != NULL && c->link_hdr.major_version != 1) {
SPICE_DEBUG("%s: error, switching to protocol 1 (spice 0.4)",
c->name);
spice_channel_switch_protocol(channel, 1);
return TRUE;
}
will never get triggered when c->peer_hdr.major_version !=
c->link_hdr.major_version
The crash described in b19acbc occurred when calling
spice_channel_recv_link_msg() in spice_channel_coroutine()
after a call to spice_channel_recv_link_hdr() failed and did
not set c->peer_msg.
This commit removes the c>peer_msg check done before calling
spice_channel_switch_protocol() so that it gets called when needed,
but makes sure that we return FALSE to indicate that an error happened
and that we need to reconnect. This way we won't try to call
spice_channel_recv_link_msg() when c->peer_msg is NULL.
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 150636e..709d7e0 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1214,10 +1214,10 @@ error:
/* Windows socket seems to give early CONNRESET errors. The server
does not linger when closing the socket if the protocol is
incompatible. Try with the oldest protocol in this case: */
- if (c->peer_msg != NULL && c->link_hdr.major_version != 1) {
+ if (c->link_hdr.major_version != 1) {
SPICE_DEBUG("%s: error, switching to protocol 1 (spice 0.4)", c->name);
spice_channel_switch_protocol(channel, 1);
- return TRUE;
+ return FALSE;
}
emit_main_context(channel, SPICE_CHANNEL_EVENT, SPICE_CHANNEL_ERROR_LINK);
commit 02aaa8678f311602793ede0f4e081fbd9cf28acd
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Thu Sep 26 16:30:27 2013 +0200
Use latest warnings.m4 from gnulib
The one we were using does not work properly with clang, causing
the build process to try to use -f/-W options that are not
supported by clang.
diff --git a/m4/warnings.m4 b/m4/warnings.m4
index 69d05a6..e3d239b 100644
--- a/m4/warnings.m4
+++ b/m4/warnings.m4
@@ -1,5 +1,5 @@
-# warnings.m4 serial 5
-dnl Copyright (C) 2008-2012 Free Software Foundation, Inc.
+# warnings.m4 serial 11
+dnl Copyright (C) 2008-2013 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
@@ -14,24 +14,66 @@ m4_ifdef([AS_VAR_APPEND],
[m4_define([gl_AS_VAR_APPEND],
[AS_VAR_SET([$1], [AS_VAR_GET([$1])$2])])])
-# gl_WARN_ADD(PARAMETER, [VARIABLE = WARN_CFLAGS])
-# ------------------------------------------------
-# Adds parameter to WARN_CFLAGS if the compiler supports it. For example,
-# gl_WARN_ADD([-Wparentheses]).
-AC_DEFUN([gl_WARN_ADD],
-dnl FIXME: gl_Warn must be used unquoted until we can assume
-dnl autoconf 2.64 or newer.
-[AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_$1])dnl
-AC_CACHE_CHECK([whether compiler handles $1], m4_defn([gl_Warn]), [
- gl_save_CPPFLAGS="$CPPFLAGS"
- CPPFLAGS="${CPPFLAGS} $1"
- AC_PREPROC_IFELSE([AC_LANG_PROGRAM([])],
- [AS_VAR_SET(gl_Warn, [yes])],
- [AS_VAR_SET(gl_Warn, [no])])
- CPPFLAGS="$gl_save_CPPFLAGS"
+
+# gl_COMPILER_OPTION_IF(OPTION, [IF-SUPPORTED], [IF-NOT-SUPPORTED],
+# [PROGRAM = AC_LANG_PROGRAM()])
+# -----------------------------------------------------------------
+# Check if the compiler supports OPTION when compiling PROGRAM.
+#
+# FIXME: gl_Warn must be used unquoted until we can assume Autoconf
+# 2.64 or newer.
+AC_DEFUN([gl_COMPILER_OPTION_IF],
+[AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
+AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl
+AS_LITERAL_IF([$1],
+ [m4_pushdef([gl_Positive], m4_bpatsubst([$1], [^-Wno-], [-W]))],
+ [gl_positive="$1"
+case $gl_positive in
+ -Wno-*) gl_positive=-W`expr "X$gl_positive" : 'X-Wno-\(.*\)'` ;;
+esac
+m4_pushdef([gl_Positive], [$gl_positive])])dnl
+AC_CACHE_CHECK([whether _AC_LANG compiler handles $1], m4_defn([gl_Warn]), [
+ gl_save_compiler_FLAGS="$gl_Flags"
+ gl_AS_VAR_APPEND(m4_defn([gl_Flags]),
+ [" $gl_unknown_warnings_are_errors ]m4_defn([gl_Positive])["])
+ AC_LINK_IFELSE([m4_default([$4], [AC_LANG_PROGRAM([])])],
+ [AS_VAR_SET(gl_Warn, [yes])],
+ [AS_VAR_SET(gl_Warn, [no])])
+ gl_Flags="$gl_save_compiler_FLAGS"
])
-AS_VAR_IF(gl_Warn, [yes],
- [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_CFLAGS]], [[$2]]), [" $1"])])
+AS_VAR_IF(gl_Warn, [yes], [$2], [$3])
+m4_popdef([gl_Positive])dnl
+AS_VAR_POPDEF([gl_Flags])dnl
AS_VAR_POPDEF([gl_Warn])dnl
-m4_ifval([$2], [AS_LITERAL_IF([$2], [AC_SUBST([$2])], [])])dnl
])
+
+# gl_UNKNOWN_WARNINGS_ARE_ERRORS
+# ------------------------------
+# Clang doesn't complain about unknown warning options unless one also
+# specifies -Wunknown-warning-option -Werror. Detect this.
+AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS],
+[gl_COMPILER_OPTION_IF([-Werror -Wunknown-warning-option],
+ [gl_unknown_warnings_are_errors='-Wunknown-warning-option -Werror'],
+ [gl_unknown_warnings_are_errors=])])
+
+# gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS],
+# [PROGRAM = AC_LANG_PROGRAM()])
+# ---------------------------------------------
+# Adds parameter to WARN_CFLAGS if the compiler supports it when
+# compiling PROGRAM. For example, gl_WARN_ADD([-Wparentheses]).
+#
+# If VARIABLE is a variable name, AC_SUBST it.
+AC_DEFUN([gl_WARN_ADD],
+[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS])
+gl_COMPILER_OPTION_IF([$1],
+ [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_CFLAGS]], [[$2]]), [" $1"])],
+ [],
+ [$3])
+m4_ifval([$2],
+ [AS_LITERAL_IF([$2], [AC_SUBST([$2])])],
+ [AC_SUBST([WARN_CFLAGS])])dnl
+])
+
+# Local Variables:
+# mode: autoconf
+# End:
More information about the Spice-commits
mailing list