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
