On 9 February 2012 11:15, Peter Stuge <pe...@stuge.se> wrote:
> 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?
Yes. Originally I got rid of the loop but then realised we might read 0.
> I think it would be nice to return EAGAIN if we read 0 bytes from
> transport.
I like this idea. That would be cleaner and might remove the weird
situation where we return 0 at the end of the function without it
being EOF.
>> @@ -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?
Not unless it did so before. total_read was always 0 at this point so
the changes shouldn't make any difference.
>> + /* 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.
I'll try this out tonight.
Alex
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
