Cleaned up sftp_read and added more explanation.

Replaced the gotos which were implementing the state machine with
a switch statement which makes the states more explicit.
This commit is contained in:
Alexander Lamaison 2012-02-07 16:23:11 +00:00 committed by Daniel Stenberg
parent 9836b0889f
commit 0d824e5702

View File

@ -1089,11 +1089,47 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
struct _libssh2_sftp_handle_file_data *filep = struct _libssh2_sftp_handle_file_data *filep =
&handle->u.file; &handle->u.file;
/* This function can be interrupted in three different places where it
might need to wait for data from the network. It returns EAGAIN to
allow non-blocking clients to do other work but these client are
expected to call this function again (possibly many times) to finish
the operation.
The tricky part is that if we previously aborted a sftp_read due to
EAGAIN, we must continue at the same spot to continue the previously
interrupted operation. This is done using a state machine to record
what phase of execution we were at. The state is stored in
sftp->read_state.
libssh2_NB_state_idle: The first phase is where we prepare multiple
FXP_READ packets to do optimistic read-ahead. We send off as many as
possible in the second phase without waiting for a response to each
one; this is the key to fast reads. But we may have to adjust the
channel window size to do this which may interrupt this function while
waiting. The state machine saves the phase as libssh2_NB_state_idle so
it returns here on the next call.
libssh2_NB_state_sent: The second phase is where we send the FXP_READ
packets. Writing them to the channel can be interrupted with EAGAIN
but the state machine ensures we skip the first phase on the next call
and resume sending.
libssh2_NB_state_sent2: In the third phase (indicated by ) we read the
data from the responses that have arrived so far. Reading can be
interrupted with EAGAIN but the state machine ensures we skip the first
and second phases on the next call and resume sending.
*/
/* Number of bytes asked for that haven't been acked yet */ /* Number of bytes asked for that haven't been acked yet */
size_t already = (filep->offset_sent - filep->offset); size_t already = (filep->offset_sent - filep->offset);
switch (sftp->read_state) {
case libssh2_NB_state_idle:
/* Some data may already have been read from the server in the
previous call but didn't fit in the buffer at the time. We can
start by adding that to the buffer. */
if(filep->data_left) { if(filep->data_left) {
/* data left from previous call */
size_t copy = MIN(buffer_size, filep->data_left); size_t copy = MIN(buffer_size, filep->data_left);
memcpy(buffer, &filep->data[ filep->data_len - filep->data_left], memcpy(buffer, &filep->data[ filep->data_len - filep->data_left],
@ -1110,15 +1146,8 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
filep->data = NULL; filep->data = NULL;
} }
/* if we previously aborted a sftp_read due to EAGAIN, we must continue at /* We allow a number of bytes being requested at any given time
the same spot to continue the previously aborted operation */ without having been acked - until we reach EOF. */
if(sftp->read_state == libssh2_NB_state_sent)
goto send_read_requests;
else if(sftp->read_state == libssh2_NB_state_sent2)
goto read_acks;
/* We allow a number of bytes being requested at any given time without
having been acked - until we reach EOF. */
if(!filep->eof) { if(!filep->eof) {
size_t max_read_ahead = buffer_size*4; size_t max_read_ahead = buffer_size*4;
unsigned long recv_window; unsigned long recv_window;
@ -1126,28 +1155,29 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
if(max_read_ahead > LIBSSH2_CHANNEL_WINDOW_DEFAULT*4) if(max_read_ahead > LIBSSH2_CHANNEL_WINDOW_DEFAULT*4)
max_read_ahead = LIBSSH2_CHANNEL_WINDOW_DEFAULT*4; max_read_ahead = LIBSSH2_CHANNEL_WINDOW_DEFAULT*4;
/* if the buffer_size passed in now is smaller than what has already /* if the buffer_size passed in now is smaller than what has
been sent, we risk getting count become a very large number */ already been sent, we risk getting count become a very large
number */
if(max_read_ahead > already) if(max_read_ahead > already)
count = max_read_ahead - already; count = max_read_ahead - already;
/* 'count' is how much more data to ask for, and 'already' is how much /* 'count' is how much more data to ask for, and 'already' is how
data that already has been asked for but not yet returned. much data that already has been asked for but not yet returned.
Specificly, 'count' means how much data that have or will be asked Specificly, 'count' means how much data that have or will be
for by the nodes that are already added to the linked list. Some of asked for by the nodes that are already added to the linked
those read requests may not actually have been sent off list. Some of those read requests may not actually have been
successfully yet. sent off successfully yet.
If 'already' is very large it should be perfectly fine to have If 'already' is very large it should be perfectly fine to have
count set to 0 as then we don't have to ask for more data (right count set to 0 as then we don't have to ask for more data
now). (right now).
buffer_size*4 is just picked more or less out of the air. The idea buffer_size*4 is just picked more or less out of the air. The
is that when reading SFTP from a remote server, we send away idea is that when reading SFTP from a remote server, we send
multiple read requests guessing that the client will read more than away multiple read requests guessing that the client will read
only this 'buffer_size' amount of memory. So we ask for maximum more than only this 'buffer_size' amount of memory. So we ask
buffer_size*4 amount of data so that we can return them very fast for maximum buffer_size*4 amount of data so that we can return
in subsequent calls. them very fast in subsequent calls.
*/ */
recv_window = libssh2_channel_window_read_ex(sftp->channel, recv_window = libssh2_channel_window_read_ex(sftp->channel,
@ -1166,6 +1196,8 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
0, NULL); 0, NULL);
/* if this returns EAGAIN, we will get back to this function /* if this returns EAGAIN, we will get back to this function
at next call */ at next call */
assert(rc != LIBSSH2_ERROR_EAGAIN || !filep->data_left);
assert(rc != LIBSSH2_ERROR_EAGAIN || !filep->eof);
if (rc) if (rc)
return rc; return rc;
} }
@ -1208,19 +1240,19 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
to create more packets */ to create more packets */
} }
send_read_requests: case libssh2_NB_state_sent:
/* move through the READ packets that haven't been sent and send as many
as possible - remember that we don't block */
chunk = _libssh2_list_first(&handle->packet_list);
sftp->read_state = libssh2_NB_state_idle; sftp->read_state = libssh2_NB_state_idle;
/* move through the READ packets that haven't been sent and send as
many as possible - remember that we don't block */
chunk = _libssh2_list_first(&handle->packet_list);
while(chunk) { while(chunk) {
if(chunk->lefttosend) { if(chunk->lefttosend) {
if(total_read) if(total_read)
/* since we risk getting EAGAIN below, we return here if there /* since we risk getting EAGAIN below, we return here if
is data available */ there is data available */
return total_read; return total_read;
rc = _libssh2_channel_write(channel, 0, rc = _libssh2_channel_write(channel, 0,
@ -1244,7 +1276,7 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
chunk = _libssh2_list_next(&chunk->node); chunk = _libssh2_list_next(&chunk->node);
} }
read_acks: case libssh2_NB_state_sent2:
sftp->read_state = libssh2_NB_state_idle; sftp->read_state = libssh2_NB_state_idle;
@ -1262,8 +1294,8 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
}; };
if(chunk->lefttosend) if(chunk->lefttosend)
/* if the chunk still has data left to send, we shouldn't wait for /* if the chunk still has data left to send, we shouldn't wait
an ACK for it just yet */ for an ACK for it just yet */
break; break;
if(total_read) if(total_read)
@ -1279,14 +1311,14 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
} }
/* /*
* We get DATA or STATUS back. STATUS can be error, or it is FX_EOF * We get DATA or STATUS back. STATUS can be error, or it is
* when we reach the end of the file. * FX_EOF when we reach the end of the file.
*/ */
switch (data[0]) { switch (data[0]) {
case SSH_FXP_STATUS: case SSH_FXP_STATUS:
/* we must remove all outstanding READ requests, as either we got /* we must remove all outstanding READ requests, as either we
an error or we're at end of file */ got an error or we're at end of file */
sftp_packetlist_flush(handle); sftp_packetlist_flush(handle);
rc32 = _libssh2_ntohu32(data + 5); rc32 = _libssh2_ntohu32(data + 5);
@ -1310,9 +1342,9 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
"SFTP Protocol badness"); "SFTP Protocol badness");
if(rc32 != chunk->len) { if(rc32 != chunk->len) {
/* a short read does not imply end of file, but we must adjust /* a short read does not imply end of file, but we must
the offset_sent since it was advanced with a full adjust the offset_sent since it was advanced with a
chunk->len before */ full chunk->len before */
filep->offset_sent -= (chunk->len - rc32); filep->offset_sent -= (chunk->len - rc32);
} }
@ -1331,8 +1363,8 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
else else
filep->data_len = 0; filep->data_len = 0;
/* copy the received data from the received FXP_DATA packet to the /* copy the received data from the received FXP_DATA packet to
buffer at the correct index */ the buffer at the correct index */
memcpy(buffer + total_read, data + 9, rc32); memcpy(buffer + total_read, data + 9, rc32);
filep->offset += rc32; filep->offset += rc32;
total_read += rc32; total_read += rc32;
@ -1359,9 +1391,10 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
chunk = next; chunk = next;
} }
break;
if(! total_read) { default:
fprintf(stderr, "MOO\n"); assert(!"State machine error; unrecognised read state");
} }
return total_read; return total_read;