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
