dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kio_sftp.cpp:1939
> QT_STATBUF buff;
> const bool bDestExists = (QT_STAT(QFile::encodeName(sCopyFile), &buff) != -1);
> + QFileInfo info(sCopyFile);
We're now doing stat() twice, once here, and once in QFileInfo just below.
This could be fixed by using QFileInfo for everything:
const bool bDestExists = info.exists();
> kio_sftp.cpp:1960
> if (bMarkPartial && bPartExists && buff.st_size > 0) {
> - if (S_ISDIR(buff.st_mode)) {
> + if(info.isDir()) {
> errorCode = ERR_DIR_ALREADY_EXIST;
WRONG. Here we were testing the result of stat() on the .part file, see old line 1942.
Granted, reusing "buff" didn't make the code very readable....
Create a different QFileInfo instance for the .part file, and use it for bPartExists and for isDir() here.
> kio_sftp.cpp:2037
> + if (!receivedFile.exists()) {
> + if (!receivedFile.open(QIODevice::ReadWrite | QIODevice::Text)) {
> + QString error_msg = receivedFile.errorString();
Why ReadWrite, if we know it doesn't exist?
Does setFileTime() even need open() first? I wouldn't have thought so.
> kio_sftp.cpp:2042
> + else {
> + receivedFile.open(QIODevice::ReadWrite | QIODevice::Text);
> + receivedFile.setFileTime(QDateTime::fromTime_t(buff.st_atime),
Again??
> kio_sftp.cpp:2060
> QT_STATBUF buff;
> bool bSrcExists = (QT_STAT(QFile::encodeName(sCopyFile), &buff) != -1);
> + QFileInfo info(sCopyFile);
= info.exists()
REPOSITORY
R320 KIO Extras
REVISION DETAIL
https://phabricator.kde.org/D22105
To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190709/e21dae0c/attachment-0001.html>