Tweaks to std::net address types

[carllerche]

• Renames SocketAddr -> InetAddr (since there are other socket address types like unix domain socket addresses).
• Add InetAddr::is_unspecified() and InetAddr::any_v*()
• Adds some functions to IpAddr that proxy to the versioned ip address type.
• Add any_* functions to IpAddr and the versioned ip address types

[seanmonstar]

Your other rfc for moving thread_local was included.

On Mon, Mar 2, 2015, 11:34 AM Carl Lerche [email protected] wrote:

• Renames SocketAddr -> InetAddr (since there are other socket address
  types like unix domain socket addresses).
• Add InetAddr::is_unspecified() and InetAddr::any_v*()
• Adds some functions to IpAddr that proxy to the versioned ip address
  type.
• Add any_* functions to IpAddr and the versioned ip address types

---

You can view, comment on, or merge this pull request online at:

#923
Commit Summary

• Move std::thread_local::* into std::thread
• Focus on a single std::thread_local renaming strategy
• Rename to InetAddr, add any* functions

File Changes

• A text/0000-move-thread-local-to-std-thread.md
  https://github.com/rust-lang/rfcs/pull/923/files#diff-0 (47)
• M text/0517-io-os-reform.md
  https://github.com/rust-lang/rfcs/pull/923/files#diff-1 (111)

Patch Links:

• https://github.com/rust-lang/rfcs/pull/923.patch
• https://github.com/rust-lang/rfcs/pull/923.diff

—
Reply to this email directly or view it on GitHub
#923.

[carllerche]

I removed it really fast but I guess you saw it! Thanks!

[codyps]

Is there a reason new takes separate parameters instead of a [u16; 8] ?

[ktossell]

Only that this made it easier for users to switch from the old enum-based IpAddr::Ipv6Addr. Otherwise they'd need to do more than a simple addition of ::new. But that's not a good enough reason, and I think making callers pack the integers into a slice (just add brackets?) is better than making callers who already have a slice break that slice into separate arguments. So I'd change it to [u16; 8], as you said.

[ktossell]

... and the appropriate changes for Ipv4Addr and the new_v4-style functions.

[alexcrichton]

If we're going to spell this out in the RFC I think we may want to hold off on some of these methods. These are pretty ambitious and a more conservative design would only have new and segments, for example. We can always add more over time but I think we may want to start off conservative.

[alexcrichton]

(same for the v4 addr)

[ktossell]

I would keep is_unspecific, to_ipv4 and maybe is_multicast, but +1 for marking the others as experimental or removing them, at least until more people can read through the IP RFCs and validate the code.

[alexcrichton]

cc @ktossell

[ktossell]

I don't like how similar IpAddr and InetAddr are, and I think it's not obvious which one is which, just from looking at the names.

Could we choose either Inet or Ip and have, say, InetAddr (host address) and InetSocketAddr?

[alexcrichton]

After thinking about this some more, I think that we need some deeper changes to these type other than the cosmetic level. The recent rewrite created specific Ipv4Addr and Ipv6Addr type to allow for inspection and setting of various fields, and I think that we want to do the same with socket address. The underlying sockaddr_in and sockaddr_in6 types are fundamentally different in that the IPv6 type exposed more information that is not available in IPv4. Along these lines, I would like to propose the following API:

impl InetAddr {
    pub fn new(addr: IpAddr, port: u16) -> InetAddr;
    pub fn new_v4(addr: InetV4Addr) -> InetAddr;
    pub fn new_v6(addr: InetV6Addr) -> InetAddr;
    pub fn as_v4(&self) -> Option<&InetV4Addr>;
    pub fn as_v6(&self) -> Option<&InetV6Addr>;
    pub fn ip(&self) -> &IpAddr;
    pub fn port(&self) -> u16;
}

impl InetV4Addr {
    pub fn new(addr: Ipv4Addr, port: u16) -> InetV4Addr;
    pub fn ip(&self) -> &Ipv4Addr;
    pub fn port(&self) -> u16;
}

impl InetV6Addr {
    pub fn new(addr: Ipv6Addr, port: u16, flowinfo: u32, scope_id: u32) -> InetV6Addr;
    pub fn ip(&self) -> &Ipv6Addr;
    pub fn port(&self) -> u16;
    pub fn flowinfo(&self) -> u32;
    pub fn scope_id(&self) -> u32;
}

The idea here is to expose all information inside of sockaddr_in and sockaddr_in6 and have a static type corresponding to each. It is likely that we will not expand InetAddr to include Unix domain sockets in the near future.

How does that sound?

[alexcrichton]

Ok, I have pushed a large update to this RFC (@carllerche gave me push rights to his repo) with the following changes:

• Pare back the APIs
• inet_addr methods to local_addr
• InetV{4,6}Addr types
• Remove IpAddr
• Show impls of ToInetAddrs

In general the API is probably even more conservative than it is today, but I believe it poises us for more advanced functionality such as the existing methods on IP addresses and getters/setters/mutators for socket addresses.

It is intentional that it is not possible and/or is difficult to create an InetAddr directly for now. It is thought that the main consumers of these types are currently users of T: ToInetAddrs so we can try to funnel impls through there to start out with.

cc @ktossell, @carllerche, @jmesmon, curious what you guys think!

[alexcrichton]

One point @carllerche brings up is that this current proposal, as it stands, removes all matching on v4/v6 exhaustively. We could perhaps define InetAddr as:

enum InetAddr { V4(InetV4Addr), V6(InetV6Addr) }

This would be an alternative to the as_v4 and as_v6 methods. I'm personally worried about the extensibility of the enum-based approach, however. The downside of the as-based approach is that you don't get nice match statements. Decisions!

[sfackler]

The lack of a constructor for InetAddr will be a bummer for rust-postgres, which maps it to and from some postgres types.

[sfackler]

How likely is it that IPv9 or something is going to come out and cause sadness for an enum-based InetAddr?

[carllerche]

@sfackler I think that a new IP version is not worth worrying about. Rust will have to solve iterating std while maintaining backwards well before anything like that will become a reality.

[alexcrichton]

The lack of a constructor for InetAddr will be a bummer for rust-postgres, which maps it to and from some postgres types.

Aha interesting! This was precisely the use case I was looking for. Would you have a preference over .to_inet_addr methods or InetAddr::from_v4 functions? Although this question is also moot if we use an enum.

How likely is it that IPv9 or something is going to come out and cause sadness for an enum-based InetAddr?

I'm not necessarily worried about something past IPv6, but I am not personally confident enough to say that we will never expose another "instance of struct sockaddr" through this type (InetAddr). For example I do not think that I am personally aware of the exhaustive set of instances of struct sockaddr.

I do agree though that this restriction may just be unnecessarily unergonomic and we may wish to expose the enum, however.

[sfackler]

I don't feel strongly how an InetAddr is constructible as long as it is constructible :).

[carllerche]

There are other sockaddr_* structs, but this is specifically why I recommended renaming SocketAddr -> InetAddr. This way, there could be (if deemed necessary) a SocketAddr type that includes all the possible variants.

[stepancheg]

InetAddr for socket address is incorrect. Inet is short for Internet. Internet address is an IP. For example, when you ping a host, you specify IP, Internet address, not socket address. Socket address is an (IP, port) pair.

SocketAddr is fine with me, but if you think it is not clear enough, it should be InetSocketAddr.

[alexcrichton]

While InetAddr may not be the best name, the motivation behind this change is that in C at least a "socket address" could be anything from a unix domain socket to a TCP address. The motivation here was to move away from the word "socket" as we're not providing the full set of bindings for the equivalent struct sockaddr in C, but rather just for IPv4 and IPv6 addresses.

I don't personally mind the naming either way, but I would consider InetSocketAddr as being a bit too wordy.

[stepancheg]

FYI, in Java (where folks love identifiers like AbstactXmlHttpSecureSocketFactoryCreator) similar class is called InetSocketAdress.

I'm fine with SocketAddr. It does not cover all possible socket addresses, not a big deal. Similarly, int does not contain all possible integers.

I'm fine with InetSocketAddr either.

I see no practical usefulness in Inet6SocketAddr and Inet4SocketAddr. I never used these or similar structures. So InetSocketAddr (or SocketAddr) which is just a pair or (IP, port) is OK to me.

(Disclaimer: my largest programming experience is in C++ and Java, and I'm biased towards decisions made by the authors of JDK, some of them I find good. (But not all of them: Inet4Address in JDK contains both IP and host, which I think is wrong.))

[carllerche]

@stepancheg the InetAddr name comes from the fact that it is backed by INET address family sockets, and is backed by sockaddr_in: http://man7.org/linux/man-pages/man7/ip.7.html

I would be ok w/ InetSocketAddr but rust seems to prefer shorter names. I really do not want SocketAddr due to the fact that there are other types of sockets and other libs (like nix) expose SocketAddr correctly (as an enum of all the types of socket addresses)

[stepancheg]

@carllerche InetAddr is in_addr/in_addr6, not sockaddr_in/sockaddr_in6 from that man page.

[carllerche]

Like I said, I would also be ok w/ InetSocketAddr, but rust seems to favor shorter abbreviated names. I just don't want SocketAddr.

[alexcrichton]

@stepancheg to be clear, the InetAddr proposed in this PR is roughly equivalent to struct sockaddr and InetV4Addr is the equivalent of sockaddr_in (where InetV6Addr is the same as sockaddr_in6)

[stepancheg]

@alexcrichton internet address is an IP (without port). socket address (in internet) is IP+port. InetAddr is short for "internet address". It looks so obvious to me, so I simply don't understand why someone could possibly insist on term InetAddr for socket address.

[aturon]

FWIW, I agree with @stepancheg that InetSocketAddr is the most precise name for this type. The name is a bit long but I think it's fine.

[aturon]

@alexcrichton

It is intentional that it is not possible and/or is difficult to create an InetAddr directly for now. It is thought that the main consumers of these types are currently users of T: ToInetAddrs so we can try to funnel impls through there to start out with.

Can you elaborate on why this is the intent? What problem do you see with exposing constructor functions, for example?

[alexcrichton]

@aturon

FWIW, I agree with @stepancheg that InetSocketAddr is the most precise name for this type. The name is a bit long but I think it's fine.

Given this, I would want to reconsider renaming SocketAddr in the first place. I don't think that tacking on Inet is really buying us much because it's relative location, net::SocketAddr in theory is just as telling that it's networking-related socket address (and the primitives are all related to TCP/UDP).

Can you elaborate on why this is the intent? What problem do you see with exposing constructor functions, for example?

It is mainly motivated to stick to being conservative. Given @sfackler's use case though it may be best to just expose enum SocketAddr { V4(...), V6(...) }

[aturon]

@alexcrichton

Sticking with SocketAddr is possibly problematic given that:

1. We don't intend to cover e.g. unix domain sockets with this, and
2. It is therefore a mismatch with the underlying system notion of socket addresses

(I think @carllerche is raising these points as well).

That said, (1) may not be so important since domain sockets would live in std::os::unix (so the std::net::SocketAddr type might not be confused with them).

I guess this is what you mean about the "relative location"?

Personally, I'm fine with basically any of the proposed names; we just need to ship.

[alexcrichton]

I guess this is what you mean about the "relative location"?

Yeah I'm thinking that InetSocketAddr is not a whole lot more descriptive than net::SocketAddr as I would consider the latter as only being for networking (and documentation could clarify of course).

[alexcrichton]

I've pushed another update which goes back to SocketAddr and uses an enum instead of an opaque struct to allow both construction and matching.

[brson]

Since SocketAddress lives in 'net', and unix sockets if ever implemented could live somewhere else, it can be clear that a net::SocketAddress refers to an IP socket, and even that std::net means IP networking.

I'm inclined to agree with @stepancheg that InetAdddress is less accurate than SocketAddress.

[aturon]

After discussion here and with the RFC author on IRC, we have decided to move forward with this RFC. In the end, this largely keeps the design of the std::net as it was, but ties the Tcp and Udp streams more closely to IP addresses, with the intent that things like domain sockets involve distinct types.

[carllerche]

@aturon I actually agree w/ @murarth that it is strange for hostname resolution to return a socket address type (vs just an IpAddr). I know that getaddrinfo allows for it, but it is pretty clunky.