D22105: WIP : Fix SFTP Plugin of KIO for Windows

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Almost there :-)

INLINE COMMENTS

> kio_sftp.cpp:406
> +#ifdef Q_OS_WIN
> +            // TODO Check if this works for other OSes too.
> +            fileType = QT_STAT_LNK;

Yes, QT_STAT_LNK works on Unix too, we use it in many places in KIO
(and it has the value S_IFLNK)

So I'm pretty sure you can remove this ifdef.

You could also replace S_IFREG with QT_STAT_REG and S_IFDIR with QT_STAT_DIR, for consistency.

> kio_sftp.cpp:2032
>          }
> -        else if (QT_STAT( QFile::encodeName(sPart), &buff ) == 0) { // should a very small ".part" be deleted?
> +        else if (partFile.exists()) { // should a very small ".part" be deleted?
>              const int size = config()->readEntry("MinimumKeepSize", DEFAULT_MINIMUM_KEEP_SIZE);

At this point we were doing another stat() in order to see how big the part file is *after* sftpGet.
You're reusing the old QFileInfo so that won't contain updated data.

You need to call QFileInfo::refresh() first.

> kio_sftp.cpp:2050
> +                else {
> +                    receivedFile.setFileTime(partFile.fileTime(QFileDevice::FileAccessTime),
> +                                            QFileDevice::FileAccessTime);

Sorry I gave wrong advice.
The old code was preserving the atime just because the utime() API forces us to specify an atime.
But what we want to do here is set the modification time, nothing else.
We don't care about the access time at all.
Please just remove this line.

> albertvaka wrote in kio_sftp.cpp:2257
> You can keep QT_STAT_LNK on every platform and remove the ifdef. That's the point of the abstraction Qt provides.
> 
> For consistency, I would also change the other (S_IFDIR, etc.) to their Qt counterparts.

This is marked as Done, but it's not Done. In fact it matches my own suggestion above, so I agree with Albert ;-)

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190721/b08db4bf/attachment.htm>