From 0b71d709c8754c3fc533195f63b674281b3dcc36 Mon Sep 17 00:00:00 2001 From: Andrew Ayer Date: Fri, 12 Feb 2016 17:24:48 -0800 Subject: [PATCH 1/3] Add RFC for ipv6addr octets interface --- text/0000-ipv6addr-octets.md | 69 ++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 text/0000-ipv6addr-octets.md diff --git a/text/0000-ipv6addr-octets.md b/text/0000-ipv6addr-octets.md new file mode 100644 index 00000000000..d0aa85926d1 --- /dev/null +++ b/text/0000-ipv6addr-octets.md @@ -0,0 +1,69 @@ +- Feature Name: ipv6addr_octets_interface +- Start Date: 2016-02-12 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Add constructor and getter functions to `std::net::Ipv6Addr` that are +oriented around octets. + +# Motivation +[motivation]: #motivation + +Currently, the interface for `std::net::Ipv6Addr` is oriented around 16-bit +"segments". The constructor takes eight 16-bit integers as arguments, +and the sole getter function, `segments`, returns an array of eight +16-bit integers. This interface is unnatural when doing low-level network +programming, where IPv6 addresses are treated as a sequence of 16 octets. +For example, building and parsing IPv6 packets requires doing +bitwise arithmetic with careful attention to byte order in order to convert +between the on-wire format of 16 octets and the eight segments format used +by `std::net::Ipv6Addr`. + +# Detailed design +[design]: #detailed-design + +Two functions would be added to `impl std::net::Ipv6Addr`: + +``` +pub fn from_octets(octets: &[u8; 16]) -> Ipv6Addr { + let mut addr: c::in6_addr = unsafe { std::mem::zeroed() }; + addr.s6_addr = *octets; + Ipv6Addr { inner: addr } +} +pub fn octets(&self) -> &[u8; 16] { + &self.inner.s6_addr +} +``` + +# Drawbacks +[drawbacks]: #drawbacks + +It adds additional functions to the `Ipv6Addr` API, which increases cognitive load +and maintenance burden. That said, the functions are conceptually very simple +and their implementations short. + +Returning a reference from `octets` ties the interface to the internal representation +of `Ipv6Addr`, which is currently `[u8; 16]`. It would not be possible to change `Ipv6Addr` +to use a different representation without changing the return type of `octets` to be a non-reference. + +# Alternatives +[alternatives]: #alternatives + +Do nothing. The downside is that developers will need to resort to +bitwise arithmetic, which is awkward and error-prone (particularly with +respect to byte ordering) to convert between `Ipv6Addr` and the on-wire +representation of IPv6 addresses. Or they will use their alternative +implementations of `Ipv6Addr`, fragmenting the ecosystem. + +`octets` could return a non-reference to avoid tying the interface to the +internal representation. However, it seems unlikely that the internal +representation would ever be anything besides a `[u8; 16]`. + +# Unresolved questions +[unresolved]: #unresolved-questions + +Should `octets` return a reference? Pro: avoid a copy. Con: ties the interface to the internal +representation, which is presently `[u8; 16]`. From f26c41b57084bf9858c0d363c3444c201dc940bb Mon Sep 17 00:00:00 2001 From: Andrew Ayer Date: Wed, 9 Mar 2016 20:56:22 -0800 Subject: [PATCH 2/3] Update RFC in response to feedback * `octets` no longer returns a reference, due to bad experiences with returning internal references to libc structures in the past. * Replace `from_octets` with an implementation of `From<[u8; 16]>` for `Ipv6Addr`. * For consistency, also implement `From<[u8; 4]>` for `Ipv4Addr`. --- text/0000-ipv6addr-octets.md | 46 +++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/text/0000-ipv6addr-octets.md b/text/0000-ipv6addr-octets.md index d0aa85926d1..f63141a0d21 100644 --- a/text/0000-ipv6addr-octets.md +++ b/text/0000-ipv6addr-octets.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -Add constructor and getter functions to `std::net::Ipv6Addr` that are +Add constructor and conversion functions for `std::net::Ipv6Addr` that are oriented around octets. # Motivation @@ -25,30 +25,44 @@ by `std::net::Ipv6Addr`. # Detailed design [design]: #detailed-design -Two functions would be added to `impl std::net::Ipv6Addr`: +The following method would be added to `impl std::net::Ipv6Addr`: ``` -pub fn from_octets(octets: &[u8; 16]) -> Ipv6Addr { - let mut addr: c::in6_addr = unsafe { std::mem::zeroed() }; - addr.s6_addr = *octets; - Ipv6Addr { inner: addr } +pub fn octets(&self) -> [u8; 16] { + self.inner.s6_addr } -pub fn octets(&self) -> &[u8; 16] { - &self.inner.s6_addr +``` + +The following `From` trait would be implemented: + +``` +impl From<[u8; 16]> for Ipv6Addr { + fn from(octets: [u8; 16]) -> Ipv6Addr { + let mut addr: c::in6_addr = unsafe { std::mem::zeroed() }; + addr.s6_addr = octets; + Ipv6Addr { inner: addr } + } +} +``` + +For consistency, the following `From` trait would be +implemented for `Ipv4Addr`: + +``` +impl From<[u8; 4]> for Ipv4Addr { + fn from(octets: [u8; 4]) -> Ipv4Addr { + Ipv4Addr::new(octets[0], octets[1], octets[2], octets[3]) + } } ``` # Drawbacks [drawbacks]: #drawbacks -It adds additional functions to the `Ipv6Addr` API, which increases cognitive load +It adds additional functions to the API, which increases cognitive load and maintenance burden. That said, the functions are conceptually very simple and their implementations short. -Returning a reference from `octets` ties the interface to the internal representation -of `Ipv6Addr`, which is currently `[u8; 16]`. It would not be possible to change `Ipv6Addr` -to use a different representation without changing the return type of `octets` to be a non-reference. - # Alternatives [alternatives]: #alternatives @@ -58,12 +72,6 @@ respect to byte ordering) to convert between `Ipv6Addr` and the on-wire representation of IPv6 addresses. Or they will use their alternative implementations of `Ipv6Addr`, fragmenting the ecosystem. -`octets` could return a non-reference to avoid tying the interface to the -internal representation. However, it seems unlikely that the internal -representation would ever be anything besides a `[u8; 16]`. - # Unresolved questions [unresolved]: #unresolved-questions -Should `octets` return a reference? Pro: avoid a copy. Con: ties the interface to the internal -representation, which is presently `[u8; 16]`. From 0fede964c112958bb7525efaf9c8e4dcfcfef3fa Mon Sep 17 00:00:00 2001 From: Andrew Ayer Date: Wed, 9 Mar 2016 21:05:37 -0800 Subject: [PATCH 3/3] Expand summary and rename feature to account for expanded scope --- text/0000-ipv6addr-octets.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/text/0000-ipv6addr-octets.md b/text/0000-ipv6addr-octets.md index f63141a0d21..2dcdec1f798 100644 --- a/text/0000-ipv6addr-octets.md +++ b/text/0000-ipv6addr-octets.md @@ -1,4 +1,4 @@ -- Feature Name: ipv6addr_octets_interface +- Feature Name: ipaddr_octet_arrays - Start Date: 2016-02-12 - RFC PR: (leave this empty) - Rust Issue: (leave this empty) @@ -6,8 +6,8 @@ # Summary [summary]: #summary -Add constructor and conversion functions for `std::net::Ipv6Addr` that are -oriented around octets. +Add constructor and conversion functions for `std::net::Ipv6Addr` and +`std::net::Ipv4Addr` that are oriented around arrays of octets. # Motivation [motivation]: #motivation @@ -56,6 +56,8 @@ impl From<[u8; 4]> for Ipv4Addr { } ``` +Note: `Ipv4Addr` already has an `octets` method that returns a `[u8; 4]`. + # Drawbacks [drawbacks]: #drawbacks