[Spice-devel] [PATCH spice-gtk] channel: rely on couroutine instead of channel state

[Hans de Goede]

Hi,

On 11/30/2012 09:12 PM, Marc-André Lureau wrote:
> We can simplify the channel state callback and simplify a little
> the code by relying on coroutine state instead.
> ---
>   gtk/spice-channel-priv.h |  3 ---
>   gtk/spice-channel.c      | 34 +++++++++-------------------------
>   2 files changed, 9 insertions(+), 28 deletions(-)
>
> diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
> index 41a1b34..5e6ec1e 100644
> --- a/gtk/spice-channel-priv.h
> +++ b/gtk/spice-channel-priv.h
> @@ -68,9 +68,6 @@ struct _SpiceMsgIn {
>   enum spice_channel_state {
>       SPICE_CHANNEL_STATE_UNCONNECTED = 0,
>       SPICE_CHANNEL_STATE_CONNECTING,
> -    SPICE_CHANNEL_STATE_LINK_HDR,
> -    SPICE_CHANNEL_STATE_LINK_MSG,
> -    SPICE_CHANNEL_STATE_AUTH,
>       SPICE_CHANNEL_STATE_READY,
>       SPICE_CHANNEL_STATE_SWITCHING,
>       SPICE_CHANNEL_STATE_MIGRATING,
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index 41ca6f8..cff047d 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -1203,7 +1203,6 @@ static void spice_channel_recv_link_hdr(SpiceChannel *channel)
>           goto error;
>       }
>
> -    c->state = SPICE_CHANNEL_STATE_LINK_MSG;
>       return;
>
>   error:
> @@ -1705,7 +1704,6 @@ static void spice_channel_recv_link_msg(SpiceChannel *channel)
>           CHANNEL_DEBUG(channel, "got channel caps %u:0x%X", i, *caps);
>       }
>
> -    c->state = SPICE_CHANNEL_STATE_AUTH;
>       if (!spice_channel_test_common_capability(channel,
>               SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION)) {
>           CHANNEL_DEBUG(channel, "Server supports spice ticket auth only");
> @@ -2022,28 +2020,10 @@ static void spice_channel_iterate_write(SpiceChannel *channel)
>   static void spice_channel_iterate_read(SpiceChannel *channel)
>   {
>       SpiceChannelPrivate *c = channel->priv;
> +    g_return_if_fail(c->state != SPICE_CHANNEL_STATE_MIGRATING);
>
> -    /* TODO: get rid of state, and use coroutine state */
> -    switch (c->state) {
> -    case SPICE_CHANNEL_STATE_LINK_HDR:
> -        spice_channel_recv_link_hdr(channel);
> -        break;
> -    case SPICE_CHANNEL_STATE_LINK_MSG:
> -        spice_channel_recv_link_msg(channel);
> -        break;
> -    case SPICE_CHANNEL_STATE_AUTH:
> -        spice_channel_recv_auth(channel);
> -        break;
> -    case SPICE_CHANNEL_STATE_READY:
> -    case SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE:
> -        spice_channel_recv_msg(channel,
> -            (handler_msg_in)SPICE_CHANNEL_GET_CLASS(channel)->handle_msg, NULL);
> -        break;
> -    case SPICE_CHANNEL_STATE_MIGRATING:
> -        return;
> -    default:
> -        g_critical("unknown state %d", c->state);
> -    }
> +    spice_channel_recv_msg(channel,
> +        (handler_msg_in)SPICE_CHANNEL_GET_CLASS(channel)->handle_msg, NULL);
>   }
>
>   static gboolean wait_migration(gpointer data)
> @@ -2076,7 +2056,9 @@ static gboolean spice_channel_iterate(SpiceChannel *channel)
>           }
>
>           SPICE_CHANNEL_GET_CLASS(channel)->iterate_write(channel);
> -        ret = g_coroutine_socket_wait(&c->coroutine, c->sock, G_IO_IN);
> +
> +        ret = g_coroutine_socket_wait(&c->coroutine, c->sock,
> +            c->state != SPICE_CHANNEL_STATE_MIGRATING ? G_IO_IN : 0);
>
>   #ifdef WIN32
>           /* FIXME: windows gsocket is buggy, it doesn't return correct condition... */


Hmm, this seems like an unrelated change/fix and one which could do with its own
commit message. Otherwise ack.

> @@ -2309,8 +2291,10 @@ connected:
>                     strerror(errno));
>       }
>
> -    c->state = SPICE_CHANNEL_STATE_LINK_HDR;
>       spice_channel_send_link(channel);
> +    spice_channel_recv_link_hdr(channel);
> +    spice_channel_recv_link_msg(channel);
> +    spice_channel_recv_auth(channel);
>
>       while (spice_channel_iterate(channel))
>           ;
>

Regards,

Hans