sitter added 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 ?

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.

  R320 KIO Extras


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

Reply via email to