Re: sftp_packet_require() patch

Hello Daniel,

On 2013-08-27, you wrote:
> 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.

Right.

> 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!

OK, it looks like sftp_packet_requirev() might solve the problem.
I will try that..

Regards
Leif

_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2013-08-30