channel_read() was changed to _libssh2_channel_read() as lots of

internal code was changed to use that instead of wrongly using
libssh2_channel_read_ex(). Some files now need to include
channel.h to get this proto.

channel_read() calls libssh2_error() properly on transport_read()
failures

channel_read() was adjusted to not "invent" EAGAIN return code in
case the transport_read() didn't return it

channel_close() now returns 0 or error code, as
documented. Previously it would return number of bytes read in
the last read, which was confusing (and useless).

src/channel.c

@@ -1700,7 +1700,7 @@ libssh2_channel_handle_extended_data(LIBSSH2_CHANNEL *channel,
 
 
 /*
- * channel_read
+ * _libssh2_channel_read
  *
  * Read data from a channel
  *
@@ -1708,7 +1708,7 @@ libssh2_channel_handle_extended_data(LIBSSH2_CHANNEL *channel,
  * complete. If we read stuff from the wire but it was no payload data to fill
  * in the buffer with, we MUST make sure to return PACKET_EAGAIN.
  */
-static ssize_t channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
+ssize_t _libssh2_channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
                               char *buf, size_t buflen)
 {
     LIBSSH2_SESSION *session = channel->session;
@@ -1735,8 +1735,10 @@ static ssize_t channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
     while (rc > 0)
         rc = _libssh2_transport_read(session);
 
-    if ((rc < 0) && (rc != PACKET_EAGAIN))
+    if ((rc < 0) && (rc != PACKET_EAGAIN)) {
+        libssh2_error(session, rc, "tranport read", 0);
         return rc;
+    }
 
     /*
      * =============================== NOTE ===============================
@@ -1747,8 +1749,6 @@ static ssize_t channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
         goto channel_read_ex_point1;
     }
 
-    rc = 0;
-
     read_packet = _libssh2_list_first(&session->packets);
     while (read_packet && (bytes_read < (int) buflen)) {
         /* previously this loop condition also checked for
@@ -1825,24 +1825,11 @@ static ssize_t channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
         read_packet = read_next;
     }
 
-    if (bytes_read == 0) {
+    if (!bytes_read) {
         channel->read_state = libssh2_NB_state_idle;
-        if (channel->remote.close) {
+
-            libssh2_error(session, LIBSSH2_ERROR_CHANNEL_CLOSED,
+        /* if the transport layer said EAGAIN then we say so as well */
-                          "Remote end has closed this channel", 0);
+        return (rc == PACKET_EAGAIN)?rc:0;
-            return 0;
-        }
-        else {
-            /*
-             * when non-blocking, we must return PACKET_EAGAIN if we haven't
-             * completed reading the channel
-             */
-            if (!libssh2_channel_eof(channel)) {
-                /* TODO FIXME THIS IS WRONG */
-                return PACKET_EAGAIN;
-            }
-            return 0;
-        }
     }
     else
         /* make sure we remain in the created state to focus on emptying the
@@ -1888,8 +1875,8 @@ libssh2_channel_read_ex(LIBSSH2_CHANNEL *channel, int stream_id, char *buf,
                         size_t buflen)
 {
     int rc;
-    BLOCK_ADJUST(rc, channel->session, channel_read(channel, stream_id,
+    BLOCK_ADJUST(rc, channel->session,
-                                                    buf, buflen));
+                 _libssh2_channel_read(channel, stream_id, buf, buflen));
     return rc;
 }
 
@@ -2274,6 +2261,7 @@ channel_close(LIBSSH2_CHANNEL * channel)
         }
     }
 
+    if(rc != LIBSSH2_ERROR_EAGAIN) {
         /* set the local close state first when we're perfectly confirmed to not
            do any more EAGAINs */
         channel->local.close = 1;
@@ -2285,8 +2273,10 @@ channel_close(LIBSSH2_CHANNEL * channel)
         }
 
         channel->close_state = libssh2_NB_state_idle;
+    }
 
-    return rc;
+    /* return 0 or an error */
+    return rc>=0?0:rc;
 }
 
 /*
@@ -2388,12 +2378,14 @@ int _libssh2_channel_free(LIBSSH2_CHANNEL *channel)
     if (!channel->local.close
         && (session->socket_state == LIBSSH2_SOCKET_CONNECTED)) {
         rc = channel_close(channel);
+
+        fprintf(stderr, "channel_close: %d\n", rc);
+
         if(rc == PACKET_EAGAIN)
             return rc;
-
+        else if (rc < 0) {
-        if (rc) {
             channel->free_state = libssh2_NB_state_idle;
-            return -1;
+            return rc;
         }
     }
 

src/channel.h

@@ -104,5 +104,17 @@ _libssh2_channel_process_startup(LIBSSH2_CHANNEL *channel,
                                  const char *request, unsigned int request_len,
                                  const char *message, unsigned int message_len);
 
+
+/*
+ * _libssh2_channel_read
+ *
+ * Read data from a channel
+ *
+ * It is important to not return 0 until the currently read channel is
+ * complete. If we read stuff from the wire but it was no payload data to fill
+ * in the buffer with, we MUST make sure to return PACKET_EAGAIN.
+ */
+ssize_t _libssh2_channel_read(LIBSSH2_CHANNEL *channel, int stream_id,
+                              char *buf, size_t buflen);
 #endif /* __LIBSSH2_CHANNEL_H */
 

src/libssh2_priv.h

@@ -1048,7 +1048,7 @@ _libssh2_debug(LIBSSH2_SESSION * session, int context, const char *format, ...)
     session->err_msglen = strlen(errmsg); \
     session->err_should_free = should_free; \
     session->err_code = errcode; \
-    _libssh2_debug(session, LIBSSH2_DBG_ERROR, "%d - %s", session->err_code, session->err_msg); \
+    _libssh2_debug(session, LIBSSH2_DBG_ERROR, "%s:%d %d - %s", __func__, __LINE__, session->err_code, session->err_msg); \
 }
 
 #else /* ! LIBSSH2DEBUG */

src/publickey.c

@@ -37,6 +37,7 @@
 
 #include "libssh2_priv.h"
 #include "libssh2_publickey.h"
+#include "channel.h"
 
 #define LIBSSH2_PUBLICKEY_VERSION               2
 
@@ -168,7 +169,7 @@ publickey_packet_receive(LIBSSH2_PUBLICKEY * pkey,
     int rc;
 
     if (pkey->receive_state == libssh2_NB_state_idle) {
-        rc = libssh2_channel_read_ex(channel, 0, (char *) buffer, 4);
+        rc = _libssh2_channel_read(channel, 0, (char *) buffer, 4);
         if (rc == PACKET_EAGAIN) {
             return rc;
         } else if (rc != 4) {
@@ -190,7 +191,7 @@ publickey_packet_receive(LIBSSH2_PUBLICKEY * pkey,
     }
 
     if (pkey->receive_state == libssh2_NB_state_sent) {
-        rc = libssh2_channel_read_ex(channel, 0, (char *) pkey->receive_packet,
+        rc = _libssh2_channel_read(channel, 0, (char *) pkey->receive_packet,
                                    pkey->receive_packet_len);
         if (rc == PACKET_EAGAIN) {
             return rc;

src/scp.c

@@ -39,6 +39,8 @@
 #include <errno.h>
 #include <stdlib.h>
 
+#include "channel.h"
+
 
 /* Max. length of a quoted string after libssh2_shell_quotearg() processing */
 #define libssh2_shell_quotedsize(s)	(3 * strlen(s) + 2)
@@ -378,7 +380,7 @@ libssh2_scp_recv(LIBSSH2_SESSION * session, const char *path, struct stat * sb)
             unsigned char *s, *p;
 
             if (session->scpRecv_state == libssh2_NB_state_sent2) {
-                rc = libssh2_channel_read_ex(session->scpRecv_channel, 0,
+                rc = _libssh2_channel_read(session->scpRecv_channel, 0,
                                            (char *) session->
                                            scpRecv_response +
                                            session->scpRecv_response_len, 1);
@@ -415,7 +417,7 @@ libssh2_scp_recv(LIBSSH2_SESSION * session, const char *path, struct stat * sb)
                            session->scpRecv_err_len + 1);
 
                     /* Read the remote error message */
-                    rc = libssh2_channel_read_ex(session->scpRecv_channel, 0,
+                    rc = _libssh2_channel_read(session->scpRecv_channel, 0,
                                                session->scpRecv_err_msg,
                                                session->scpRecv_err_len);
                     if (rc <= 0) {
@@ -591,7 +593,7 @@ libssh2_scp_recv(LIBSSH2_SESSION * session, const char *path, struct stat * sb)
             char *s, *p, *e = NULL;
 
             if (session->scpRecv_state == libssh2_NB_state_sent5) {
-                rc = libssh2_channel_read_ex(session->scpRecv_channel, 0,
+                rc = _libssh2_channel_read(session->scpRecv_channel, 0,
                                            (char *) session->
                                            scpRecv_response +
                                            session->scpRecv_response_len, 1);
@@ -857,7 +859,7 @@ libssh2_scp_send_ex(LIBSSH2_SESSION * session, const char *path, int mode,
 
     if (session->scpSend_state == libssh2_NB_state_sent1) {
         /* Wait for ACK */
-        rc = libssh2_channel_read_ex(session->scpSend_channel, 0,
+        rc = _libssh2_channel_read(session->scpSend_channel, 0,
                                    (char *) session->scpSend_response, 1);
         if (rc == PACKET_EAGAIN) {
             libssh2_error(session, LIBSSH2_ERROR_EAGAIN,
@@ -903,9 +905,8 @@ libssh2_scp_send_ex(LIBSSH2_SESSION * session, const char *path, int mode,
 
         if (session->scpSend_state == libssh2_NB_state_sent3) {
             /* Wait for ACK */
-            rc = libssh2_channel_read_ex(session->scpSend_channel, 0,
+            rc = _libssh2_channel_read(session->scpSend_channel, 0,
-                                         (char *) session->scpSend_response,
+                                       (char *) session->scpSend_response, 1);
-                                         1);
             if (rc == PACKET_EAGAIN) {
                 libssh2_error(session, LIBSSH2_ERROR_EAGAIN,
                               "Would block waiting for response", 0);
@@ -962,7 +963,7 @@ libssh2_scp_send_ex(LIBSSH2_SESSION * session, const char *path, int mode,
 
     if (session->scpSend_state == libssh2_NB_state_sent6) {
         /* Wait for ACK */
-        rc = libssh2_channel_read_ex(session->scpSend_channel, 0,
+        rc = _libssh2_channel_read(session->scpSend_channel, 0,
                                    (char *) session->scpSend_response, 1);
         if (rc == PACKET_EAGAIN) {
             libssh2_error(session, LIBSSH2_ERROR_EAGAIN,
@@ -990,7 +991,7 @@ libssh2_scp_send_ex(LIBSSH2_SESSION * session, const char *path, int mode,
             memset(session->scpSend_err_msg, 0, session->scpSend_err_len + 1);
 
             /* Read the remote error message */
-            rc = libssh2_channel_read_ex(session->scpSend_channel, 0,
+            rc = _libssh2_channel_read(session->scpSend_channel, 0,
                                        session->scpSend_err_msg,
                                        session->scpSend_err_len);
             if (rc <= 0) {

src/sftp.c

@@ -167,7 +167,7 @@ sftp_packet_read(LIBSSH2_SFTP *sftp)
                        "partial read cont, len: %lu", packet_len);
     }
     else {
-        rc = libssh2_channel_read_ex(channel, 0, (char *) buffer, 4);
+        rc = _libssh2_channel_read(channel, 0, (char *) buffer, 4);
         if (rc == PACKET_EAGAIN) {
             return rc;
         }
@@ -175,6 +175,7 @@ sftp_packet_read(LIBSSH2_SFTP *sftp)
             /* TODO: this is stupid since we can in fact get 1-3 bytes in a
                legitimate working case as well if the connection happens to be
                super slow or something */
+            fprintf(stderr, "GOT %d\n", rc);
             libssh2_error(session, LIBSSH2_ERROR_CHANNEL_FAILURE,
                           "Read part of packet", 0);
             return LIBSSH2_ERROR_CHANNEL_FAILURE;
@@ -202,7 +203,7 @@ sftp_packet_read(LIBSSH2_SFTP *sftp)
     /* Read as much of the packet as we can */
     while (packet_len > packet_received) {
         bytes_received =
-            libssh2_channel_read_ex(channel, 0,
+            _libssh2_channel_read(channel, 0,
                                   (char *) packet + packet_received,
                                   packet_len - packet_received);
 
@@ -798,8 +799,7 @@ LIBSSH2_API int
 libssh2_sftp_shutdown(LIBSSH2_SFTP *sftp)
 {
     int rc;
-    BLOCK_ADJUST(rc, sftp->channel->session,
+    BLOCK_ADJUST(rc, sftp->channel->session, sftp_shutdown(sftp));
-                 sftp_shutdown(sftp));
     return rc;
 }
 
@@ -1113,7 +1113,7 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
                 sftp_packet_requirev(sftp, 2, read_responses,
                                      request_id, &data, &data_len);
             if (retcode == PACKET_EAGAIN) {
-                libssh2_error(session, LIBSSH2_ERROR_EAGAIN,
+                libssh2_error(session, retcode,
                               "Would block waiting for status message", 0);
                 return retcode;
             } else if (retcode) {