Skip to content

Swift: correct dispatch source construction on Win32 #479

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

Merged

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented May 9, 2019

Windows uses handles internally for the dispatch sources. Convert the
fileDescriptor parameter on the incoming point to HANDLEs when
building the DispatchSource. This fixes passing invalid parameters to
the Windows system functions.

Windows uses handles internally for the dispatch sources.  Convert the
`fileDescriptor` parameter on the incoming point to `HANDLE`s when
building the `DispatchSource`.  This fixes passing invalid parameters to
the Windows system functions.
@compnerd
Copy link
Member Author

compnerd commented May 9, 2019

@swift-ci please test

@compnerd
Copy link
Member Author

compnerd commented May 9, 2019

CC: @ktopley-apple

@@ -179,7 +182,13 @@ extension DispatchSource {
#endif

public class func makeReadSource(fileDescriptor: Int32, queue: DispatchQueue? = nil) -> DispatchSourceRead {
let source = dispatch_source_create(_swift_dispatch_source_type_READ(), UInt(fileDescriptor), 0, queue?.__wrapped)
#if os(Windows)
let handle: UInt = UInt(_get_osfhandle(fileDescriptor))
Copy link
Contributor

Choose a reason for hiding this comment

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

Strictly speaking you don't need to declare the type here.

@ktopley-apple ktopley-apple merged commit b073d89 into swiftlang:master May 10, 2019
@compnerd compnerd deleted the get-a-handle-on-the-situation branch May 12, 2019 01:16
rokhinip pushed a commit that referenced this pull request Nov 5, 2021
Swift: correct dispatch source construction on Win32
Signed-off-by: Kim Topley <ktopley@apple.com>
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.

2 participants