Make the unix::net::SocketAddr creation methods public

[kleimkuhler]

Motivation

Recently tokio-rs/mio needed support for creating the
std::os::unix::net::SocketAddr type. While the type is currently public, it is
not possible to create one as both creation methods are private:

• SocketAddr::new
• SocketAddr::from_parts

The workaround was creating a mio specific SocketAddr that is equivalent to
this type.

A use of needing SocketAddr::from_parts is shown here.

Details

This PR makes the creation methods public so that the already public type can be
created as well. Comments with examples have been added.

I did not feel an RFC process was needed for this; I hope that assumtion is
correct!

Signed-off-by: Kevin Leimkuhler kleimkuhler@icloud.com

[rust-highfive]

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

[rust-highfive]

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2019-10-10T00:42:14.6808276Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-10T00:42:14.6904399Z ##[command]git config gc.auto 0
2019-10-10T00:42:14.6958600Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-10T00:42:14.7022710Z ##[command]git config --get-all http.proxy
2019-10-10T00:42:14.7170264Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65255/merge:refs/remotes/pull/65255/merge
---
2019-10-10T00:47:45.7181257Z     Checking panic_abort v0.0.0 (/checkout/src/libpanic_abort)
2019-10-10T00:47:45.7675741Z     Checking rustc-std-workspace-alloc v1.99.0 (/checkout/src/tools/rustc-std-workspace-alloc)
2019-10-10T00:47:46.7067022Z     Checking panic_unwind v0.0.0 (/checkout/src/libpanic_unwind)
2019-10-10T00:47:46.9680870Z     Checking backtrace v0.3.37
2019-10-10T00:47:51.4385764Z error: item has missing stability attribute
2019-10-10T00:47:51.4387139Z     |
2019-10-10T00:47:51.4387139Z     |
2019-10-10T00:47:51.4388324Z 126 | /     pub fn new<F>(f: F) -> io::Result<SocketAddr>
2019-10-10T00:47:51.4388989Z 127 | |         where F: FnOnce(*mut libc::sockaddr, *mut libc::socklen_t) -> libc::c_int
2019-10-10T00:47:51.4390015Z 129 | |         unsafe {
2019-10-10T00:47:51.4390472Z ...   |
2019-10-10T00:47:51.4390940Z 134 | |         }
2019-10-10T00:47:51.4391431Z 135 | |     }
2019-10-10T00:47:51.4391431Z 135 | |     }
2019-10-10T00:47:51.4391991Z     | |_____^
2019-10-10T00:47:51.4392134Z 
2019-10-10T00:47:51.4396202Z error: item has missing stability attribute
2019-10-10T00:47:51.4397256Z     |
2019-10-10T00:47:51.4397256Z     |
2019-10-10T00:47:51.4398187Z 149 | /     pub fn from_parts(addr: libc::sockaddr_un, mut len: libc::socklen_t) -> io::Result<SocketAddr> {
2019-10-10T00:47:51.4398787Z 150 | |         if len == 0 {
2019-10-10T00:47:51.4399360Z 151 | |             // When there is a datagram from unnamed unix socket
2019-10-10T00:47:51.4399886Z 152 | |             // linux returns zero bytes of address
2019-10-10T00:47:51.4400851Z 162 | |         })
2019-10-10T00:47:51.4401508Z 163 | |     }
2019-10-10T00:47:51.4402039Z     | |_____^
2019-10-10T00:47:51.4402177Z 
---
2019-10-10T00:47:51.6274575Z == clock drift check ==
2019-10-10T00:47:51.6289455Z   local time: Thu Oct 10 00:47:51 UTC 2019
2019-10-10T00:47:51.7157010Z   network time: Thu, 10 Oct 2019 00:47:51 GMT
2019-10-10T00:47:51.7160460Z == end clock drift check ==
2019-10-10T00:47:52.8525106Z ##[error]Bash exited with code '1'.
2019-10-10T00:47:52.8568167Z ##[section]Starting: Checkout
2019-10-10T00:47:52.8570144Z ==============================================================================
2019-10-10T00:47:52.8570200Z Task         : Get sources
2019-10-10T00:47:52.8570248Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[jonas-schievink]

These need #[unstable] attributes

[KodrAus]

Hmm, in their current state I don’t think the new or from_parts methods are really suitable in the public API because they expose external libc types, and I find the signature for new a little surprising as anything but an implementation detail.

It looks like we do have some examples of defining libc types with friendlier names though, like RawPthread. The sockaddr_un type isn’t just a simple alias though.

So I think we’ll need to sketch out a more suitable public API for constructing SocketAddrs than what’s currently there first, that doesn’t publicly use libc and has appropriate safety.

[sfackler]

Why can't mio just use the standard library's UnixListener? https://doc.rust-lang.org/std/os/unix/net/struct.UnixListener.html

[kleimkuhler]

@sfackler It is mainly because accept on Socket which UnixListener::accept dispatches to does not set the O_NONBLOCK flag. It can be done with a separate fcntl call after the socket is returned, but then that is an additional call that needs to take place. We can use accept4 on the OSs that support it and set these flags atomically.

[kleimkuhler]

@KodrAus That is a good point. I found the signatures a little surprising when first looking at them. If exposing libc types is an issue, there would definitely need to be some additional design around the fields returned in SocketAddr. I can close this PR for now if additional discussion should take place in the issue? It would be nice to use libstd when possible, but this understandably isn't going to be solved just by making these methods public.

[KodrAus]

@kleimkuhler that sounds like a good plan me to! Maybe we can also look at whether it’s feasible for mio to move to the standard UnixListener and what might need to be added to support it?

[kleimkuhler]

@KodrAus So it's not just UnixListener that mio has to special case all this for, it's pretty much all the net resources that would be expected to not block when no read/write is available.

This ultimately is because the sockets need this additional flag set as part of the socket creation instead of a separate call to libstd's set_nonblocking. Maybe there is some design discussion for offering this through libstd, but it definitely would be a larger change!

Closing this PR per discussion above and further conversation can take place in #65275.