D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-30 Thread Alexander Saoutkin
This revision was automatically updated to reflect the committed changes.
Closed by commit R320:28fdc41f1d45: Fixing implementation of FileJob interface 
in smb/sftp slaves (authored by feverfew).

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23207?vs=63876=64977

REVISION DETAIL
  https://phabricator.kde.org/D23207

AFFECTED FILES
  sftp/kio_sftp.cpp
  smb/kio_smb.h
  smb/kio_smb_file.cpp

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: bcooksley, kde-frameworks-devel, kfm-devel, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-29 Thread Ben Cooksley
bcooksley removed a subscriber: fsitter.

REPOSITORY
  R320 KIO Extras

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: bcooksley, kde-frameworks-devel, kfm-devel, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov, fsitter


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-29 Thread Fuk Sitter
fsitter added a comment.


  You think sending your minions to insult me and then disabling my account 
will solve the issue? what will you do next? Disable registration so no one 
points out your hypocrisy? Which overlord made the decision to disable my 
account and for what?

REPOSITORY
  R320 KIO Extras

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: fsitter, kde-frameworks-devel, kfm-devel, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-29 Thread Fuk Sitter
fsitter added a comment.


  You think sending your minions to insult me and then disabling my account 
will solve the issue? what will you do next? Disable registration so no one 
points out your hypocrisy? Which overlord made the decision to disable my 
account and for what?

REPOSITORY
  R320 KIO Extras

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: fsitter, kde-frameworks-devel, kfm-devel, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-29 Thread Fuk Sitter
fsitter added a comment.


  You think sending your minions to insult me and then disabling my account 
will solve the issue? what will you do next? Disable registration so no one 
points out your hypocrisy? Which overlord made the decision to disable my 
account and for what?

REPOSITORY
  R320 KIO Extras

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: fsitter, kde-frameworks-devel, kfm-devel, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-28 Thread Harald Sitter
sitter added a comment.


  @dfaure this is actually a different beast altogether, the current state 
validation is only run on the API for all slaves, seek/read/write are run in a 
completely different loop which I see isn't being verified at all. I'll add it 
in a separate diff once the assertion is in. Should be fairly trivial. As I 
understand it open/read/write/seek must only ever call error() or their 
specific "send" function (opened()/data()/written()/position()) but never 
finished. Close meanwhile must reach finality (finished or error).

REPOSITORY
  R320 KIO Extras

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-28 Thread David Faure
dfaure added a comment.


  OK, I'm wrong, it doesn't check that at the moment. But IIRC your slavebase 
patch does that meanwhile ;)

REPOSITORY
  R320 KIO Extras

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-28 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Well I guess these fixes came to be exactly *because* SlaveBase warns about 
finished() being called when it shouldn't :-)

REPOSITORY
  R320 KIO Extras

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-27 Thread Harald Sitter
sitter added a comment.


  SlaveBase (which the slaves derive from) runs a command loop, out of this 
command loop come the actual calls to the API functions. So, in said command 
loop we can verify which state the slave is in before and more importantly 
after any API call. Specifically about the defects you are repairing here I'd 
guess that none of the commands running on an open connection must result in 
the finished state. We already do this for (some) of the other commands.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-27 Thread Alexander Saoutkin
feverfew added a comment.


  In D23207#520233 , @sitter wrote:
  
  > Not knowing the background here at a glance I would argue that SlaveBase in 
KIO should be getting state verification on all of this,.
  
  
  Sorry, I'm not sure what you mean by "state verification" in this case and 
how it relates to SlaveBase.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-27 Thread Harald Sitter
sitter added a comment.


  Not knowing the background here at a glance I would argue that SlaveBase in 
KIO should be getting state verification on all of this,.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-26 Thread Alexander Saoutkin
feverfew added reviewers: sitter, dfaure.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt, sitter, dfaure
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-16 Thread Alexander Saoutkin
feverfew updated this revision to Diff 63876.
feverfew added a comment.


  Unnecessary min version upgraded

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23207?vs=63875=63876

BRANCH
  fixSFTP (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D23207

AFFECTED FILES
  sftp/kio_sftp.cpp
  smb/kio_smb.h
  smb/kio_smb_file.cpp

To: feverfew, chinmoyr, fvogt
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D23207: Fixing implementation of FileJob interface in smb/sftp slaves

2019-08-16 Thread Alexander Saoutkin
feverfew retitled this revision from "Fixing finished() called after error() in 
smb slave. Do not call finished() in open/read/write/seek operations" to 
"Fixing implementation of FileJob interface in smb/sftp slaves".
feverfew edited the summary of this revision.
feverfew edited the test plan for this revision.
feverfew added reviewers: chinmoyr, fvogt.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D23207

To: feverfew, chinmoyr, fvogt
Cc: kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov