CollectionScheduler: port to std::chrono to avoid the year-2038 bug
ClosedPublic

Authored by dfaure on Apr 7 2019, 5:22 PM.

Details

Test Plan

Test still passes. I did not try changing the date to 2039 ;)

Diff Detail

Repository
R165 Akonadi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dfaure requested review of this revision.Apr 7 2019, 5:22 PM
dfaure created this revision.
pino added a subscriber: pino.Apr 7 2019, 5:29 PM
pino added inline comments.
src/server/collectionscheduler.cpp
224–227

isn't this still problematic (i.e. the cast to int)?

dfaure added a comment.Apr 7 2019, 7:50 PM

Oops, yes.

dfaure updated this revision to Diff 55708.Apr 7 2019, 8:00 PM

Remove int cast, thanks Pino!

dvratil added inline comments.Apr 8 2019, 11:01 AM
src/server/collectionscheduler.h
60

I'm tempted to use std::chrono::time_point here instead of qint64, which is a better abstraction for time units.

You can create a time_point from std::chrono::secods(QDateTime::currentSecsSinceEpoch()), and a difference of two time_points is a std::chrono::duration which can be passed directly to QTimer without having to care about conversion to correct units.

If you don't feel up to it (I remember you mentioned you are not much familiar with std::chrono), I'm fine with this version too, I can enhance it later when I get time. Getting this tested and fixed is more important.

dfaure updated this revision to Diff 55965.Apr 11 2019, 6:48 AM

Port to std::chrono.

dfaure updated this revision to Diff 55966.Apr 11 2019, 6:50 AM

update comment

dvratil requested changes to this revision.Apr 12 2019, 8:21 AM
dvratil added inline comments.
autotests/server/collectionschedulertest.cpp
54–55

Add using namespace std::chrono_literals to the top, then you can use 4m instead of having to spell out the whole type (also elsewhere in the code).

This revision now requires changes to proceed.Apr 12 2019, 8:21 AM
dfaure updated this revision to Diff 56117.Apr 13 2019, 8:46 AM

Port to chrono_literals, even if it makes QtCreator 4.8 barf on the syntax ;)

Restricted Application added a project: KDE PIM. · View Herald TranscriptApr 13 2019, 8:46 AM
dfaure updated this revision to Diff 56247.Apr 14 2019, 9:27 PM
dfaure retitled this revision from CollectionScheduler: port to qint64 to avoid the year-2038 bug to CollectionScheduler: port to std::chrono to avoid the year-2038 bug.
dfaure removed a subscriber: pino.

fix commit log

dvratil accepted this revision.Apr 15 2019, 8:08 AM

Thanks! (you should go fix QtCreator now ;-))

This revision is now accepted and ready to land.Apr 15 2019, 8:08 AM
This revision was automatically updated to reflect the committed changes.