[Spice-commits] 4 commits - src/spice-channel.c

Victor Toso de Carvalho victortoso at kemper.freedesktop.org
Fri Mar 3 07:57:53 UTC 2017


 src/spice-channel.c |  150 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 100 insertions(+), 50 deletions(-)

New commits:
commit b8dd12ac81a90644a4f41941d7d172987c4689cd
Author: Victor Toso <me at victortoso.com>
Date:   Wed Feb 22 15:42:07 2017 +0100

    spice_channel_read_wire: move variables to internal scope
    
    And avoid single line if plus comment
    
    Signed-off-by: Victor Toso <victortoso at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/spice-channel.c b/src/spice-channel.c
index f2d1b8a..c2e8a01 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1048,12 +1048,15 @@ static int spice_channel_read_wire_nonblocking(SpiceChannel *channel,
 static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len)
 {
     SpiceChannelPrivate *c = channel->priv;
-    GIOCondition cond;
-    gssize ret;
 
     while (TRUE) {
+        gssize ret;
+        GIOCondition cond;
 
-        if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */
+        if (c->has_error) {
+            /* has_error is set by disconnect(), return no error */
+            return 0;
+        }
 
         ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
 
commit f3b5130b2d35fb4be0790b711208343714e95c04
Author: Victor Toso <me at victortoso.com>
Date:   Wed Feb 22 15:30:36 2017 +0100

    spice_channel_read_wire: prefer while(TRUE) instead of goto
    
    Although this is likely to be a single loop iteration based on the
    implementation of g_coroutine_socket_wait(), using goto to reiterate
    in the code should be avoided based on spice style recommendation.
    
    This also make it easier to add follow up changes that can increase
    the loop iterations.
    
    This patch changes:
     * The reread label with a while(TRUE);
     * The goto keyword with continue;
    
    All other changes are only related to new indentation.
    
    Signed-off-by: Victor Toso <victortoso at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 999c615..f2d1b8a 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1051,29 +1051,30 @@ static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len
     GIOCondition cond;
     gssize ret;
 
-reread:
+    while (TRUE) {
 
-    if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */
+        if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */
 
-    ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
+        ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
 
-    if (ret == -1) {
-        if (cond != 0) {
-            // TODO: should use g_pollable_input/output_stream_create_source() ?
-            g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
-            goto reread;
-        } else {
+        if (ret == -1) {
+            if (cond != 0) {
+                // TODO: should use g_pollable_input/output_stream_create_source() ?
+                g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
+                continue;
+            } else {
+                c->has_error = TRUE;
+                return -errno;
+            }
+        }
+        if (ret == 0) {
+            CHANNEL_DEBUG(channel, "Closing the connection: spice_channel_read() - ret=0");
             c->has_error = TRUE;
-            return -errno;
+            return 0;
         }
-    }
-    if (ret == 0) {
-        CHANNEL_DEBUG(channel, "Closing the connection: spice_channel_read() - ret=0");
-        c->has_error = TRUE;
-        return 0;
-    }
 
-    return ret;
+        return ret;
+    }
 }
 
 #ifdef HAVE_SASL
commit 2bbf3ceb34f99ce9de07318cfff22f729eae7d87
Author: Victor Toso <me at victortoso.com>
Date:   Thu Feb 2 15:25:27 2017 +0100

    spice-channel: move out non blocking logic of _flush_wire()
    
    This patch introduces spice_channel_flush_wire_nonblocking() helper
    without changing any logic.
    
    Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
    Signed-off-by: Victor Toso <victortoso at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 4210c31..999c615 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -769,6 +769,53 @@ void spice_msg_out_send_internal(SpiceMsgOut *out)
 }
 
 /*
+ * Helper function to deal with the nonblocking part of _flush_wire() function.
+ * It returns the result of the write and will set the proper bits in @cond in
+ * case the write function would block.
+ *
+ * Returns -1 in case of any problems.
+ */
+/* coroutine context */
+static gint spice_channel_flush_wire_nonblocking(SpiceChannel *channel,
+                                                 const gchar *ptr,
+                                                 size_t len,
+                                                 GIOCondition *cond)
+{
+    SpiceChannelPrivate *c = channel->priv;
+    gssize ret;
+
+    g_assert(cond != NULL);
+    *cond = 0;
+
+    if (c->tls) {
+        ret = SSL_write(c->ssl, ptr, len);
+        if (ret < 0) {
+            ret = SSL_get_error(c->ssl, ret);
+            if (ret == SSL_ERROR_WANT_READ)
+                *cond |= G_IO_IN;
+            if (ret == SSL_ERROR_WANT_WRITE)
+                *cond |= G_IO_OUT;
+            ret = -1;
+        }
+    } else {
+        GError *error = NULL;
+        ret = g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(c->out),
+                                                         ptr, len, NULL, &error);
+        if (ret < 0) {
+            if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
+                *cond = G_IO_OUT;
+            } else {
+                CHANNEL_DEBUG(channel, "Send error %s", error->message);
+            }
+            g_clear_error(&error);
+            ret = -1;
+        }
+    }
+
+    return ret;
+}
+
+/*
  * Write all 'data' of length 'datalen' bytes out to
  * the wire
  */
@@ -784,34 +831,10 @@ static void spice_channel_flush_wire(SpiceChannel *channel,
 
     while (offset < datalen) {
         gssize ret;
-        GError *error = NULL;
 
         if (c->has_error) return;
 
-        cond = 0;
-        if (c->tls) {
-            ret = SSL_write(c->ssl, ptr+offset, datalen-offset);
-            if (ret < 0) {
-                ret = SSL_get_error(c->ssl, ret);
-                if (ret == SSL_ERROR_WANT_READ)
-                    cond |= G_IO_IN;
-                if (ret == SSL_ERROR_WANT_WRITE)
-                    cond |= G_IO_OUT;
-                ret = -1;
-            }
-        } else {
-            ret = g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(c->out),
-                                                             ptr+offset, datalen-offset, NULL, &error);
-            if (ret < 0) {
-                if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
-                    cond = G_IO_OUT;
-                } else {
-                    CHANNEL_DEBUG(channel, "Send error %s", error->message);
-                }
-                g_clear_error(&error);
-                ret = -1;
-            }
-        }
+        ret = spice_channel_flush_wire_nonblocking(channel, ptr+offset, datalen-offset, &cond);
         if (ret == -1) {
             if (cond != 0) {
                 // TODO: should use g_pollable_input/output_stream_create_source() in 2.28 ?
commit 7a33ce920d2a2b5fed30256591df06aaca49dd08
Author: Victor Toso <me at victortoso.com>
Date:   Thu Feb 2 14:51:08 2017 +0100

    spice-channel: move out non blocking logic of _read_wire()
    
    This patch introduces spice_channel_read_wire_nonblocking() helper
    without changing any logic.
    
    Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
    Signed-off-by: Victor Toso <victortoso at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/spice-channel.c b/src/spice-channel.c
index af67931..4210c31 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -971,29 +971,32 @@ gint spice_channel_unix_read_fd(SpiceChannel *channel)
 #endif
 
 /*
- * Read at least 1 more byte of data straight off the wire
- * into the requested buffer.
+ * Helper function to deal with the nonblocking part of _read_wire() function.
+ * It returns the result of the read and will set the proper bits in @cond in
+ * case the read function would block.
+ *
+ * Returns -1 in case of any problems.
  */
 /* coroutine context */
-static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len)
+static int spice_channel_read_wire_nonblocking(SpiceChannel *channel,
+                                               void *data,
+                                               size_t len,
+                                               GIOCondition *cond)
 {
     SpiceChannelPrivate *c = channel->priv;
     gssize ret;
-    GIOCondition cond;
-
-reread:
 
-    if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */
+    g_assert(cond != NULL);
+    *cond = 0;
 
-    cond = 0;
     if (c->tls) {
         ret = SSL_read(c->ssl, data, len);
         if (ret < 0) {
             ret = SSL_get_error(c->ssl, ret);
             if (ret == SSL_ERROR_WANT_READ)
-                cond |= G_IO_IN;
+                *cond |= G_IO_IN;
             if (ret == SSL_ERROR_WANT_WRITE)
-                cond |= G_IO_OUT;
+                *cond |= G_IO_OUT;
             ret = -1;
         }
     } else {
@@ -1002,7 +1005,7 @@ reread:
                                                        data, len, NULL, &error);
         if (ret < 0) {
             if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
-                cond = G_IO_IN;
+                *cond = G_IO_IN;
             } else {
                 CHANNEL_DEBUG(channel, "Read error %s", error->message);
             }
@@ -1011,6 +1014,26 @@ reread:
         }
     }
 
+    return ret;
+}
+
+/*
+ * Read at least 1 more byte of data straight off the wire
+ * into the requested buffer.
+ */
+/* coroutine context */
+static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len)
+{
+    SpiceChannelPrivate *c = channel->priv;
+    GIOCondition cond;
+    gssize ret;
+
+reread:
+
+    if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */
+
+    ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
+
     if (ret == -1) {
         if (cond != 0) {
             // TODO: should use g_pollable_input/output_stream_create_source() ?


More information about the Spice-commits mailing list