D28478: [FileProtocol] change statx stat_dev() to use device major + minor
Méven Car
noreply at phabricator.kde.org
Wed Apr 1 11:28:05 BST 2020
meven added inline comments.
INLINE COMMENTS
> jobtest.cpp:1382
> const QUrl url(QUrl::fromLocalFile(filePath));
> - KIO::StatJob *job = KIO::stat(url, KIO::HideProgressInfo);
> + KIO::StatJob *job = KIO::statDetails(url, KIO::StatJob::SourceSide, KIO::StatDefaultDetails | KIO::StatInode);
> QVERIFY(job);
Please add a separate test dedicated for Inode like `statWithInode()` like `statSymlink` does for instance.
Here you change the purpose of this test : default stat behavior with stat(), to default + inode with statDetails.
> jobtest.cpp:1398
> +#if 1 // should be HAVE_STATX
> + QVERIFY(entry.contains(KIO::UDSEntry::UDS_CREATION_TIME));
> + QVERIFY(entry.contains(KIO::UDSEntry::UDS_DEVICE_ID));
UDS_CREATION_TIME is statx specific only on Linux
Just make it clear in the comment.
> jobtest.cpp:1400
> + QVERIFY(entry.contains(KIO::UDSEntry::UDS_DEVICE_ID));
> + QVERIFY(entry.contains(KIO::UDSEntry::UDS_INODE));
> + QCOMPARE(entry.count(), 11);
KIO::UDSEntry::UDS_DEVICE_ID and KIO::UDSEntry::UDS_INODE are not statx specific (it is supported for all UNIX systems)
> file_unix.cpp:286
> inline static uint16_t stat_mode(struct statx &buf) { return buf.stx_mode; }
> -inline static uint32_t stat_dev(struct statx &buf) { return buf.stx_dev_major; }
> +inline static uint32_t stat_dev(struct statx &buf) { return (buf.stx_dev_major * 100) + buf.stx_dev_minor; }
> inline static uint64_t stat_ino(struct statx &buf) { return buf.stx_ino; }
We should probably use `makedev` http://man7.org/linux/man-pages/man3/makedev.3.html in fact to ensure to match original stat behavior.
> file_unix.cpp:304
> inline static mode_t stat_mode(QT_STATBUF &buf) { return buf.st_mode; }
> inline static dev_t stat_dev(QT_STATBUF &buf) { return buf.st_dev; }
> inline static ino_t stat_ino(QT_STATBUF &buf) { return buf.st_ino; }
Did you compare with the no-statx case ?
We should try to have the same output.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D28478
To: ahmadsamir, #frameworks, dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200401/99d9182c/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list