Skip to content
New issue

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

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

Already on GitHub? # to your account

use Posix.read instead of Glibc.read for timerfd #1432

Merged
merged 4 commits into from
Mar 4, 2020

Conversation

ghost
Copy link

@ghost ghost commented Mar 4, 2020

Motivation:

To read the bytes from timerfd, we use Glibc.read directly but we should use try! Posix.read

Modifications:

Using Posix instead of Glibc for read operation with try! to check for any error.

Result:

Fixes #1341

@@ -549,7 +549,7 @@ internal class Selector<R: Registration> {
// Consume event
var val: UInt64 = 0
// We are not interested in the result
_ = Glibc.read(self.timerFD, &val, MemoryLayout.size(ofValue: val))
_ = try! Posix.read(self.timerFD, &val, MemoryLayout.size(ofValue: val))
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you're missing some labels here:

public static func read(descriptor: CInt, pointer: UnsafeMutableRawPointer, size: size_t) throws -> IOResult<ssize_t> {

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for checking. I am figuring out the values.

@glbrntt glbrntt requested a review from weissi March 4, 2020 13:50
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@weissi
Copy link
Member

weissi commented Mar 4, 2020

@swift-nio-bot test this please

@ghost
Copy link
Author

ghost commented Mar 4, 2020

I didn't understand the CI error. Please help me to understand it.

@weissi
Copy link
Member

weissi commented Mar 4, 2020

ignore it, our CI is underpowered is what that means :(

@weissi
Copy link
Member

weissi commented Mar 4, 2020

@swift-nio-bot test this please

@weissi weissi merged commit 95f014c into apple:master Mar 4, 2020
@weissi weissi added the 🔨 semver/patch No public API change. label Mar 4, 2020
@weissi weissi added this to the 2.15.0 milestone Mar 4, 2020
@ghost
Copy link
Author

ghost commented Mar 4, 2020

Thanks! :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should use Posix.read instead of Glibc.read for timerfd
3 participants