From 85198c1cdb54f156212ee18bdf768fa43e035d61 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sun, 30 Aug 2009 17:00:49 +0200 Subject: [PATCH] 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 | 72 +++++++++++++++++++++------------------------- src/channel.h | 12 ++++++++ src/libssh2_priv.h | 2 +- src/publickey.c | 7 +++-- src/scp.c | 43 +++++++++++++-------------- src/sftp.c | 14 ++++----- 6 files changed, 78 insertions(+), 72 deletions(-) diff --git a/src/channel.c b/src/channel.c index d1c5906..b9c32f4 100644 --- a/src/channel.c +++ b/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,8 +1708,8 @@ 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, - char *buf, size_t buflen) +ssize_t _libssh2_channel_read(LIBSSH2_CHANNEL *channel, int stream_id, + char *buf, size_t buflen) { LIBSSH2_SESSION *session = channel->session; int rc; @@ -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, - "Remote end has closed this channel", 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; - } + + /* if the transport layer said EAGAIN then we say so as well */ + return (rc == PACKET_EAGAIN)?rc: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, - buf, buflen)); + BLOCK_ADJUST(rc, channel->session, + _libssh2_channel_read(channel, stream_id, buf, buflen)); return rc; } @@ -2057,8 +2044,8 @@ _libssh2_channel_write(LIBSSH2_CHANNEL *channel, int stream_id, if (channel->write_state == libssh2_NB_state_created) { rc = _libssh2_transport_write(session, channel->write_packet, - channel->write_s - - channel->write_packet); + channel->write_s - + channel->write_packet); if (rc == PACKET_EAGAIN) { _libssh2_debug(session, LIBSSH2_DBG_CONN, "libssh2_transport_write returned EAGAIN"); @@ -2274,19 +2261,22 @@ channel_close(LIBSSH2_CHANNEL * channel) } } - /* set the local close state first when we're perfectly confirmed to not - do any more EAGAINs */ - channel->local.close = 1; + 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; - /* We call the callback last in this function to make it keep the local - data as long as EAGAIN is returned. */ - if (channel->close_cb) { - LIBSSH2_CHANNEL_CLOSE(session, channel); + /* We call the callback last in this function to make it keep the local + data as long as EAGAIN is returned. */ + if (channel->close_cb) { + LIBSSH2_CHANNEL_CLOSE(session, channel); + } + + channel->close_state = libssh2_NB_state_idle; } - 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; - - if (rc) { + else if (rc < 0) { channel->free_state = libssh2_NB_state_idle; - return -1; + return rc; } } diff --git a/src/channel.h b/src/channel.h index 5141153..19b5457 100644 --- a/src/channel.h +++ b/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 */ diff --git a/src/libssh2_priv.h b/src/libssh2_priv.h index cfb8262..800bf9c 100644 --- a/src/libssh2_priv.h +++ b/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 */ diff --git a/src/publickey.c b/src/publickey.c index 6f087aa..e5ed944 100644 --- a/src/publickey.c +++ b/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,8 +191,8 @@ 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, - pkey->receive_packet_len); + rc = _libssh2_channel_read(channel, 0, (char *) pkey->receive_packet, + pkey->receive_packet_len); if (rc == PACKET_EAGAIN) { return rc; } else if (rc != (int)pkey->receive_packet_len) { diff --git a/src/scp.c b/src/scp.c index 05ff4ca..6908755 100644 --- a/src/scp.c +++ b/src/scp.c @@ -39,6 +39,8 @@ #include #include +#include "channel.h" + /* Max. length of a quoted string after libssh2_shell_quotearg() processing */ #define libssh2_shell_quotedsize(s) (3 * strlen(s) + 2) @@ -378,10 +380,10 @@ 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, - (char *) session-> - scpRecv_response + - session->scpRecv_response_len, 1); + rc = _libssh2_channel_read(session->scpRecv_channel, 0, + (char *) session-> + scpRecv_response + + session->scpRecv_response_len, 1); if (rc == PACKET_EAGAIN) { libssh2_error(session, LIBSSH2_ERROR_EAGAIN, "Would block waiting for SCP response", 0); @@ -415,9 +417,9 @@ 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, - session->scpRecv_err_msg, - session->scpRecv_err_len); + rc = _libssh2_channel_read(session->scpRecv_channel, 0, + session->scpRecv_err_msg, + session->scpRecv_err_len); if (rc <= 0) { /* * Since we have alread started reading this packet, @@ -591,10 +593,10 @@ 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, - (char *) session-> - scpRecv_response + - session->scpRecv_response_len, 1); + rc = _libssh2_channel_read(session->scpRecv_channel, 0, + (char *) session-> + scpRecv_response + + session->scpRecv_response_len, 1); if (rc == PACKET_EAGAIN) { libssh2_error(session, LIBSSH2_ERROR_EAGAIN, "Would block waiting for SCP response", 0); @@ -857,8 +859,8 @@ 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, - (char *) session->scpSend_response, 1); + rc = _libssh2_channel_read(session->scpSend_channel, 0, + (char *) session->scpSend_response, 1); if (rc == PACKET_EAGAIN) { libssh2_error(session, LIBSSH2_ERROR_EAGAIN, "Would block waiting for response from remote", 0); @@ -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, - (char *) session->scpSend_response, - 1); + rc = _libssh2_channel_read(session->scpSend_channel, 0, + (char *) session->scpSend_response, 1); if (rc == PACKET_EAGAIN) { libssh2_error(session, LIBSSH2_ERROR_EAGAIN, "Would block waiting for response", 0); @@ -962,8 +963,8 @@ 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, - (char *) session->scpSend_response, 1); + rc = _libssh2_channel_read(session->scpSend_channel, 0, + (char *) session->scpSend_response, 1); if (rc == PACKET_EAGAIN) { libssh2_error(session, LIBSSH2_ERROR_EAGAIN, "Would block waiting for response", 0); @@ -990,9 +991,9 @@ 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, - session->scpSend_err_msg, - session->scpSend_err_len); + rc = _libssh2_channel_read(session->scpSend_channel, 0, + session->scpSend_err_msg, + session->scpSend_err_len); if (rc <= 0) { /* * Since we have alread started reading this packet, it is diff --git a/src/sftp.c b/src/sftp.c index 33ae87f..e1f11ff 100644 --- a/src/sftp.c +++ b/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,9 +203,9 @@ 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, - (char *) packet + packet_received, - packet_len - packet_received); + _libssh2_channel_read(channel, 0, + (char *) packet + packet_received, + packet_len - packet_received); if (bytes_received == PACKET_EAGAIN) { /* @@ -798,8 +799,7 @@ LIBSSH2_API int libssh2_sftp_shutdown(LIBSSH2_SFTP *sftp) { int rc; - BLOCK_ADJUST(rc, sftp->channel->session, - sftp_shutdown(sftp)); + BLOCK_ADJUST(rc, sftp->channel->session, 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) {