This revision was automatically updated to reflect the committed changes.
Closed by commit R320:74850eb21654: sftp: port to Result system to force
serialization of error/finish condition (authored by sitter).
REPOSITORY
R320 KIO Extras
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D2
feverfew accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R320 KIO Extras
BRANCH
sftp-errors
REVISION DETAIL
https://phabricator.kde.org/D27153
To: sitter, dfaure, feverfew
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n,
cbl
dfaure accepted this revision.
REPOSITORY
R320 KIO Extras
REVISION DETAIL
https://phabricator.kde.org/D27153
To: sitter, dfaure, feverfew
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n,
cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven,
mich
sitter updated this revision to Diff 76300.
sitter added a comment.
fix problems found by dfaure
I've also added Q_REQUIRED_RESULT to fail() and pass() to help not forget to
do something with
the return value. there's no reason to call them and ignore the constructed
object
REPOSITORY
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
Nice work! Found 2 bugs though.
INLINE COMMENTS
> kio_sftp.cpp:1306
> rc = ssh_channel_poll(mSftp->channel, 1);
> -}
> -
> -if (rc < 0) {
> +} else if (rc < 0
sitter added inline comments.
INLINE COMMENTS
> feverfew wrote in kio_sftp.cpp:145
> Technically unrelated to your changes, but does this even work? isn't the
> check supposed to be against `errno` in this case, not offset?
True enough. I expect, like many things in the slaves, this never actua
sitter updated this revision to Diff 75698.
sitter marked 2 inline comments as done.
sitter added a comment.
s/LL/U
call finished() from frontend close() (internal close cannot finish and
cannot error). " // must call finish(), which will set d->inOpenLoop=false"
one of many expectation
feverfew requested changes to this revision.
feverfew added a comment.
This revision now requires changes to proceed.
Good stuff, I'll admit I kind of skimmed over the bits that were rewrites
`error(...); return;` -> `Result::fail()`, I assume you've mapped correctly
there. Only minor commen
sitter created this revision.
sitter added reviewers: dfaure, feverfew.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.
REVISION SUMMARY
the Result system was originally introduced to the FTP slave a