Merge #1685

1685: Deprecate IpAddr, Ipv4Addr, and Ipv6Addr r=rtzoeller a=asomers Because they're redundant with types in the standard library. Fixes #1681 Co-authored-by: Alan Somers <asomers@gmail.com>
nix-rust · Mar 23, 2022 · 3ca28f6 · 3ca28f6
2 parents 0fe6682 + fbeabf3
commit 3ca28f6
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -77,6 +77,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 - Deprecated `InetAddr` and `SockAddr` in favor of `SockaddrIn`, `SockaddrIn6`,
   and `SockaddrStorage`.
   (#[1684](https://github.com/nix-rust/nix/pull/1684))
+- Deprecated `IpAddr`, `Ipv4Addr`, and `Ipv6Addr` in favor of their equivalents
+  from the standard library.
+  (#[1685](https://github.com/nix-rust/nix/pull/1685))
 
 ### Fixed
 

diff --git a/src/sys/socket/addr.rs b/src/sys/socket/addr.rs
@@ -32,6 +32,26 @@ pub use self::datalink::LinkAddr;
 #[cfg(any(target_os = "android", target_os = "linux"))]
 pub use self::vsock::VsockAddr;
 
+/// Convert a std::net::Ipv4Addr into the libc form.
+#[cfg(feature = "net")]
+pub(crate) fn ipv4addr_to_libc(addr: net::Ipv4Addr) -> libc::in_addr {
+    let octets = addr.octets();
+    libc::in_addr {
+        s_addr: u32::to_be(((octets[0] as u32) << 24) |
+            ((octets[1] as u32) << 16) |
+            ((octets[2] as u32) << 8) |
+            (octets[3] as u32))
+    }
+}
+
+/// Convert a std::net::Ipv6Addr into the libc form.
+#[cfg(feature = "net")]
+pub(crate) const fn ipv6addr_to_libc(addr: &net::Ipv6Addr) -> libc::in6_addr {
+    libc::in6_addr {
+        s6_addr: addr.octets()
+    }
+}
+
 /// These constants specify the protocol family to be used
 /// in [`socket`](fn.socket.html) and [`socketpair`](fn.socketpair.html)
 ///
@@ -497,14 +517,20 @@ impl fmt::Display for InetAddr {
  * ===== IpAddr =====
  *
  */
-#[allow(missing_docs)]  // https://github.com/nix-rust/nix/issues/1681
+#[allow(missing_docs)]  // Since they're all deprecated anyway
+#[allow(deprecated)]
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
+#[deprecated(
+    since = "0.24.0",
+    note = "Use std::net::IpAddr instead"
+)]
 pub enum IpAddr {
     V4(Ipv4Addr),
     V6(Ipv6Addr),
 }
 
-#[allow(missing_docs)]  // https://github.com/nix-rust/nix/issues/1681
+#[allow(deprecated)]
+#[allow(missing_docs)]  // Since they're all deprecated anyway
 impl IpAddr {
     /// Create a new IpAddr that contains an IPv4 address.
     ///
@@ -537,6 +563,7 @@ impl IpAddr {
     }
 }
 
+#[allow(deprecated)]
 impl fmt::Display for IpAddr {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
@@ -552,12 +579,17 @@ impl fmt::Display for IpAddr {
  *
  */
 
-#[allow(missing_docs)]  // https://github.com/nix-rust/nix/issues/1681
+#[deprecated(
+    since = "0.24.0",
+    note = "Use std::net::Ipv4Addr instead"
+)]
+#[allow(missing_docs)]  // Since they're all deprecated anyway
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 #[repr(transparent)]
 pub struct Ipv4Addr(pub libc::in_addr);
 
-#[allow(missing_docs)]  // https://github.com/nix-rust/nix/issues/1681
+#[allow(deprecated)]
+#[allow(missing_docs)]  // Since they're all deprecated anyway
 impl Ipv4Addr {
     #[allow(clippy::identity_op)]   // More readable this way
     pub const fn new(a: u8, b: u8, c: u8, d: u8) -> Ipv4Addr {
@@ -591,6 +623,7 @@ impl Ipv4Addr {
     }
 }
 
+#[allow(deprecated)]
 impl fmt::Display for Ipv4Addr {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         let octets = self.octets();
@@ -604,7 +637,11 @@ impl fmt::Display for Ipv4Addr {
  *
  */
 
-#[allow(missing_docs)]  // https://github.com/nix-rust/nix/issues/1681
+#[deprecated(
+    since = "0.24.0",
+    note = "Use std::net::Ipv6Addr instead"
+)]
+#[allow(missing_docs)]  // Since they're all deprecated anyway
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 #[repr(transparent)]
 pub struct Ipv6Addr(pub libc::in6_addr);
@@ -625,7 +662,8 @@ macro_rules! to_u16_array {
     }
 }
 
-#[allow(missing_docs)]  // https://github.com/nix-rust/nix/issues/1681
+#[allow(deprecated)]
+#[allow(missing_docs)]  // Since they're all deprecated anyway
 impl Ipv6Addr {
     #[allow(clippy::many_single_char_names)]
     #[allow(clippy::too_many_arguments)]
@@ -649,6 +687,7 @@ impl Ipv6Addr {
     }
 }
 
+#[allow(deprecated)]
 impl fmt::Display for Ipv6Addr {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         self.to_std().fmt(fmt)
@@ -1172,7 +1211,7 @@ impl From<net::SocketAddrV4> for SockaddrIn {
             sin_len: mem::size_of::<libc::sockaddr_in>() as u8,
             sin_family: AddressFamily::Inet as sa_family_t,
             sin_port: addr.port().to_be(),  // network byte order
-            sin_addr: Ipv4Addr::from_std(addr.ip()).0,
+            sin_addr: ipv4addr_to_libc(*addr.ip()),
             .. unsafe { mem::zeroed() }
         })
     }
@@ -1266,7 +1305,7 @@ impl From<net::SocketAddrV6> for SockaddrIn6 {
             sin6_len: mem::size_of::<libc::sockaddr_in6>() as u8,
             sin6_family: AddressFamily::Inet6 as sa_family_t,
             sin6_port: addr.port().to_be(),  // network byte order
-            sin6_addr: Ipv6Addr::from_std(addr.ip()).0,
+            sin6_addr: ipv6addr_to_libc(addr.ip()),
             sin6_flowinfo: addr.flowinfo(),  // host byte order
             sin6_scope_id: addr.scope_id(),  // host byte order
             .. unsafe { mem::zeroed() }
@@ -2545,6 +2584,24 @@ pub mod vsock {
 mod tests {
     use super::*;
 
+    mod types {
+        use super::*;
+
+        #[test]
+        fn test_ipv4addr_to_libc() {
+            let s = std::net::Ipv4Addr::new(1, 2, 3, 4);
+            let l = ipv4addr_to_libc(s);
+            assert_eq!(l.s_addr, u32::to_be(0x01020304));
+        }
+
+        #[test]
+        fn test_ipv6addr_to_libc() {
+            let s = std::net::Ipv6Addr::new(1, 2, 3, 4, 5, 6, 7, 8);
+            let l = ipv6addr_to_libc(&s);
+            assert_eq!(l.s6_addr, [0, 1, 0, 2, 0, 3, 0, 4, 0, 5, 0, 6, 0, 7, 0, 8]);
+        }
+    }
+
     mod link {
         #[cfg(any(target_os = "ios",
                   target_os = "macos",

diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs
@@ -8,6 +8,8 @@ use libc::{self, c_void, c_int, iovec, socklen_t, size_t,
 use std::convert::TryInto;
 use std::{mem, ptr, slice};
 use std::os::unix::io::RawFd;
+#[cfg(feature = "net")]
+use std::net;
 #[cfg(target_os = "linux")]
 #[cfg(feature = "uio")]
 use crate::sys::time::TimeSpec;
@@ -93,6 +95,9 @@ pub use libc::{sockaddr_in, sockaddr_in6};
 #[doc(hidden)]
 pub use libc::{c_uint, CMSG_SPACE};
 
+#[cfg(feature = "net")]
+use crate::sys::socket::addr::{ipv4addr_to_libc, ipv6addr_to_libc};
+
 /// These constants are used to specify the communication semantics
 /// when creating a socket with [`socket()`](fn.socket.html)
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
@@ -496,10 +501,16 @@ impl IpMembershipRequest {
     /// Instantiate a new `IpMembershipRequest`
     ///
     /// If `interface` is `None`, then `Ipv4Addr::any()` will be used for the interface.
-    pub fn new(group: Ipv4Addr, interface: Option<Ipv4Addr>) -> Self {
+    pub fn new(group: net::Ipv4Addr, interface: Option<net::Ipv4Addr>)
+        -> Self
+    {
+        let imr_addr = match interface {
+            None => net::Ipv4Addr::UNSPECIFIED,
+            Some(addr) => addr
+        };
         IpMembershipRequest(libc::ip_mreq {
-            imr_multiaddr: group.0,
-            imr_interface: interface.unwrap_or_else(Ipv4Addr::any).0,
+            imr_multiaddr: ipv4addr_to_libc(group),
+            imr_interface: ipv4addr_to_libc(imr_addr)
         })
     }
 }
@@ -513,9 +524,9 @@ pub struct Ipv6MembershipRequest(libc::ipv6_mreq);
 
 impl Ipv6MembershipRequest {
     /// Instantiate a new `Ipv6MembershipRequest`
-    pub const fn new(group: Ipv6Addr) -> Self {
+    pub const fn new(group: net::Ipv6Addr) -> Self {
         Ipv6MembershipRequest(libc::ipv6_mreq {
-            ipv6mr_multiaddr: group.0,
+            ipv6mr_multiaddr: ipv6addr_to_libc(&group),
             ipv6mr_interface: 0,
         })
     }

diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs
@@ -1833,7 +1833,7 @@ mod linux_errqueue {
                     if let Some(origin) = err_addr {
                         // Validate that our network error originated from 127.0.0.1:0.
                         assert_eq!(origin.sin_family, AddressFamily::Inet as _);
-                        assert_eq!(Ipv4Addr(origin.sin_addr), Ipv4Addr::new(127, 0, 0, 1));
+                        assert_eq!(origin.sin_addr.s_addr, u32::from_be(0x7f000001));
                         assert_eq!(origin.sin_port, 0);
                     } else {
                         panic!("Expected some error origin");
@@ -1877,8 +1877,8 @@ mod linux_errqueue {
                         // Validate that our network error originated from localhost:0.
                         assert_eq!(origin.sin6_family, AddressFamily::Inet6 as _);
                         assert_eq!(
-                            Ipv6Addr(origin.sin6_addr),
-                            Ipv6Addr::from_std(&"::1".parse().unwrap()),
+                            origin.sin6_addr.s6_addr,
+                            std::net::Ipv6Addr::LOCALHOST.octets()
                         );
                         assert_eq!(origin.sin6_port, 0);
                     } else {