Alexander Lamaison wrote:
> The changes are more extensive that I'd planned so I'd appreciate some
> careful review. Basically, the total_read variable turned out to be
> completely pointless.
Yes. What about the loop? It's kept for the chunk stuff, so that
libssh2_sftp_read() never returns 0 bytes to the caller unless EOF?
I think it would be nice to return EAGAIN if we read 0 bytes from
transport.
> @@ -1348,13 +1334,13 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
> filep->offset_sent -= (chunk->len - rc32);
> }
>
> - if(total_read + rc32 > buffer_size) {
> + if(rc32 > buffer_size) {
> /* figure out the overlap amount */
> - filep->data_left = (total_read + rc32) - buffer_size;
> + filep->data_left = rc32 - buffer_size;
Doesn't this lose the bytes that we received from transport but which
do not fit into the buffer given to us by the user?
> + /* remove the chunk we just processed keeping track of the
> + * next one in case we need it */
> + next = _libssh2_list_next(&chunk->node);
> + _libssh2_list_remove(&chunk->node);
> + LIBSSH2_FREE(session, chunk);
> + chunk = NULL;
> +
> + if(rc32 > 0) {
> + /* we must return as we wrote some data to the buffer */
> + return rc32;
> + } else {
> + /* A zero-byte read is not necessarily EOF so we must not
> + * return 0 (that would signal EOF to the caller) so
> + * instead we carry on to the next chunk */
> + chunk = next;
> }
Here I suggest return EAGAIN instead of having the loop and chunk
messyness.
//Peter
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2012-02-09