----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129134/#review99990 -----------------------------------------------------------
Good strategy overall, but it seems to me that some of the patch is incorrect. sftp/kio_sftp.cpp (line 941) <https://git.reviewboard.kde.org/r/129134/#comment67156> should probably be error? sftp/kio_sftp.cpp (line 975) <https://git.reviewboard.kde.org/r/129134/#comment67157> It seems to me that sftpLogin() already calls error() in case of an error (at least in most code paths --- if it ever doesn't do that, then that's where it should be fixed). It's wrong to call finished() after error(), so this (and the many other calls like it in your patch) should be removed, I would think. sftp/kio_sftp.cpp (line 1142) <https://git.reviewboard.kde.org/r/129134/#comment67158> Are you sure? close() is not standard SlaveBase API. sftp/kio_sftp.cpp (line 2288) <https://git.reviewboard.kde.org/r/129134/#comment67159> should be error(unsupported) - David Faure On Oct. 9, 2016, 3:33 p.m., Emmanuel Pescosta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129134/ > ----------------------------------------------------------- > > (Updated Oct. 9, 2016, 3:33 p.m.) > > > Review request for kde-workspace, Andreas Schneider and David Faure. > > > Bugs: 362988 > https://bugs.kde.org/show_bug.cgi?id=362988 > > > Repository: kio-extras > > > Description > ------- > > Fix the hanging sftp ioslave by always calling either finished() or error() > to signal the completion of the command, as stated in the ioslave API > description. > > > Diffs > ----- > > sftp/kio_sftp.cpp f6ec556 > > Diff: https://git.reviewboard.kde.org/r/129134/diff/ > > > Testing > ------- > > Works here. I use it since a couple of days without any problems so far. > > > Thanks, > > Emmanuel Pescosta > >
