Issue1189 SFTP FSETATTTR fix + timeout 0 support #1190
Conversation
petersilva
requested review from
reidsunderland,
MagikEh,
iMacadan,
andreleblanc11 and
mshak2
Test Results247 tests 245 ✅ 1m 34s ⏱️ For more details on these failures, see this check. Results for commit 7fb6e28. ♻️ This comment has been updated with latest results. |
The default value is listed in the man page is zero, which does not match the source code (300) so changed that. Also previous patch enabled use of 0 to disable timeouts, so documented that behaviour.
petersilva
changed the title
There was a problem hiding this comment.
I'm confused by the change to alarm_cancel?
The change adds if time <= 0: return which changes the code from cancelling the alarm to doing nothing? And if a time > 0 is passed in, it would cancel the previous alarm but set a new one that will cause a signal to be sent after time seconds.
signal.alarm(time)
If time is non-zero, this function requests that a SIGALRM signal be sent to the process in time seconds. Any previously scheduled alarm is canceled (only one alarm can be scheduled at any time). The returned value is then the number of seconds before any previously set alarm was to have been delivered. If time is zero, no alarm is scheduled, and any scheduled alarm is canceled. If the return value is zero, no alarm is currently scheduled.
|
The change is symmetric. If you cancel a non-existent alarm... that's an error. so when I changed the thing setting the alarm to... not do that... the cancel has to match... I don't know why one would want to cancel an alarm that we didn't set... how do we know? |
|
This branch will never be merged. Kept here as a place holder, reminder for a future branch.
|
close #1189
These are changes that deal with sftp servers.
passes flow tests, but No tested with a server that restricts the permissions.
@iMacadan perhaps you can test this in sending to such a client.