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
