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

Detect if include/cctz/time_zone.h:join_seconds() would overflow the output time_point #199

Open
devbww opened this issue Oct 7, 2021 · 2 comments

Comments

@devbww
Copy link
Contributor

devbww commented Oct 7, 2021

template <typename Rep, std::intmax_t Denom>
bool join_seconds(
    const time_point<seconds>& sec, const femtoseconds& fs,
    time_point<std::chrono::duration<Rep, std::ratio<1, Denom>>>* tpp)

needs to return false when its result is unrepresentable, and without invoking UB.

@devjgm
Copy link
Contributor

devjgm commented Oct 7, 2021

I don't see this function in the code. Link?

@devbww
Copy link
Contributor Author

devbww commented Oct 7, 2021

Yeah ... I have to create the issue before merging the code so that I can refer to the issue from the code. So, soon.

devbww added a commit to devbww/cctz that referenced this issue Oct 7, 2021
When `cctz::parse()` produces seconds+femtoseconds values that cannot
be represented in the output `time_point<D>` it should return `false`.
Here we add overloads of `join_seconds()` that break the overflow issue
down into four cases, three of which are implemented and have test cases.

The fourth overload (for 1/N duration ratios) is yet to be implemented,
so its test cases are currently commented out, but they provide a guide
to what remains to be done.  See google#199.

Also, clarify that the `ToUnixSeconds()` implementation requires that
the `std::chrono::system_clock` uses the Unix epoch in order to avoid
arithmetic overflow.
devbww added a commit that referenced this issue Oct 8, 2021
…200)

When `cctz::parse()` produces seconds+femtoseconds values that cannot
be represented in the output `time_point<D>` it should return `false`.
Here we add overloads of `join_seconds()` that break the overflow issue
down into four cases, three of which are implemented and have test cases.

The fourth overload (for 1/N duration ratios) is yet to be implemented,
so its test cases are currently commented out, but they provide a guide
to what remains to be done.  See #199.

Also, clarify that the `ToUnixSeconds()` implementation requires that
the `std::chrono::system_clock` uses the Unix epoch in order to avoid
arithmetic overflow.
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

No branches or pull requests

2 participants