D27153: port sftp to Result system to force serialization of error/finish condition

Harald Sitter noreply at phabricator.kde.org
Fri Feb 14 15:41:01 GMT 2020


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 actually worked. that line was introduced as `KDE_lseek` and that too was just an alias for lseek :|
Also lseek doesn't even errno EAGAIN according to the manpage. I expect seeking as a whole needs fixing, I'll record a bug.

> feverfew wrote in kio_sftp.cpp:302
> `if (result.success) { ...} else { ... } seems clearer to me

That function is a verbatim copy from ftp so I'd rather not change it.

> feverfew wrote in kio_sftp.cpp:328
> I'm a bit confused, why is this necessary?

Kinda unrelated to the diff, I only changed it because this class has way too many type coercions.
`i` goes into `ssh_userauth_kbdint_getprompt` which has an unsigned signature, the counter `n` coming out of `ssh_userauth_kbdint_getnprompts` is signed though. The explicit cast silences the warning of the type coercion between int and unsigned.

Mind you, I think n<1 cannot even happen. The server challenged us, so there's always going to be at least one prompt, so the fact that it returns a signed prompt count is silly to begin with.

> feverfew wrote in kio_sftp.h:77
> TODO KF6 https://en.cppreference.com/w/cpp/language/attributes/nodiscard ?

I guess, that applies to all slaves though. ftp already has it and smb is also going to get it at some point. If we want a todo comment I'd put it in the ftp code. Though TBH I think that should just be a clazy check or deprecation inside Qt when built with c++17.

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, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200214/0caac065/attachment.htm>


More information about the kfm-devel mailing list