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

Re-addition of std::net::IpAddr #988

Closed
murarth opened this issue Mar 19, 2015 · 8 comments
Closed

Re-addition of std::net::IpAddr #988

murarth opened this issue Mar 19, 2015 · 8 comments

Comments

@murarth
Copy link

murarth commented Mar 19, 2015

(Firstly, I saw the discussion around its removal in #923, but that decision appears to have been made before SocketAddr was decided to have an enum structure like that of IpAddr. I'm thinking maybe it was an oversight that IpAddr was left out after that decision. In case it wasn't an oversight, here's my argument in favor of re-introducing IpAddr.)

I propose that the type IpAddr, as an enum with V4(Ipv4Addr) and V6(Ipv6Addr) variants should be re-added to std::net. In my experience, it is often necessary to express an IP address that is either IPv4 or IPv6 without the address being associated with a socket and, therefore, it not having an associated port value. For example, when a server retains the remote addresses of past clients. In such cases, it is not intuitive to store such an address as a SocketAddr instance. Also, the re-addition would mirror the SocketAddr enum, which itself has V4 and V6 variants containing structures to the specific types.

Additionally, I propose that the std::net::LookupHost type should be reverted to yield IpAddr types rather than SocketAddr as this also permits dealing with hostnames in cases where they are not directly related to any socket operations. It should be noted that the ToSocketAddrs trait is already widely used within std::net to yield a series of SocketAddr instances and keeping lookup_host as-is would constitute a duplication of this functionality.

With these changes, I think it would also be helpful to add a convenience method for creating SocketAddr instances; e.g. fn new(ip: &IpAddr, port: u16) -> SocketAddr.

@murarth
Copy link
Author

murarth commented Mar 19, 2015

I missed the discussion around the RFC changes. I apologize if this isn't the usual way to bring up a discussion, but I feel like it's really clunky to have to make my own IpAddr enum in every net-related library and I hoped to have my concerns heard before the standard library became more difficult to change.

cc @carllerche @alexcrichton

@reem
Copy link

reem commented Mar 19, 2015

cc me

@carllerche
Copy link
Member

@murarth I agree w/ you. I didn't really notice the removal of IpAddr in the RFC.

@carllerche
Copy link
Member

Another point to add, joining / leaving multicast groups should take an IP address as an argument, not a socket address as it does currently.

@aturon
Copy link
Member

aturon commented Mar 19, 2015

I agree with this issue -- I think this was an oversight in considering the usage too narrowly. @alexcrichton, I believe, agrees; I think we can file a PR together with an amendment.

@tupshin
Copy link

tupshin commented Mar 23, 2015

+1. ran into this last night. it absolutely makes sense to have an enum over addres types irrespective of having a particular to connect to.

@mikedilger
Copy link

Also revert UdpSocket join_multicast() and leave_multicast() to take an IpAddr. Could be others like these too. edit: oops sorry @carllerche already said this.

@alexcrichton
Copy link
Member

This was re-added some time ago, so closing!

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

No branches or pull requests

7 participants