Add support for FileJob->truncate() in smb/sftp slaves
ClosedPublic

Authored by feverfew on Mon, Dec 23, 5:12 PM.

Details

Summary

This implements the new method added in D26148 in the sftp/smb slaves.

Test Plan

Tested sftp locally with truncate. Seems to work.
Haven't tested locally with smb though.

Diff Detail

Repository
R320 KIO Extras
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
feverfew created this revision.Mon, Dec 23, 5:12 PM
Restricted Application added projects: Dolphin, Frameworks. · View Herald TranscriptMon, Dec 23, 5:12 PM
Restricted Application added subscribers: kfm-devel, kde-frameworks-devel. · View Herald Transcript
feverfew requested review of this revision.Mon, Dec 23, 5:12 PM
feverfew updated this revision to Diff 72104.Mon, Dec 23, 5:13 PM
  • Free unneeded struct pointer
fvogt requested changes to this revision.Mon, Dec 23, 7:01 PM
fvogt added inline comments.
sftp/kio_sftp.cpp
1484

This entire block is duplicated - can that be improved?

1496

AFAICT, this has to be done in every case.

This revision now requires changes to proceed.Mon, Dec 23, 7:01 PM
feverfew added inline comments.Mon, Dec 23, 7:12 PM
sftp/kio_sftp.cpp
1496

I wasn't too sure if attr was needed by libssh if the call succeeded. Looking at the source code it seems that you are probably right and can be freed in all circumstances.

feverfew updated this revision to Diff 72154.Tue, Dec 24, 6:47 PM

Avoid free(NULL)

feverfew updated this revision to Diff 72155.Tue, Dec 24, 6:52 PM

Avoid code duplication

feverfew marked 3 inline comments as done.Tue, Dec 24, 11:04 PM
feverfew updated this revision to Diff 72181.Wed, Dec 25, 10:49 PM

Add fact that protocol supports truncation

feverfew updated this revision to Diff 72584.Wed, Jan 1, 11:58 PM

Fix introduction of BIC method

dfaure accepted this revision.Thu, Jan 2, 8:50 AM
dfaure added inline comments.
sftp/kio_sftp.cpp
1478

[minor: qDebug already appends spaces between arguments, so you get two here after the '=']

1497

self-assignment in some cases is a weird thing to do.
Why not if (errorCode==..) { errorCode = ..; } ?

sitter accepted this revision.Tue, Jan 7, 12:10 PM

I do wonder if maybe more granular return value handling of the smb truncate would be in order, but then I suppose the most relevant error is EACCES and that'd be handled at opening ¯\_(ツ)_/¯

LGTM

sftp/kio_sftp.cpp
1497

I'd just get rid of the assignment TBH. The line below is the last place errorCode is used anyway, so the ternary could just move there.

fvogt accepted this revision.Wed, Jan 8, 8:10 PM
This revision is now accepted and ready to land.Wed, Jan 8, 8:10 PM
feverfew updated this revision to Diff 73306.Sat, Jan 11, 9:24 PM

Cleanup code according to comments

feverfew updated this revision to Diff 73308.Sat, Jan 11, 9:30 PM
  • Merge branch 'master' into arcpatch-D26191
  • adhere to protcol -> json switch
This revision was automatically updated to reflect the committed changes.