wait for entire buffer to be written before returning when calling sftp_write #28
Conversation
|
Hi there, Thanks for the interest and PR! The change seems good to make, have left some comments. Will also need to rebuild the and commit the C sources with Cython for changes to be applied and tested either locally or on CI. CI does not install Cython, it only builds from C files. See contribution guide. |
ssh2/sftp_handle.pyx
Outdated
| @@ -270,7 +270,12 @@ cdef class SFTPHandle: | |||
| cdef char *cbuf = buf | |||
| cdef ssize_t rc | |||
| with nogil: | |||
| rc = c_sftp.libssh2_sftp_write(self._handle, cbuf, _size) | |||
| while _size > 0: | |||
There was a problem hiding this comment.
With this change if _size is 0 rc will be undefined and will cause a crash. Can skip call to write and just return 0 in that case.
ssh2/sftp_handle.pyx
Outdated
| @@ -270,7 +270,16 @@ cdef class SFTPHandle: | |||
| cdef char *cbuf = buf | |||
| cdef ssize_t rc | |||
| with nogil: | |||
| rc = c_sftp.libssh2_sftp_write(self._handle, cbuf, _size) | |||
| while _size >= 0: | |||
| if _size == 0: | |||
There was a problem hiding this comment.
Might be better to just set cdef ssize_t rc = 0 by default and let it fall through when size > 0 fails rather than have to do conditional checks. What do you think?
There was a problem hiding this comment.
Yes, that does make more sense. Cheaper in general, will update it
|
CI errors are from docker login which does not work for PRs - tests all pass. Will make a separate change for that. |
| rc = c_sftp.libssh2_sftp_write(self._handle, cbuf, _size) | ||
| while _size > 0: | ||
| rc = c_sftp.libssh2_sftp_write(self._handle, cbuf, _size) | ||
| if rc < 0: |
There was a problem hiding this comment.
Could you elaborate on this? why not?
Would think that you would keep writing the content of 'cbuf' until it is empty
|
Looks good, thanks for the PR! |
libssh2 sftp_write does not guarantee that the entire buffer is written.
Sends up to 32K packets which are individually acked.
https://www.libssh2.org/libssh2_sftp_write.html