D28745: Skip caching thumbnails on encrypted filesystems

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> marcingu wrote in thumbnail.cpp:776
> No. The m_thumbnailDirDeviceId is set to 0 and only changed if lstat executes properly. Then it takes value of st_dev. I don't know if st_dev can be 0 though and I couldn't find this information in manual either. If it can, we should change it.

I don't see how stat would succeed and st_dev would be 0, but I could be wrong. Maybe assert that it's not 0, at least?

> thumbnail.cpp:779
> +        if (lstat(QFile::encodeName(m_thumbBasePath).data(), &baseStat) == -1) {
> +            qCWarning(KIO_THUMBNAIL_LOG) << "Cannot read information about filesystem";
> +            return false;

print out the path in the warning (same on line 786)

> thumbnail.cpp:785
> +    struct stat fileStat;
> +    if(lstat(QFile::encodeName(path).data(), &fileStat) == -1) {
> +        qCWarning(KIO_THUMBNAIL_LOG) << "Cannot read information about filesystem";

missing space after `if`

s/data/constData/

> thumbnail.h:93
>      qint64 m_maxFileSize;
> +    dev_t m_thumbnailDirDeviceId = 0;
>  };

This will break compilation on Windows, I assume?

On Unix, is the corresponding include present in this file? The lack of context makes reviewing difficult indeed. Any chance to move this to gitlab? (see invent.kde.org)

REPOSITORY
  R320 KIO Extras

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

To: marcingu, ivan, broulik, #dolphin, ngraham, meven, bruns, dfaure
Cc: dfaure, thiago, bruns, meven, ngraham, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200819/36ade848/attachment.htm>