D28478: [FileProtocol] change statx stat_dev() to use device major + minor

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> meven wrote in jobtest.cpp:1382
> 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.

Righto. Done.

> meven wrote in jobtest.cpp:1398
> UDS_CREATION_TIME is statx specific only on Linux
> Just make it clear in the comment.

In the new unit test, I think using HAVE_STATX is self-explanatory (I'll leave JobTest::stat() alone in this diff to keep the commit atomic).

> meven wrote in file_unix.cpp:286
> We should probably use `makedev` http://man7.org/linux/man-pages/man3/makedev.3.html in fact to ensure to match original stat behavior.

I am not an expert on stat low-level system calls; but if the goal is to have a unique UDS_DEVICE_ID, we could leave it as uint32 which is what statx returns by default, it's system specific anyway, and a system will have statx or not...

> meven wrote in file_unix.cpp:304
> Did you compare with the no-statx case ?
> We should try to have the same output.

This buf.st_dev is the decimal part in:
$ stat /usr/bin/file | grep Device
Device: 804h/2052d Inode: 9168 Links: 1
that would be "2052"

whereas with statx it returns buf.stx_dev_major which is an uint32_t.

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/a15b417d/attachment-0001.html>