Re: sftp_packet_require() patch

On Fri, 9 Aug 2013, Leif Salomonsson wrote:

Sorry for the delay of reviewing this.

>> I noticed this problem when calling sftp_statvfs() when connected to a
>> server that does not support statvfs extension. This results in the server
>> returning a STATUS instead of an EXTENDED_REPLY. Libssh2 is not prepared
>> for this, and just waits for EXTENDED_REPLY to arrive, which never happens.
>> My proposed fix is attached. It modifies sftp_packet_require() to handle
>> this specific scenario.
>
> Updated the patch file.
> A "_" character got lost from a _libssh2_debug() call..

First, please follow the existing source code indentation style we use.

Then, I really don't like having the same code chunk pasted twice into that
sftp_packet_require() function and it seems odd to put that kind of specific
logic into the generic requrie function.

Isn't it rather so that the funtion that calls sftp_packet_require() in this
case should rather ask for one of SSH_FXP_EXTENDED_REPLY _or_ SSH_FXP_STATUS ?
That's what the sftp_packet_requirev() function is for already!

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