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