D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  In D20096#453194 , @dfaure wrote:
  
  > I'm talking about jobtest and kdirmodeltest regressing exactly in build 87, 
which is where this commit landed.
  >  If you click on history for a given test you can see that those aren't 
sporadic failures: 
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/junit/projectroot/autotests/kiowidgets_kdirmodeltest/history/
  > and
  >  
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/junit/projectroot/autotests/kiocore_jobtest/history/
  >
  > The failures in kfileitemtest and kdirlistertest (note: model != lister) 
are unrelated, I'll look into those.
  
  
  Thank you for making this clear.
  
  The regression are associated with symlink resolution.
  I think I have identified the issue.
  Please see D20694  and could we test this 
on CI.
  
  It makes me think that for some reviews, it would be good to test it on CI 
before merging.
  At my past place of work we used this extensively, it was quite helpful.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[David Faure]

dfaure added a comment.


  I'm talking about jobtest and kdirmodeltest regressing exactly in build 87, 
which is where this commit landed.
  If you click on history for a given test you can see that those aren't 
sporadic failures: 
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/junit/projectroot/autotests/kiowidgets_kdirmodeltest/history/
  and
  
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/junit/projectroot/autotests/kiocore_jobtest/history/
  
  The failures kdirlistertest and kfileitemtest are unrelated, and indeed look 
like they come from the switch to Qt 5.12, I'll look into those.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  In D20096#451170 , @dfaure wrote:
  
  > This change also introduced regressions in two unittests: jobtest and 
kdirmodeltest. I'll let you look into those :-)
  >
  > 
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/junit/projectroot/autotests/kiocore_jobtest/
  >  
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/projectroot/autotests/kiowidgets_kdirmodeltest/
  >
  > The first one is probably just an extra file in the dir (I know, bad 
isolation of the tests), the second one is more unexpected.
  
  
  The tests failures are not regressions as they are happened before the merge 
: 
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.12/72/testReport/
 although on Qt 5.12.
  
  Failures are:
  
  FAIL!  : KFileItemTest::testListProperties(two (text+html) files) Compared 
values are not the same
  
Actual   (props.mimeGroup()): ""
Expected (expectedMimeGroup): "text"
Loc: [/home/jenkins/workspace/Frameworks/kio/kf5-qt5 
SUSEQt5.10/autotests/kfileitemtest.cpp(590)]
  
  I don't reproduce locally.
  
  FAIL!  : KDirListerTest::testRenameItem() Compared values are not the same
  
Actual   (entry.second.mimetype()): "application/xhtml+xml"
Expected (QString("text/html"))   : "text/html"
Loc: [/home/jenkins/workspace/Frameworks/kio/kf5-qt5 
SUSEQt5.10/autotests/kdirlistertest.cpp(557)]
  
  I didn't reproduce locally, but I could see some issue in the test.
  Here is a fix proposal for this test : D20692 

  Is it possible to test it on jenkins before merging ?
  
  So it seems to me as sporadic failures of slightly unconsistent tests.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[David Faure]

dfaure added a comment.


  This change also introduced regressions in two unittests: jobtest and 
kdirmodeltest. I'll let you look into those :-)
  
  
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/junit/projectroot/autotests/kiocore_jobtest/
  
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/87/testReport/projectroot/autotests/kiowidgets_kdirmodeltest/
  
  The first one is probably just an extra file in the dir (I know, bad 
isolation of the tests), the second one is more unexpected.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  In D20096#451148 , @bcooksley 
wrote:
  
  > MSVC doesn't like this change i'm afraid - see 
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.11/259/console
  >  Looks like you're making use of C++17 features?
  
  
  I am a bit surprised. I used std:c++11 as for the rest of current kde compli
  
  In D20096#451150 , @dfaure wrote:
  
  > Nah, no C++17 here, the compiler is just confused because it doesn't know 
the types uid_t and gid_t.
  >
  > I'll add some #ifndef Q_OS_WIN.
  
  
  
  
  In D20096#451150 , @dfaure wrote:
  
  > Nah, no C++17 here, the compiler is just confused because it doesn't know 
the types uid_t and gid_t.
  >
  > I'll add some #ifndef Q_OS_WIN.
  
  
  @bcooksley 
  Could you test https://phabricator.kde.org/D20599 ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[David Faure]

dfaure added a comment.


  Nah, no C++17 here, the compiler is just confused because it doesn't know the 
types uid_t and gid_t.
  
  I'll add some #ifndef Q_OS_WIN.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Ben Cooksley]

bcooksley added a comment.


  MSVC doesn't like this change i'm afraid - see 
https://build.kde.org/view/OS%20-%20Windows/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.11/259/console
  Looks like you're making use of C++17 features?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  There is still a couple raw QT_LSTAT in KFileItem 
https://cgit.kde.org/kio.git/tree/src/core/kfileitem.cpp#n224 .
  Do you think we should apply the same kind of solution there, with #idef ?
  
  I would be in favor of reusing code instead.
  Eventually moving FileProtocol::createUDSEntry out of FileProtocol in some 
other util so that it can be reused by KFileitem.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  In D20096#449198 , @bruns wrote:
  
  > Thanks for working on this!
  
  
  I am very glad :)
  
  Thank you for your great reviews. I learned quite a bit along the way.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:f3b0f220a78d: Fill UDSEntry::UDS_CREATION_TIME under 
linux when glibc = 2.28 (authored by meven).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55967=56137

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Stefan Brüns]

bruns accepted this revision.
bruns added a comment.


  I think its better go give it sufficient testing time than to wait for more 
comments. The code has been around for sufficient time, and the last changes 
were just minor.
  
  Thanks for working on this!

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Fabian Vogt]

fvogt accepted this revision.
fvogt added a comment.


  Code still looks good to me - I can't comment on the cmake parts though.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  Any new feedback ? @fvogt @bruns  @dfaure ?

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked 2 inline comments as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> bruns wrote in file.cpp:890
> This is wrong in case someone uses details > 3, should be `case 0: 
> reserve(5)`, `case 3: default: reserve(15)` .
> all checks below do e.g `if (details > 2)`, so handle 5 the same as 3.

Thanks

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55967.
meven added a comment.


  Fix switch

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55850=55967

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> file.cpp:890
> +break;
> +default: // case 0:
> +// filename, access, type, size, linkdest

This is wrong in case someone uses details > 3, should be `case 0: reserve(5)`, 
`case 3: default: reserve(15)` .
all checks below do e.g `if (details > 2)`, so handle 5 the same as 3.

> file.cpp:939
> +#endif
> +auto bufferSize = qBound(lowerBound, size +1, higherBound);
>  QByteArray linkTargetBuffer;

missing space, `size + 1`

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  In D20096#446742 , @ngraham wrote:
  
  > So are we ready to land this, or is there anything left to do?
  
  
  Are there some more feedback ?
  
  I would appreciate a second accept.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> pino wrote in file.cpp:850-870
> No, he means using a const& for the argument, e.g:
> 
>   inline static uint16_t stat_mode(struct statx ) { return buf.stx_mode; }

> @pino 
>  No, he means using a const& for the argument, e.g:

I think he meant both.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked 2 inline comments as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> pino wrote in file.cpp:850-870
> No, he means using a const& for the argument, e.g:
> 
>   inline static uint16_t stat_mode(struct statx ) { return buf.stx_mode; }

Thanks

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55850.
meven added a comment.


  Fix src/ioslaves/file/ConfigureChecks.cmake

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55849=55850

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55849.
meven added a comment.


  Use passing references to stat_xxx accessors, use a signed long long for file 
size

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55709=55849

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the test plan for this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> meven wrote in file.cpp:850-870
> Unfortunately this is not possible here : statx is also a function, the 
> compiler gets messed up when removing the struct keyword interpreting it as a 
> function call.
> 
> > /file/file.cpp:850:34: warning: inline variables are only available with 
> > -std=c++17 or -std=gnu++17
> > 
> >   377   │  inline static uint16_t stat_mode(statx buf) { return 
> > buf.stx_mode; }

No, he means using a const& for the argument, e.g:

  inline static uint16_t stat_mode(struct statx ) { return buf.stx_mode; }

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked 2 inline comments as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked 2 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> meven wrote in file.cpp:850-870
> Do you mean the struct keyword in the argument in "inline static uint16_t 
> stat_mode(struct statx buf) { return buf.stx_mode; } " for instance ?

Unfortunately this is not possible here : statx is also a function, the 
compiler gets messed up when removing the struct keyword interpreting it as a 
function call.

> /file/file.cpp:850:34: warning: inline variables are only available with 
> -std=c++17 or -std=gnu++17
> 
>   377   │  inline static uint16_t stat_mode(statx buf) { return buf.stx_mode; 
> }

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in file.cpp:850-870
> We can get all stat_xxx functions buf as reference, struct is not needed when 
> you use C++.

Do you mean the struct keyword in the argument in "inline static uint16_t 
stat_mode(struct statx buf) { return buf.stx_mode; } " for instance ?

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Nathaniel Graham]

ngraham added a dependent revision: D20404: Allow the baloo widgets to display 
creation date and access date..

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> file.cpp:850-870
> +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 uint64_t stat_ino(struct statx buf) { return buf.stx_ino; }
> +inline static uint64_t stat_size(struct statx buf) { return buf.stx_size; }
> +inline static uint32_t stat_uid(struct statx buf) { return buf.stx_uid; }
> +inline static uint32_t stat_gid(struct statx buf) { return buf.stx_gid; }
> +inline static uint64_t stat_atime(struct statx buf) { return 
> buf.stx_atime.tv_sec; }

We can get all stat_xxx functions buf as reference, struct is not needed when 
you use C++.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: anthonyfieroni, pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, 
bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Nathaniel Graham]

ngraham added a comment.


  So are we ready to land this, or is there anything left to do?

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked 2 inline comments as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> pino wrote in ConfigureChecks.cmake:19
> sorry, i realized that my sentence was ambiguous: i meant to check for the 
> existance of statx no matter the platform

We discussed previously that we would restrict using statx to the platform 
where it was tested i.e GLIBC to avoid future bugs with different potential 
different implementation of statx.
https://phabricator.kde.org/D20096#440921

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> pino wrote in ConfigureChecks.cmake:19
> just do the test unconditionally

sorry, i realized that my sentence was ambiguous: i meant to check for the 
existance of statx no matter the platform

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55709.
meven marked 4 inline comments as done.
meven added a comment.


  review feedback

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55683=55709

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> ConfigureChecks.cmake:19
> +
> +if (LIBC_IS_GLIBC)
> +check_cxx_source_compiles("

just do the test unconditionally

> ConfigureChecks.cmake:24
> +int main() {
> +  int t = STATX_BASIC_STATS;
> +  return 0;

other than this define, try to do a simple usage of statx:

  struct statx buf;
  statx(AT_FDCWD, "/foo", AT_EMPTY_PATH, STATX_BASIC_STATS, );

(it will not be run anyway)

> file.cpp:68
>  
> +#if HAVE_STATX && defined Q_OS_LINUX
> +#include 

no need for the Q_OS_LINUX check here

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[David Faure]

dfaure added a comment.


  The month of testing starts today. Code pushed to master starting from today 
will end up in 5.58.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> ConfigureChecks.cmake:19
> +
> +if (LIBC_IS_GLIBC)
> +check_cxx_source_compiles("

I am not sure this is acceptable, this variable is set by ECM in 
extra-cmake-modules/kde-modules/KDECompilerSettings.cmake

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55683.
meven added a comment.


  Remove reference to old differential revision from the commit comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55679=55683

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added a comment.


  In D20096#445410 , @ngraham wrote:
  
  > BTW Frameworks 5.57 has been tagged, so whenever folks are good with this, 
it can be landed at any time. :)
  
  
  I think this would be best to wait for this for 5.58 to have a month of 
testing as you suggested.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> pino wrote in file.cpp:72-74
> TBH, instead of this static define, I'd do a proper cmake check (see 
> ConfigureChecks.cmake, and config-kioslave-file.h.cmake in src/ioslaves/file).

It was not too easy to do.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55679.
meven added a comment.


  Use proper cmake check, fix an issue with KIO::UDSEntry::UDS_DEVICE_ID having 
value of stat_mode instead of stat_dev

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55644=55679

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/ConfigureChecks.cmake
  src/ioslaves/file/config-kioslave-file.h.cmake
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Nathaniel Graham]

ngraham added a comment.


  BTW Frameworks 5.57 has been tagged, so whenever folks are good with this, it 
can be landed at any time. :)

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Fabian Vogt]

fvogt accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> file.cpp:72-74
> +#if defined(STATX_BASIC_STATS) && defined(__GLIBC__)
> +#define USE_STATX
> +#endif

TBH, instead of this static define, I'd do a proper cmake check (see 
ConfigureChecks.cmake, and config-kioslave-file.h.cmake in src/ioslaves/file).

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55644.
meven added a comment.


  Updating commit message

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55539=55644

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the summary of this revision.
meven edited the test plan for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Nathaniel Graham]

ngraham added a comment.


  Yeah, we should land this after 5.57 tagging so we have a month of 
pre-release testing, so it should be `FIXED-IN: 5.58`.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Stefan Brüns]

bruns added a comment.


  I think the code itself is good to go now.
  
  Can you please update the summary somewhat (and better do it directly in 
phabricator, as arc will throw away any changes in the commit message, unless 
you do a  - `arc amend`, update the commit message in git, `arc diff 
--verbatim` - cycle).
  
  1. describe the dolphin screenshot textually
  2. update the FIXED-IN, unless it lands before KF 5.57 is tagged
  3. in the test plan, say what **is** tested
  4. probably better first sentence:
  
  > The birthtime can be retrieved using using the statx function, available 
since glibc 2.28. In case the kernel lacks support for the underlying syscall, 
glibc falls back to stat internally. The validity of the btime field is 
indicated in the mask field, e.g. when the kernel or the filesystem lacks 
support for btime. For details, see .

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Fabian Vogt]

fvogt added a comment.


  Looking good to me, @bruns: any addiitonal comments?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> fvogt wrote in file.cpp:1033
> This check seems to be wrong with me - there can be files with legitimate 
> zero `tv_nsec`.
> 
> Use `buff.stx_mask & STATX_BTIME` instead.

Thanks for the feedback, I read about stx_mask after writing this.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55539.
meven marked an inline comment as done.
meven added a comment.


  Use stx_mask to check for btime availability

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55183=55539

BRANCH
  arcpatch-D20096

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Fabian Vogt]

fvogt requested changes to this revision.
fvogt added a comment.
This revision now requires changes to proceed.


  Looks good to me otherwise.

INLINE COMMENTS

> file.cpp:1033
> +/* And linux version using statx syscall */
> +if (buff.stx_btime.tv_nsec > 0) {
> +entry.fastInsert(KIO::UDSEntry::UDS_CREATION_TIME, 
> buff.stx_btime.tv_sec);

This check seems to be wrong with me - there can be files with legitimate zero 
`tv_nsec`.

Use `buff.stx_mask & STATX_BTIME` instead.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  Any suggestion ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  After this patch I intend to update KFileItemPrivate::init to add birthtime 
there in the same manner.
  Perhaps it would be good to group the stax/QL_STAT code somewhere rather than 
spread.
  
  And then there is baloo-widgets and perhaps some dolphin work for this to 
flow to the UIs, or even gwenview.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55183.
meven marked an inline comment as done.
meven added a comment.


  Fix position of default in switch statement

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55182=55183

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> bruns wrote in file.cpp:932
> statx.stx_size is __u64, and readlink uses (unsigned) size_t for the buffer 
> size.

Let me know if I have handled this correctly.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55182.
meven added a comment.


  Add AT_SYMLINK_NOFOLLOW flag to statx call matching lstat behavior, use 
size_t type for readlink parameter, avoid a warning correct signedness of statx 
struct accessors

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55147=55182

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> file.cpp:932
> +int64_t lowerBound = 256;
> +int64_t higherBound = 1024;
> +#else

statx.stx_size is __u64, and readlink uses (unsigned) size_t for the buffer 
size.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55147.
meven added a comment.


  Use time_t instead of __time_t and fix higherBound

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55146=55147

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55146.
meven added a comment.


  Handle properly respective types for stat and statx syscalls

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55132=55146

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55132.
meven added a comment.


  Avoid including twice errno.h

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55124=55132

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked 2 inline comments as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added inline comments.

INLINE COMMENTS

> fvogt wrote in file.cpp:71
> This won't work as expected, `USE_STATX` is unconditionally defined.

Thanks

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55124.
meven added a comment.


  Conditionnaly define USE_STATX

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55123=55124

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Fabian Vogt]

fvogt added a comment.


  In D20096#440923 , @meven wrote:
  
  > In D20096#440921 , @fvogt wrote:
  >
  > > In D20096#440919 , @meven 
wrote:
  > >
  > > > So unless I am mistaken, I feel this is not a great concern.
  > > >  I would perhaps need to restrict when statx is used even when 
`STATX_BASIC_STATS` is defined to when __GLIBC__ is defined as well.
  > >
  > >
  > > Yes, please do that.
  >
  >
  > Will do.
  >
  > And thinking again about the issue, could we have kio compiled with glibc 
but running on a system with musl for instance ?
  >  If it is possible, then I need to treat this case as you suggested to 
handle the runtime dependency on glibc.
  
  
  I don't think that's possible, no.

INLINE COMMENTS

> file.cpp:71
> +#include 
> +#define USE_STATX STATX_BASIC_STATS && __GLIBC__
> +#endif

This won't work as expected, `USE_STATX` is unconditionally defined.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55123.
meven added a comment.


  Restrict statx use when c standard library is Glibc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55120=55123

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  In D20096#440921 , @fvogt wrote:
  
  > In D20096#440919 , @meven wrote:
  >
  > > > There are platforms out there which don't use glibc. So I suggest 
either handling ENOSYS properly by falling back to stat or erroring out if 
statx is supported but __GLIBC__ not defined.
  > >
  > > If the platform does not use glibc, `STATX_BASIC_STATS` will be false and 
statx won't be called, the other existing code path will be used.
  > >  `STATX_BASIC_STATS` implies GLIBC and statx availability at least until 
other C standard libraries support it.
  >
  >
  > The "until" is the issue here - code that will knowingly break in the 
future is simply not acceptable.
  >
  > > So unless I am mistaken, I feel this is not a great concern.
  > >  I would perhaps need to restrict when statx is used even when 
`STATX_BASIC_STATS` is defined to when __GLIBC__ is defined as well.
  >
  > Yes, please do that.
  
  
  Will do.
  
  And thinking again about the issue, could we have kio compiled with glibc but 
running on a system with musl for instance ?
  If it is possible, then I need to treat this case as you suggested to handle 
the runtime dependency on glibc.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Fabian Vogt]

fvogt added a comment.


  In D20096#440919 , @meven wrote:
  
  > In D20096#440868 , @fvogt wrote:
  >
  > > In D20096#440842 , @meven 
wrote:
  > >
  > > > In D20096#440830 , @pino 
wrote:
  > > >
  > > > > Note that, even if the system supports statx() (so with glibc >= 
2.28), you must support systems without it at runtime anyway: for example, if 
you boot with a kernel older than 4.11 (which IIRC is the version where the 
syscall was added) then the glibc function will return ENOSYS (IIRC, better 
check). This can happen for example in containers: you boot a Debian testing 
container (so with glibc 2.28) on a Debian stable system (with Linux 4.9).
  > > >
  > > >
  > > > This case is covered by glibc 
https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/unix/sysv/linux/statx.c
  > > >  In case the syscall does not exist, glibc provides a generic 
implementation based on stat.
  > >
  > >
  > > There are platforms out there which don't use glibc. So I suggest either 
handling ENOSYS properly by falling back to stat or erroring out if statx is 
supported but __GLIBC__ not defined.
  >
  >
  > If the platform does not use glibc, `STATX_BASIC_STATS` will be false and 
statx won't be called, the other existing code path will be used.
  >  `STATX_BASIC_STATS` implies GLIBC and statx availability at least until 
other C standard libraries support it.
  
  
  The "until" is the issue here - code that will knowingly break in the future 
is simply not acceptable.
  
  > So unless I am mistaken, I feel this is not a great concern.
  >  I would perhaps need to restrict when statx is used even when 
`STATX_BASIC_STATS` is defined to when __GLIBC__ is defined as well.
  
  Yes, please do that.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added a comment.


  In D20096#440868 , @fvogt wrote:
  
  > In D20096#440842 , @meven wrote:
  >
  > > In D20096#440830 , @pino wrote:
  > >
  > > > Note that, even if the system supports statx() (so with glibc >= 2.28), 
you must support systems without it at runtime anyway: for example, if you boot 
with a kernel older than 4.11 (which IIRC is the version where the syscall was 
added) then the glibc function will return ENOSYS (IIRC, better check). This 
can happen for example in containers: you boot a Debian testing container (so 
with glibc 2.28) on a Debian stable system (with Linux 4.9).
  > >
  > >
  > > This case is covered by glibc 
https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/unix/sysv/linux/statx.c
  > >  In case the syscall does not exist, glibc provides a generic 
implementation based on stat.
  >
  >
  > There are platforms out there which don't use glibc. So I suggest either 
handling ENOSYS properly by falling back to stat or erroring out if statx is 
supported but __GLIBC__ not defined.
  
  
  If the platform does not use glibc, `STATX_BASIC_STATS` will be false and 
statx won't be called, the other existing code path will be used.
  `STATX_BASIC_STATS` implies GLIBC and statx availability at least until other 
C standard libraries support it.
  So unless I am mistaken, I feel this is not a great concern.
  I would perhaps need to restrict when statx is used even when 
`STATX_BASIC_STATS` is defined to when __GLIBC__ is defined as well.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55120.
meven added a comment.


  Remove unused includes

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55111=55120

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Fabian Vogt]

fvogt added a comment.


  In D20096#440842 , @meven wrote:
  
  > In D20096#440830 , @pino wrote:
  >
  > > Note that, even if the system supports statx() (so with glibc >= 2.28), 
you must support systems without it at runtime anyway: for example, if you boot 
with a kernel older than 4.11 (which IIRC is the version where the syscall was 
added) then the glibc function will return ENOSYS (IIRC, better check). This 
can happen for example in containers: you boot a Debian testing container (so 
with glibc 2.28) on a Debian stable system (with Linux 4.9).
  >
  >
  > This case is covered by glibc 
https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/unix/sysv/linux/statx.c
  >  In case the syscall does not exist, glibc provides a generic 
implementation based on stat.
  
  
  There are platforms out there which don't use glibc. So I suggest either 
handling ENOSYS properly by falling back to stat or erroring out if statx is 
supported but __GLIBC__ not defined.
  
  > The code also takes into account that the birthtime field still depends on 
the filesystem used and this information can be missing even on ext4, line 
1015. Although I haven't tested it.
  > 
  > Qt is adding support for statx in Qt 5.13 
https://github.com/qt/qtbase/commit/d3393ce25833c0afd7f0fa6b85fd6f3bd7ad520a
  
  Qt already supports statx since 5.10 - the linked commit is part of the 
v5.10.0-alpha1 tag.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.
meven added a comment.


  In D20096#440830 , @pino wrote:
  
  > Note that, even if the system supports statx() (so with glibc >= 2.28), you 
must support systems without it at runtime anyway: for example, if you boot 
with a kernel older than 4.11 (which IIRC is the version where the syscall was 
added) then the glibc function will return ENOSYS (IIRC, better check). This 
can happen for example in containers: you boot a Debian testing container (so 
with glibc 2.28) on a Debian stable system (with Linux 4.9).
  
  
  This case is covered by glibc 
https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/unix/sysv/linux/statx.c
  In case the syscall does not exist, glibc provides a generic implementation 
based on stat.
  
  The code also takes into account that the birthtime field still depends on 
the filesystem used and this information can be missing even on ext4, line 
1015. Although I haven't tested it.
  
  Qt is adding support for statx in Qt 5.13 
https://github.com/qt/qtbase/commit/d3393ce25833c0afd7f0fa6b85fd6f3bd7ad520a

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Pino Toscano]

pino added a comment.


  Note that, even if the system supports statx() (so with glibc >= 2.28), you 
must support systems without it at runtime anyway: for example, if you boot 
with a kernel older than 4.11 (which IIRC is the version where the syscall was 
added) then the glibc function will return ENOSYS (IIRC, better check). This 
can happen for example in containers: you boot a Debian testing container (so 
with glibc 2.28) on a Debian stable system (with Linux 4.9).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: pino, bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked 2 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> bruns wrote in file.cpp:878
> You should start with a more sensible size for lowerLimit, e.g. 256, maybe 
> more - the scope is local, i.e. any excess size is hardly relevant, and 
> allocating 1 and 256 byte cost the same.
> 
> lowerLimit is not used below, I think you don't have to name it.
> 
> You should use (buff.stx_size + 1), as the size is without trailing null byte.

I have 256 as I'd rather allocate the minimal size without adding cost here.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55111.
meven marked 2 inline comments as done.
meven added a comment.


  Use a better lower limit for the buffer size, add some comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55110=55111

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55110.
meven added a comment.


  Add details == 2 and correct switch

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55082=55110

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> bruns wrote in file.cpp:880
> `switch (details) { ...}`

You are off by 1 for dev/inode, these are only added with details >= 3.

details >= 2 is ACL data. As the ACL are commonly empty, add +0.

Handle 0 explicitly, and make 3 (>=3) the default, to match the remaining code.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55082.
meven added a comment.


  Use switch statement, +1 to the qbound comparison of the buffer size

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55079=55082

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Ben Cooksley]

bcooksley added a comment.


  In D20096#440451 , @dfaure wrote:
  
  > In D20096#440440 , @bruns wrote:
  >
  > > 2. Tell the application where the slave library is located: 
`QT_PLUGIN_PATH=...` There **must** be a subdirectory `kf5/kio/` containing the 
slave plugin `file.so` below the plugin path
  >
  >
  > ECM used to do this automatically btw, until 7d6d5b1e705f54a 
 
commented it out because it broke CI - should be investigated again, because 
re-reading the commit log, I fail to see how *appending* to the path would lead 
to things not being found
  
  
  If memory serves this was a Windows specific failure too.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> file.cpp:880
>  assert(entry.count() == 0); // by contract :-)
> -entry.reserve(8);
> -
> +if (details > 0) {
> +// uid, gid, atime, mtime, btime

`switch (details) { ...}`

> file.cpp:922
>  // Use readlink on Unix because symLinkTarget turns relative 
> targets into absolute (#352927)
> -const off_t lowerLimit = 1;
> -const off_t upperLimit = 1024;
> -size_t bufferSize = qBound(lowerLimit, buff.st_size, upperLimit);
> +size_t bufferSize = qBound(1, stat_size(buff), 1024);
>  QByteArray linkTargetBuffer;

stat_size + 1. Otherwise, you will always take the `n == bufferSize` path below 
at least once.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: bcooksley, ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven updated this revision to Diff 55079.
meven added a comment.


  Rename function get_stat to STAT

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20096?vs=55077=55079

BRANCH
  creation-date

REVISION DETAIL
  https://phabricator.kde.org/D20096

AFFECTED FILES
  src/ioslaves/file/file.cpp
  tests/kioslavetest.cpp

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> bruns wrote in file.cpp:855
> This can be up to 12 now, if I counted correctly. Conditionalize this on 
> details.

This code was just copied, but I will be happy to take care of this along the 
way.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28

[Méven Car]

meven retitled this revision from "[WIP/help wanted] Fill 
UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28" to "Fill 
UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28".
meven edited the test plan for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20096

To: meven, #frameworks, dfaure, fvogt, bruns, broulik
Cc: ngraham, kde-frameworks-devel, michaelh, bruns