On Friday 28 of August 2009 23:52:44 Daniel Stenberg wrote:
> > 1) Why has been the exception here?
>
> I don't remember, but I experience pain when I see it. It seems like a very
> weird assumption and action. I would think our waitfor bits approach should
> be a much better way to achieve something similar.
>
> > 2) Why are you going to drop it right now?
>
> Simply because that old #ifdef is related to the same problem we're working
> on, and with a "proper" fix going on I think it is also motivated to get
> rid of old dirty kludges.
Attached is my current version of the patch. There are no #ifdefs and it seems
to work. No extensive testing yet, just a simple sftp/scp download/upload
with the curl tool. I haven't actually had enough time to dive into code and
discover details causing this to work. I am only sharing my up-to-now results
to avoid duplicated work on the issue.
Kamil
? ares/compile
Index: lib/ssh.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/ssh.c,v
retrieving revision 1.136
diff -u -p -r1.136 ssh.c
--- lib/ssh.c 23 Jul 2009 02:15:00 -0000 1.136
+++ lib/ssh.c 30 Aug 2009 22:58:28 -0000
@@ -1503,6 +1503,9 @@ static CURLcode ssh_statemach_act(struct
result = Curl_setup_transfer(conn, -1, -1, FALSE, NULL,
FIRSTSOCKET, NULL);
+ /* not set by Curl_setup_transfer to preserve keepon bits */
+ conn->sockfd = conn->writesockfd;
+
if(result) {
state(conn, SSH_SFTP_CLOSE);
sshc->actualcode = result;
@@ -1914,6 +1917,12 @@ static CURLcode ssh_statemach_act(struct
else {
result = Curl_setup_transfer(conn, FIRSTSOCKET, data->req.size,
FALSE, NULL, -1, NULL);
+
+ /* not set by Curl_setup_transfer to preserve keepon bits */
+ conn->writesockfd = conn->sockfd;
+
+ /* FIXME: here should be explained why we need it to start the download */
+ conn->cselect_bits = CURL_CSELECT_IN;
}
if(result) {
state(conn, SSH_SFTP_CLOSE);
@@ -2031,7 +2040,7 @@ static CURLcode ssh_statemach_act(struct
}
/* upload data */
- result = Curl_setup_transfer(conn, -1, data->req.size, FALSE, NULL,
+ result = Curl_setup_transfer(conn, FIRSTSOCKET, data->req.size, FALSE, NULL,
FIRSTSOCKET, NULL);
if(result) {
@@ -2083,6 +2092,12 @@ static CURLcode ssh_statemach_act(struct
result = Curl_setup_transfer(conn, FIRSTSOCKET,
bytecount, FALSE, NULL, -1, NULL);
+ /* not set by Curl_setup_transfer to preserve keepon bits */
+ conn->writesockfd = conn->sockfd;
+
+ /* FIXME: here should be explained why we need it to start the download */
+ conn->cselect_bits = CURL_CSELECT_IN;
+
if(result) {
state(conn, SSH_SCP_CHANNEL_FREE);
sshc->actualcode = result;
@@ -2235,10 +2250,10 @@ static int ssh_perform_getsock(const str
sock[0] = conn->sock[FIRSTSOCKET];
- if(conn->proto.sshc.waitfor & KEEP_RECV)
+ if(conn->waitfor & KEEP_RECV)
bitmap |= GETSOCK_READSOCK(FIRSTSOCKET);
- if(conn->proto.sshc.waitfor & KEEP_SEND)
+ if(conn->waitfor & KEEP_SEND)
bitmap |= GETSOCK_WRITESOCK(FIRSTSOCKET);
return bitmap;
@@ -2282,15 +2297,17 @@ static void ssh_block2waitfor(struct con
{
struct ssh_conn *sshc = &conn->proto.sshc;
int dir;
- if(block && (dir = libssh2_session_block_directions(sshc->ssh_session))) {
+ if(!block)
+ conn->waitfor = 0;
+ else if((dir = libssh2_session_block_directions(sshc->ssh_session))) {
/* translate the libssh2 define bits into our own bit defines */
- sshc->waitfor = ((dir&LIBSSH2_SESSION_BLOCK_INBOUND)?KEEP_RECV:0) |
+ conn->waitfor = ((dir&LIBSSH2_SESSION_BLOCK_INBOUND)?KEEP_RECV:0) |
((dir&LIBSSH2_SESSION_BLOCK_OUTBOUND)?KEEP_SEND:0);
}
else
/* It didn't block or libssh2 didn't reveal in which direction, put back
the original set */
- sshc->waitfor = sshc->orig_waitfor;
+ conn->waitfor = sshc->orig_waitfor;
}
#else
/* no libssh2 directional support so we simply don't know */
Index: lib/transfer.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/transfer.c,v
retrieving revision 1.436
diff -u -p -r1.436 transfer.c
--- lib/transfer.c 21 Aug 2009 12:01:36 -0000 1.436
+++ lib/transfer.c 30 Aug 2009 22:58:29 -0000
@@ -1652,10 +1652,6 @@ CURLcode Curl_readwrite(struct connectda
if((k->keepon & KEEP_RECVBITS) == KEEP_RECV) {
fd_read = conn->sockfd;
-#if defined(USE_LIBSSH2)
- if(conn->protocol & (PROT_SCP|PROT_SFTP))
- select_res |= CURL_CSELECT_IN;
-#endif /* USE_LIBSSH2 */
} else
fd_read = CURL_SOCKET_BAD;
@@ -1884,33 +1880,39 @@ Transfer(struct connectdata *conn)
return CURLE_OK;
while(!done) {
- curl_socket_t fd_read;
- curl_socket_t fd_write;
+ curl_socket_t fd_read = conn->sockfd;
+ curl_socket_t fd_write = conn->writesockfd;
+ int keepon = k->keepon;
+
+ if(conn->waitfor) {
+ /* if waitfor is set, get the RECV and SEND bits from that but keep the
+ other bits */
+ keepon &= ~ (KEEP_RECV|KEEP_SEND);
+ keepon |= conn->waitfor & (KEEP_RECV|KEEP_SEND);
+ }
/* limit-rate logic: if speed exceeds threshold, then do not include fd in
select set. The current speed is recalculated in each Curl_readwrite()
call */
- if((k->keepon & KEEP_SEND) &&
+ if((keepon & KEEP_SEND) &&
(!data->set.max_send_speed ||
(data->progress.ulspeed < data->set.max_send_speed) )) {
- fd_write = conn->writesockfd;
k->keepon &= ~KEEP_SEND_HOLD;
}
else {
fd_write = CURL_SOCKET_BAD;
- if(k->keepon & KEEP_SEND)
+ if(keepon & KEEP_SEND)
k->keepon |= KEEP_SEND_HOLD; /* hold it */
}
- if((k->keepon & KEEP_RECV) &&
+ if((keepon & KEEP_RECV) &&
(!data->set.max_recv_speed ||
(data->progress.dlspeed < data->set.max_recv_speed)) ) {
- fd_read = conn->sockfd;
k->keepon &= ~KEEP_RECV_HOLD;
}
else {
fd_read = CURL_SOCKET_BAD;
- if(k->keepon & KEEP_RECV)
+ if(keepon & KEEP_RECV)
k->keepon |= KEEP_RECV_HOLD; /* hold it */
}
Index: lib/urldata.h
===================================================================
RCS file: /cvsroot/curl/curl/lib/urldata.h,v
retrieving revision 1.418
diff -u -p -r1.418 urldata.h
--- lib/urldata.h 24 Aug 2009 12:57:25 -0000 1.418
+++ lib/urldata.h 30 Aug 2009 22:58:29 -0000
@@ -568,7 +568,6 @@ struct ssh_conn {
LIBSSH2_CHANNEL *ssh_channel; /* Secure Shell channel handle */
LIBSSH2_SFTP *sftp_session; /* SFTP handle */
LIBSSH2_SFTP_HANDLE *sftp_handle;
- int waitfor; /* current READ/WRITE bits to wait for */
int orig_waitfor; /* default READ/WRITE bits wait for */
/* note that HAVE_LIBSSH2_KNOWNHOST_API is a define set in the libssh2.h
@@ -1075,6 +1074,8 @@ struct connectdata {
} proto;
int cselect_bits; /* bitmask of socket events */
+ int waitfor; /* current READ/WRITE bits to wait for */
+
#if defined(HAVE_GSSAPI) || defined(USE_WINDOWS_SSPI)
int socks5_gssapi_enctype;
#endif
