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

Expose file descriptor #19116

Closed
wants to merge 2 commits into from
Closed

Expose file descriptor #19116

wants to merge 2 commits into from

Conversation

l0kod
Copy link
Contributor

@l0kod l0kod commented Nov 19, 2014

For FFI needs, as started by #15643, this allow File, PipeStream and UnixStream to export their internal file descriptor with the AsFileDesc trait provided by #18557.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@l0kod
Copy link
Contributor Author

l0kod commented Nov 19, 2014

cc @errordeveloper

@dtantsur
Copy link

I suspect that maybe of much use for network code (like mio) as well. Could you also expose fd() on sockets?

@l0kod
Copy link
Contributor Author

l0kod commented Nov 19, 2014

I suspect that maybe of much use for network code (like mio) as well. Could you also expose fd() on sockets?

It's doable with TcpStream and UdpStream but I only tested with UnixStream. I plan to add network support if this PR is accepted.

@alexcrichton
Copy link
Member

There are plans to make the contents of std::sys public over time, but currently they're intentionally private while we focus on the design of other modules in the standard library. The intention is to clearly have to opt-in to platform-specific behavior when you're interacting with objects such as files descriptors. This way code can clearly state that it's intended to work on unix-y platforms, but will not work on windows-based platforms.

Currently just reexporting the AsFileDesc trait doesn't quite lend itself to this "opt-in" nature and it doesn't really reflect the actual underlying implementation of each primitive. For example sockets on windows are backed by a SOCKET, not a file descriptor. Additionally, unix pipes on windows are backed by a HANDLE, not a file descriptor. Here you've added an implementation of AsFileDesc for unix pipes on unix, but this is a portability hazard as it will fail to compile on windows.

I know @aturon has many thoughts on this, but it may be best to hold off until we have a stronger vision for the std::sys module and how its stabilization will play out over time.

@alexcrichton
Copy link
Member

Closing in favor of #19169 now, but thanks @l0kod!

@l0kod l0kod deleted the as-fd branch January 10, 2015 14:14
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.

5 participants