Subject: Re: libssh2 master 30e2881 sftp_read: use a state variable to avoid bad writes

Re: libssh2 master 30e2881 sftp_read: use a state variable to avoid bad writes

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Mon, 19 Sep 2011 22:49:35 +0200 (CEST)

On Mon, 19 Sep 2011, liuzl wrote:

> modify the goto conditions in sftp_read() to avoid NULL loop

...

> + /* If we previously set the read_state to libssh2_NB_state_sent
> + due to EAGAIN, but the caller canceled subsequent request(by close
> + handle or seek), then we will get a NULL loop in the next call,
> + give a judgement here to avoid this. */

Ah right. I can see how seek could cause this (close should not). Did you
actually see it happen and did the seek and everything else work out fine
afterwards?

But I would prefer another fix that doesn't make the code depend on a list and
a state to correlate like that.

What do you say about clearing the state more genericly instead like this:

--- a/src/sftp.c
+++ b/src/sftp.c
@@ -1210,12 +1210,13 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle,
ch
         as possible - remember that we don't block */
      chunk = _libssh2_list_first(&handle->packet_list);

+ sftp->read_state = libssh2_NB_state_idle;
+
      while(chunk) {
          if(chunk->lefttosend) {
              rc = _libssh2_channel_write(channel, 0,
                                          &chunk->packet[chunk->sent],
                                          chunk->lefttosend);
- sftp->read_state = libssh2_NB_state_idle;
              if(rc < 0) {
                  if(rc != LIBSSH2_ERROR_EAGAIN)
                      /* error */

-- 
  / daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2011-09-19