On Mon, 12 Mar 2012, Matthew Booth wrote:
seems to be down to an unchecked error in _libssh2_channel_write():
/* drain the incoming flow first, mostly to make sure we get all
* pending window adjust packets */
do
rc = _libssh2_transport_read(session);
while (rc > 0);
if(channel->local.window_size <= 0)
/* there's no room for data so we stop */
return (rc==LIBSSH2_ERROR_EAGAIN?rc:0);
In my case, _libssh2_transport_read is returning LIBSSH2_ERROR_SOCKET_RECV
(don't know why yet). The result of this in the above code is that
_libssh2_channel_write returns 0, throwing away the error information. My
code spots a zero return, checks for eof, doesn't find one and goes round
again: infinite loop.
I would just submit a patch which adds "if (rc<0) return rc;" immediately
after the read loop, but it seems like the author has intentionally not done
this here. Git commit 3c71ad4fce745876f7e7ad33b846518f2d50edf6 and its
associated mailing list thread doesn't shed any light on why only EAGAIN is
handled. Can anybody explain?
I think I only added code to handle the EAGAIN case since that was the
particular case I was out to fix and I played safe and didn't modify the other
cases.
But I agree that it looks wrong. I would probably suggest this change as a
slight variation of your suggestion:
diff --git a/src/channel.c b/src/channel.c
index 8d6fb0a..9e29492 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -2008,6 +2008,9 @@ _libssh2_channel_write(LIBSSH2_CHANNEL *channel, int
strea
rc = _libssh2_transport_read(session);
while (rc > 0);
+ if((rc < 0) && (rc != LIBSSH2_ERROR_EAGAIN))
+ return rc;
+
if(channel->local.window_size <= 0)
/* there's no room for data so we stop */
return (rc==LIBSSH2_ERROR_EAGAIN?rc:0);
The existing code continues even if LIBSSH2_ERROR_EAGAIN was returned as long
as there is window size to go with so we should probably continue doing so.
--
/ daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
