Add functions to convert IPv4 to long and back

[achanda]

No description provided.

[rust-highfive]

r? @alexcrichton

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

[nagisa]

Looks like impl From<u32> for Ipv4Addr would be better suited than a method.

[nagisa]

Why does this even take a &self?

[achanda]

@nagisa thanks for the review, fixing.

[achanda]

Still WIP, not ready for review

[alexcrichton]

@achanda has the rationale for this changed since #24618? I thought you meant that the original motivation didn't seem so compelling (causing me to close it).

[achanda]

@alexcrichton sorry, I misunderstood you in the other PR. I thought you were asking about the implementation.

I've been working on implementing CIDR handling functionality in Rust. Converting an IP to and int is a convenient way to work with IP ranges. And I thought these functions might be useful to others too. Another example is, say creating a hash table of IPs.

[alexcrichton]

Hm, I'm primarily worried about endianness here as there's no clear indication of what order each u32 is (going in or coming out). Creating a hash table of IPs shouldn't require going in and out of u32 as the IP itself is a hashable type?

[achanda]

@alexcrichton I do hear your concern around endianness. I guess I can make these functions local to my crate and use those. But, can't I take care of endianness in the functions and have those here?

[alexcrichton]

Whatever is done can certainly be documented here clearly, but there's not necessarily "one true ordering" to use here. I'm also not sure how common this desire it? LLVM should produce the same code in either case, and it seems like a relatively rare operation to want to go from u32 to and from an IP address.

[nagisa]

The endianess here should be the one that entering the IP address as u32
into the browser bar resolves into the correct IP. Yes you can enter IP as
just an integer. And to me it looks like this code is doing the right
thing, though I probably should test it out before claiming it.
On Apr 24, 2015 1:24 AM, "Alex Crichton" notifications@github.com wrote:

Whatever is done can certainly be documented here clearly, but there's not
necessarily "one true ordering" to use here. I'm also not sure how common
this desire it? LLVM should produce the same code in either case, and it
seems like a relatively rare operation to want to go from u32 to and from
an IP address.

—
Reply to this email directly or view it on GitHub
#24652 (comment).

[achanda]

@alexcrichton I don't think going from IP to u32 and back is uncommon. Any computation on an IP needs it to be converted to an int. If the stdlib does not have methods to do that, all consumer libs will need to write those methods. An example is a small crate I've been writing https://github.com/achanda/ipnetwork/blob/master/src/ipnetwork.rs#L52-L61
The reverse conversion is necessary to display results in a human readable form. Also, if both of these conversions are done on the same machine, endianness should not be a problem. Serializing this data, and potentially sending to another machine can be an issue though. But that is not in scope of this module. Thoughts?

@nagisa thanks for pointing out I can use an IP in the browser, pretty cool!

[achanda]

@alexcrichton @nagisa gentle reminder on this :)

I realized that the implementation is broken right now, I will fix it once we reach an agreement if this is needed or not.

[Diggsey]

What would be the closest equivalent for IPv6?

[nagisa]

@Diggsey u128, but we haven’t it yet.

---

Anyway, to me it seems fine to land this (given it works and all comments are addressed). I don’t have the power to make the call, though.

[achanda]

Yes, u128 ideally. Since it's not there, an array of two u64s maybe? But that will defeat the purpose of not having a caller do bit gymnastics for standard operations.

[nagisa]

I’d just wait until rust-lang/rfcs#521 is resolved before considering IPv6 equivalent of this functionality, honestly.

[achanda]

Makes sense.

[alexcrichton]

Ok, looking at this again and after talking it over with @aturon, I think this is fine to land for now. Unfortunately this means that the impl blocks must be insta-#[stable] (due to the way stability attributes work), but these are pretty innocuous. I agree with @nagisa that dealing with IPv6 should wait for a later date.

[alexcrichton]

Due to the way Into and From works, this is more conventionally written as impl From<Ipv4Addr> for u32

[achanda]

@alexcrichton ready for review now.

[alexcrichton]

@bors: r+ 285ab02

Thanks!

[bors]

⌛ Testing commit 285ab02 with merge ff71187...

[alexcrichton]

@bors: retry force clean

[bors]

⌛ Testing commit 285ab02 with merge 4356220...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt