On Tue, 13 Sep 2011, liuzl wrote:

The patch make sftp_read() like this:

sftp_read() {
1,window_adjust;
2,make READ packet;
3,send READ packet;
4,receive ACK packet;
}

In the first call, it works fine. and we will send blocking in the step 3 and receive blocking step 4. In the second call, if the receiving_window is small, we will always return in the step 1.

Exactly. But are you saying that is good or bad? Do you see any remaining problems with SFTP downloads after this has been applied?

The "return in the step 1" is only valid for the cases where libssh2 can't completely send the window adjust packet over the channel, which shouldn't happen _that_ often. And when it does, it really needs to send it so we just have to live with it.

In the other hand, libssh2 have many kind of internal-paket and different levels of API. It's a hard work to modify all the functions.

Actually, that's not really true. libssh2 does not have that many internal channel-using functions that send or receive data that require channel window. And for those that do, I still think that explicitly handling the window for those cases could end up being a good thing since the window size requirement isn't the same for all cases so having a generic window size handling isn't making perfect sense either.

I think we should deal user-data and internal-data in different way. Deal the internal-data in high priority. If we are blocking in sending user-data,and we try to send internal-data now, we can try to send the left half user-data, and then send the internal-data. That is not a BAT_USE.

I don't understand. How is it _not_ bad use?

If a partial channel packet have been sent before EAGAIN is returned (as EAGAIN means that not everything was sent that was intended to get sent), how can you then insert another packet without first completing the previously half-sent packet and _then_ send the internal data?

My patch removes this problem for SCP and SFTP reading, I think. It will probably require some further modifications too but I've not seen any other patch that properly fixes this problem.

--

 / daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel

Reply via email to