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.

Sign up for GitHub

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

Jump to bottom

Fix overflow in TimeSpec addition on 32-bit architectures #1074

Merged
merged 1 commit into from
Oct 27, 2022
Merged

Fix overflow in TimeSpec addition on 32-bit architectures #1074

merged 1 commit into from
Oct 27, 2022

Conversation

orobio
Copy link
Contributor

@orobio orobio commented Oct 26, 2022

When Int is 32 bits, converting seconds to nanoseconds overflows. This change uses Int64 explicitly.

The change has been verified on a custom Linux armv7 system. Cross-compilation was used for the armv7 build, so I wasn't able to run the tests for that, but all tests pass on x86_64.

@swift-server-bot
Copy link

Can one of the admins verify this patch?

9 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@ktoso ktoso self-requested a review October 26, 2022 21:12
@ktoso
Copy link
Member

ktoso commented Oct 26, 2022

@swift-server-bot add to allowlist

@ktoso
Copy link
Member

ktoso commented Oct 26, 2022

@swift-server-bot test this please

@ktoso
Copy link
Member

ktoso commented Oct 26, 2022 •
edited
Loading

Known flaky test:

@ktoso
Copy link
Member

ktoso commented Oct 26, 2022

@swift-server-bot add to allowlist

ktoso
ktoso approved these changes Oct 26, 2022
View reviewed changes
Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix and trying the cluster on other platforms!

@ktoso
Copy link
Member

ktoso commented Oct 26, 2022

@swift-server-bot test this please

@ktoso
Copy link
Member

ktoso commented Oct 26, 2022

Really flaky CI today :\ #1076
Need to revisit all those

@ktoso
Copy link
Member

ktoso commented Oct 27, 2022

@swift-server-bot test this please

@ktoso
Copy link
Member

ktoso commented Oct 27, 2022

Okey I'm going to merge this patch and do a follow up on the flaky test with more debug information around the events -- can't reproduce this failure but we should be able on CI and I found a good way to improve logging.

@ktoso ktoso merged commit 970bfdd into apple:main Oct 27, 2022
@ktoso ktoso added this to the 1.0.0-beta.3 milestone Oct 27, 2022
@ktoso ktoso added the kind/bug Feature doesn't work as expected. label Oct 27, 2022
@orobio
Copy link
Contributor Author

orobio commented Oct 27, 2022

Thanks for the fix and trying the cluster on other platforms!

No problem and thanks for creating this. Just trying it out now and so far I really like how this all works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ktoso ktoso ktoso approved these changes

Assignees
No one assigned
Labels
kind/bug Feature doesn't work as expected.
Projects
None yet
Milestone
1.0.0-beta.3
Development

Successfully merging this pull request may close these issues.
3 participants
@orobio @swift-server-bot @ktoso