On Tue, 4 Aug 2009, Alexander Lamaison wrote:
$ git bisect good
b95fe985af3c80a2babcaaaf7da69a15b1237c49 is first bad commit
commit b95fe985af3c80a2babcaaaf7da69a15b1237c49
Okay, so here comes some more testing and thougths.
First, I attach a patch that is a reversal of the mentioned commit - EXCEPT
two important edits (see below).
It would be very interesting to hear how this patch changes behavior, or not.
This reverts my code that makes it treat a short read as the end of data to
read, and goes back to always read until EAGAIN basically. I have a suspicion
that this won't make anything different.
This reversion patch leaves two changes in: the clearing of the direction bits
for EAGAIN situations. And as Peter noticed before, we did spot a case where
none of the bits were set even though libssh2 returned EAGAIN so quite
possibly this forced clearing is what triggered the problem. This will be
noticable if the problem remains after this patch has been applied.
A second test would of course to not apply this test, but to simply remove the
default clearing of the direction bits and see if that makes things run
better.
If the clearing of the bits is what triggers this problem, then we really need
to investigate how on earth libssh2 can return EAGAIN without setting the
bits. A theory: we call a function that returns a pointer and then in the
BLOCK_ADJUST_ERRNO macro, libssh2_session_last_errno() is called to see if the
reason for the NULL was an EAGAIN situation. Perhaps that function wrongly
returns EAGAIN simply because it hasn't been cleared properly since the
previous situation when EAGAIN was returned?
--
/ daniel.haxx.se
diff --git a/src/transport.c b/src/transport.c
index 119c007..ade3826 100644
--- a/src/transport.c
+++ b/src/transport.c
@@ -272,7 +272,6 @@ _libssh2_transport_read(LIBSSH2_SESSION * session)
unsigned char block[MAX_BLOCKSIZE];
int blocksize;
int encrypted = 1;
- int loop = 1;
int status;
/* default clear the bit */
@@ -297,17 +296,17 @@ _libssh2_transport_read(LIBSSH2_SESSION * session)
* is done!
*/
_libssh2_debug(session, LIBSSH2_DBG_TRANS, "Redirecting into the"
- " key re-exchange");
+ " key re-exchange");
status = libssh2_kex_exchange(session, 1, &session->startup_key_state);
if (status == PACKET_EAGAIN) {
libssh2_error(session, LIBSSH2_ERROR_EAGAIN,
- "Would block exchanging encryption keys", 0);
+ "Would block exchanging encryption keys", 0);
return PACKET_EAGAIN;
- } else if (status) {
- libssh2_error(session, LIBSSH2_ERROR_KEX_FAILURE,
- "Unable to exchange encryption keys",0);
- return LIBSSH2_ERROR_KEX_FAILURE;
- }
+ } else if (status) {
+ libssh2_error(session, LIBSSH2_ERROR_KEX_FAILURE,
+ "Unable to exchange encryption keys",0);
+ return LIBSSH2_ERROR_KEX_FAILURE;
+ }
}
/*
@@ -351,7 +350,6 @@ _libssh2_transport_read(LIBSSH2_SESSION * session)
/* If we have less than a blocksize left, it is too
little data to deal with, read more */
ssize_t nread;
- size_t recv_amount;
/* move any remainder to the start of the buffer so
that we can do a full refill */
@@ -364,12 +362,11 @@ _libssh2_transport_read(LIBSSH2_SESSION * session)
p->readidx = p->writeidx = 0;
}
- recv_amount = PACKETBUFSIZE - remainbuf;
-
/* now read a big chunk from the network into the temp buffer */
nread =
_libssh2_recv(session->socket_fd, &p->buf[remainbuf],
- recv_amount, LIBSSH2_SOCKET_RECV_FLAGS(session));
+ PACKETBUFSIZE - remainbuf,
+ LIBSSH2_SOCKET_RECV_FLAGS(session));
if (nread <= 0) {
/* check if this is due to EAGAIN and return the special
return code if so, error out normally otherwise */
@@ -380,10 +377,6 @@ _libssh2_transport_read(LIBSSH2_SESSION * session)
}
return PACKET_FAIL;
}
- else if(nread != (ssize_t)recv_amount) {
- /* we're done for this time! */
- loop = 0;
- }
debugdump(session, "libssh2_transport_read() raw",
&p->buf[remainbuf], nread);
@@ -581,11 +574,12 @@ _libssh2_transport_read(LIBSSH2_SESSION * session)
}
p->total_num = 0; /* no packet buffer available */
+
return rc;
}
- } while (loop); /* loop until done */
+ } while (1); /* loop */
- return rc;
+ return PACKET_FAIL; /* we never reach this point */
}
static libssh2pack_t
@@ -815,6 +809,3 @@ _libssh2_transport_write(LIBSSH2_SESSION * session, unsigned char *data,
return PACKET_NONE; /* all is good */
}
-
-
-
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
