Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

epicsTime: fix overflow handling #92

Closed
wants to merge 1 commit into from

Conversation

mdavidsaver
Copy link
Member

@mdavidsaver mdavidsaver commented Nov 3, 2020

As I noted on lp:1896295, epicsTimeDiffInSeconds() and class epicsTime which underpins it, do not correctly handle 32-bit overflow on targets where sizeof(long)==8 (eg. Linux).

This PR changes 'class epicsTime' to use epicsUInt32 to match 'struct epicsTimeStamp', and tries to fix the various places in epicsTime.cpp where overflow is detected. This passes existing tests, and a new test of epicsTimeDiffInSeconds() overflow.

I'm not certain that this is the best approach though. I think a rewrite would be better. Specifically to make class epicsTime a wrapper around struct epicsTimeStamp.

Change 'class epicsTime' to match 'struct epicsTimeStamp'
in use of epicsUInt32.
@mdavidsaver mdavidsaver self-assigned this Nov 3, 2020
@mdavidsaver
Copy link
Member Author

mdavidsaver commented Nov 3, 2020

Recently, while doing some benchmarks of a driver with perf, I was surprised to see epicsTimeDiffInSeconds() appear as a hot spot. Another reason I'm inclined to rewrite epicsTime*, and simplify it assuming time_t is integer seconds (as it is on all known targets).

@AppVeyorBot
Copy link

@mdavidsaver
Copy link
Member Author

Superseded by #185

@mdavidsaver mdavidsaver closed this Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants