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

Add a method to get the in6_addr from an Ipv6Addr? #27264

Closed
dimbleby opened this issue Jul 24, 2015 · 9 comments
Closed

Add a method to get the in6_addr from an Ipv6Addr? #27264

dimbleby opened this issue Jul 24, 2015 · 9 comments

Comments

@dimbleby
Copy link

How would you feel about adding a method to std::net::Ipv6Addr that returned either an in6_addr or (equivalently) a [u16; 8] in which the u16s are in network order?

My motivation is that I am writing a binding to a C library which naturally wants an in6_addr; on the Rust side I naturally have an Ipv6Addr. At the moment I think that I have to go addr.segments() and then manually convert each segment into network order via to_be().

Of course I can do this - but the implementation of an Ipv6Addr already contains an in6_addr so the upshot is that segments() converted the segments to host order and now I have to convert them back again!

Wouldn't it be nice to skip this double-conversion?

@dimbleby dimbleby changed the title Add a method to get the in_addr6 from an Ipv6Addr? Add a method to get the in6_addr from an Ipv6Addr? Jul 24, 2015
@nagisa
Copy link
Member

nagisa commented Jul 24, 2015

I pretty much interpret it as a request to expose IntoInner trait.

cc @alexcrichton

@alexcrichton
Copy link
Member

Yeah I've wanted this in the past as well from time to time, and there's a few consequences to doing this.

  • We probably want to do this through AsRef instead of IntoInner, but that's exposing an implementation detail of these types. That's probably not the end of the world though as I can't imagine these types ever changing.
  • The literal type stored right now is libc::in6_addr which is a private type to the standard library (e.g. the private distribution of libc). Notably it's different than the crates.io-based libc crate. We can fix this, however, by expanding the std::os::raw module to contain these types. I believe they're all defined on all platforms anyway.

Overall, however, I think this is fine to do. I'd like a bit more discussion to make sure there's consensus (as impls are insta-stable), but the traits we could implement are:

impl AsRef<in_addr> for Ipv4Addr { ... }
impl AsRef<in6_addr> for Ipv6Addr { ... }

impl AsRef<sockaddr_in> for SocketAddrV4 { ... }
impl AsRef<sockaddr_in6> for SocketAddrV6 { ... }

@dimbleby
Copy link
Author

Oh yes, same thing for the socket addresses, good spot.

The relevant C structures are already public in the crates-io crate.

@steveklabnik
Copy link
Member

Triage: no change. Usually, I'd suggest that minor additions like this don't deserve to have an issue open tracking them, especially since nobody has commented in over a year. However, due to @alexcrichton 's comment, it seems like this might not be a super minor addition. @rust-lang/libs what do you think?

@alexcrichton
Copy link
Member

We've backpedaled pretty hard on exposing C types due to platform differences and such. A change like this would force our hand in exporting public in_addr and such types. Unfortunately that's a pretty big hairball, so I'd probably advocate on punting on this nowadays.

@steveklabnik
Copy link
Member

Makes sense. Let's give this a close then.

@dimbleby
Copy link
Author

I understand that there are downsides in exporting in_addr, and rejecting this issue may very well be the right call. But I thought I'd let you know that I do still care about it, if only so that you can be sure that you're doing so for the right reasons - and not only because it has gone quiet.

(I'd have happily commented more often if I'd thought that this was the right way to make a difference...!)

Currently I don't think that I can write the conversions safely any better than this monstrosity. Seems like a lot of trouble for something that really could just be a cast.

Did I miss something simpler? Are we happy that this is just how it has to be?

@steveklabnik
Copy link
Member

@dimbleby so, additions to Rust these days happen in two ways: minor changes have a PR, major changes have an RFC. We don't really keep issues open to track possible additions, as in both cases, they'd be tracked by either the PR or in the RFCs repo.

So, given that there's additional complexity here, an RFC issue and/or PR would be a better place to handle that discussion. Does that make sense?

@dimbleby
Copy link
Author

@steveklabnik - yes, thanks, that's a much more sensible position!

TBH, given that I've already written the awful code, I suspect that working myself up into a full RFC / PR is going to be more trouble than I am motivated to go through.

(And maybe that's a good thing; probably there should be a does-someone-not-only-care-but-actually-care-enough-to-push-for-this bar for such changes).

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

4 participants