8567026
From cae2385ba898f71038ed4dd00ddae02f85e588e7 Mon Sep 17 00:00:00 2001
8567026
From: Salvador <sfandino@yahoo.com>
8567026
Date: Tue, 15 Oct 2013 11:45:10 +0200
8567026
Subject: [PATCH 08/11] _libssh2_channel_read: fix data drop when out of window
8567026
8567026
After filling the read buffer with data from the read queue, when the
8567026
window size was too small, "libssh2_channel_receive_window_adjust" was
8567026
called to increase it. In non-blocking mode that function could return
8567026
EAGAIN and, in that case, the EAGAIN was propagated upwards and the data
8567026
already read on the buffer lost.
8567026
8567026
The function was also moving between the two read states
8567026
"libssh2_NB_state_idle" and "libssh2_NB_state_created" both of which
8567026
behave in the same way (excepting a debug statment).
8567026
8567026
This commit modifies "_libssh2_channel_read" so that the
8567026
"libssh2_channel_receive_window_adjust" call is performed first (when
8567026
required) and if everything goes well, then it reads the data from the
8567026
queued packets into the read buffer.
8567026
8567026
It also removes the useless "libssh2_NB_state_created" read state.
8567026
8567026
Some rotted comments have also been updated.
8567026
8567026
Signed-off-by: Salvador <sfandino@yahoo.com>
8567026
8567026
[upstream commit 27f9ac2549b7721cf9d857022c0e7a311830b367]
8567026
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
8567026
---
8567026
 src/channel.c |   75 +++++++++++++++++++--------------------------------------
8567026
 1 files changed, 25 insertions(+), 50 deletions(-)
8567026
8567026
diff --git a/src/channel.c b/src/channel.c
8567026
index 499d815..82f6980 100644
8567026
--- a/src/channel.c
8567026
+++ b/src/channel.c
8567026
@@ -1751,31 +1751,33 @@ ssize_t _libssh2_channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
8567026
     LIBSSH2_PACKET *read_packet;
8567026
     LIBSSH2_PACKET *read_next;
8567026
 
8567026
-    if (channel->read_state == libssh2_NB_state_idle) {
8567026
-        _libssh2_debug(session, LIBSSH2_TRACE_CONN,
8567026
-                       "channel_read() wants %d bytes from channel %lu/%lu "
8567026
-                       "stream #%d",
8567026
-                       (int) buflen, channel->local.id, channel->remote.id,
8567026
-                       stream_id);
8567026
-        channel->read_state = libssh2_NB_state_created;
8567026
-    }
8567026
+    _libssh2_debug(session, LIBSSH2_TRACE_CONN,
8567026
+                   "channel_read() wants %d bytes from channel %lu/%lu "
8567026
+                   "stream #%d",
8567026
+                   (int) buflen, channel->local.id, channel->remote.id,
8567026
+                   stream_id);
8567026
 
8567026
-    /*
8567026
-     * =============================== NOTE ===============================
8567026
-     * I know this is very ugly and not a really good use of "goto", but
8567026
-     * this case statement would be even uglier to do it any other way
8567026
-     */
8567026
-    if (channel->read_state == libssh2_NB_state_jump1) {
8567026
-        goto channel_read_window_adjust;
8567026
-    }
8567026
+    /* expand the receiving window first if it has become too narrow */
8567026
+    if((channel->read_state == libssh2_NB_state_jump1) ||
8567026
+       (channel->remote.window_size < (LIBSSH2_CHANNEL_WINDOW_DEFAULT*30))) {
8567026
+
8567026
+        /* the actual window adjusting may not finish so we need to deal with
8567026
+           this special state here */
8567026
+        channel->read_state = libssh2_NB_state_jump1;
8567026
+        rc = _libssh2_channel_receive_window_adjust(channel,
8567026
+                                                    (LIBSSH2_CHANNEL_WINDOW_DEFAULT*60),
8567026
+                                                    0, NULL);
8567026
+        if (rc)
8567026
+            return rc;
8567026
 
8567026
-    rc = 1; /* set to >0 to let the while loop start */
8567026
+        channel->read_state = libssh2_NB_state_idle;
8567026
+    }
8567026
 
8567026
-    /* Process all pending incoming packets in all states in order to "even
8567026
-       out" the network readings. Tests prove that this way produces faster
8567026
-       transfers. */
8567026
-    while (rc > 0)
8567026
+    /* Process all pending incoming packets. Tests prove that this way
8567026
+       produces faster transfers. */
8567026
+    do {
8567026
         rc = _libssh2_transport_read(session);
8567026
+    } while (rc > 0);
8567026
 
8567026
     if ((rc < 0) && (rc != LIBSSH2_ERROR_EAGAIN))
8567026
         return _libssh2_error(session, rc, "transport read");
8567026
@@ -1857,8 +1859,6 @@ ssize_t _libssh2_channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
8567026
     }
8567026
 
8567026
     if (!bytes_read) {
8567026
-        channel->read_state = libssh2_NB_state_idle;
8567026
-
8567026
         /* If the channel is already at EOF or even closed, we need to signal
8567026
            that back. We may have gotten that info while draining the incoming
8567026
            transport layer until EAGAIN so we must not be fooled by that
8567026
@@ -1871,34 +1871,9 @@ ssize_t _libssh2_channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
8567026
         /* if the transport layer said EAGAIN then we say so as well */
8567026
         return _libssh2_error(session, rc, "would block");
8567026
     }
8567026
-    else {
8567026
-        channel->read_avail -= bytes_read;
8567026
-        channel->remote.window_size -= bytes_read;
8567026
-        /* make sure we remain in the created state to focus on emptying the
8567026
-           data we already have in the packet brigade before we try to read
8567026
-           more off the network again */
8567026
-        channel->read_state = libssh2_NB_state_created;
8567026
-    }
8567026
-
8567026
-    if(channel->remote.window_size < (LIBSSH2_CHANNEL_WINDOW_DEFAULT*30)) {
8567026
-        /* the window is getting too narrow, expand it! */
8567026
-
8567026
-      channel_read_window_adjust:
8567026
-        channel->read_state = libssh2_NB_state_jump1;
8567026
-        /* the actual window adjusting may not finish so we need to deal with
8567026
-           this special state here */
8567026
-        rc = _libssh2_channel_receive_window_adjust(channel,
8567026
-                                                    (LIBSSH2_CHANNEL_WINDOW_DEFAULT*60), 0, NULL);
8567026
-        if (rc)
8567026
-            return rc;
8567026
 
8567026
-        _libssh2_debug(session, LIBSSH2_TRACE_CONN,
8567026
-                       "channel_read() filled %d adjusted %d",
8567026
-                       bytes_read, buflen);
8567026
-        /* continue in 'created' state to drain the already read packages
8567026
-           first before starting to empty the socket further */
8567026
-        channel->read_state = libssh2_NB_state_created;
8567026
-    }
8567026
+    channel->read_avail -= bytes_read;
8567026
+    channel->remote.window_size -= bytes_read;
8567026
 
8567026
     return bytes_read;
8567026
 }
8567026
-- 
8567026
1.7.1
8567026