Repository: achanda/ipnetwork (language: rust)

Issue:
convenience: Iterator improvements
Hello

I'm reading through the code a bit. I've noticed few places for possible improvements around iterators:

* There's an iterator support for Ipv4Network as well as Ipv6Network. But there's not one for IpNetwork. I think providing a wrapper iterator would be quite trivial.
* The iterator types may benefit from some derived traits (eg. `#[derive(Debug, Copy, Clone)]`.
* Oftentimes, if there's a `.iter()` method for a type, the type-reference implements `IntoIterator`. Eg. `impl IntoIterator for &Ipv4Network`.

If these would be considered reasonable additions, I'd volunteer to send the pull request.


Discussion / hints:
A pull request will be welcome, thanks!

Fix patch:
diff --git a/src/ipv4.rs b/src/ipv4.rs
index 95e2a27..9a8578c 100644
--- a/src/ipv4.rs
+++ b/src/ipv4.rs
@@ -62,8 +62,8 @@ impl Ipv4Network {
     /// addresses.
     pub fn iter(&self) -> Ipv4NetworkIterator {
         let start = u32::from(self.network());
-        let end = start + self.size();
-        Ipv4NetworkIterator { next: start, end }
+        let end = start + (self.size() - 1);
+        Ipv4NetworkIterator { next: Some(start), end }
     }
 
     pub fn ip(&self) -> Ipv4Addr {
@@ -253,8 +253,9 @@ impl From<Ipv4Addr> for Ipv4Network {
     }
 }
 
+#[derive(Clone, Debug)]
 pub struct Ipv4NetworkIterator {
-    next: u32,
+    next: Option<u32>,
     end: u32,
 }
 
@@ -262,13 +263,21 @@ impl Iterator for Ipv4NetworkIterator {
     type Item = Ipv4Addr;
 
     fn next(&mut self) -> Option<Ipv4Addr> {
-        if self.next < self.end {
-            let next = Ipv4Addr::from(self.next as u32);
-            self.next += 1;
-            Some(next)
-        } else {
+        let next = self.next?;
+        self.next = if next == self.end {
             None
-        }
+        } else {
+            Some(next + 1)
+        };
+        Some(next.into())
+    }
+}
+
+impl IntoIterator for &'_ Ipv4Network {
+    type IntoIter = Ipv4NetworkIterator;
+    type Item = Ipv4Addr;
+    fn into_iter(self) -> Ipv4NetworkIterator {
+        self.iter()
     }
 }
 
@@ -605,4 +614,19 @@ mod test {
         assert_eq!(skynet.overlaps(other2), false);
         assert_eq!(other2.overlaps(other3), true);
     }
+
+    #[test]
+    fn edges() {
+        let low: Ipv4Network = "0.0.0.0/24".parse().unwrap();
+        let low_addrs: Vec<Ipv4Addr> = low.iter().collect();
+        assert_eq!(256, low_addrs.len());
+        assert_eq!("0.0.0.0".parse::<Ipv4Addr>().unwrap(), low_addrs[0]);
+        assert_eq!("0.0.0.255".parse::<Ipv4Addr>().unwrap(), low_addrs[255]);
+
+        let high: Ipv4Network = "255.255.255.0/24".parse().unwrap();
+        let high_addrs: Vec<Ipv4Addr> = high.iter().collect();
+        assert_eq!(256, high_addrs.len());
+        assert_eq!("255.255.255.0".parse::<Ipv4Addr>().unwrap(), high_addrs[0]);
+        assert_eq!("255.255.255.255".parse::<Ipv4Addr>().unwrap(), high_addrs[255]);
+    }
 }
diff --git a/src/ipv6.rs b/src/ipv6.rs
index 0790f93..5ddcf73 100644
--- a/src/ipv6.rs
+++ b/src/ipv6.rs
@@ -70,7 +70,7 @@ impl Ipv6Network {
         let end: u128 = dec | mask;
 
         Ipv6NetworkIterator {
-            next: start,
+            next: Some(start),
             end: end,
         }
     }
@@ -227,8 +227,9 @@ impl From<Ipv6Addr> for Ipv6Network {
     }
 }
 
+#[derive(Clone, Debug)]
 pub struct Ipv6NetworkIterator {
-    next: u128,
+    next: Option<u128>,
     end: u128,
 }
 
@@ -236,13 +237,21 @@ impl Iterator for Ipv6NetworkIterator {
     type Item = Ipv6Addr;
 
     fn next(&mut self) -> Option<Ipv6Addr> {
-        if self.next <= self.end {
-            let next = Ipv6Addr::from(self.next);
-            self.next += 1;
-            Some(next)
-        } else {
+        let next = self.next?;
+        self.next = if next == self.end {
             None
-        }
+        } else {
+            Some(next + 1)
+        };
+        Some(next.into())
+    }
+}
+
+impl IntoIterator for &'_ Ipv6Network {
+    type IntoIter = Ipv6NetworkIterator;
+    type Item = Ipv6Addr;
+    fn into_iter(self) -> Ipv6NetworkIterator {
+        self.iter()
     }
 }
 
@@ -580,4 +589,15 @@ mod test {
 
         assert_eq!(other2.overlaps(other), true);
     }
+
+    #[test]
+    fn edges() {
+        let low: Ipv6Network = "::0/120".parse().unwrap();
+        let low_addrs: Vec<Ipv6Addr> = low.iter().collect();
+        assert_eq!(256, low_addrs.len());
+
+        let high: Ipv6Network = "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ff00/120".parse().unwrap();
+        let high_addrs: Vec<Ipv6Addr> = high.iter().collect();
+        assert_eq!(256, high_addrs.len());
+    }
 }
diff --git a/src/lib.rs b/src/lib.rs
index 24abf7a..09377d4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -16,6 +16,8 @@ mod ipv6;
 pub use crate::common::IpNetworkError;
 pub use crate::ipv4::{ipv4_mask_to_prefix, Ipv4Network};
 pub use crate::ipv6::{ipv6_mask_to_prefix, Ipv6Network};
+use crate::ipv4::Ipv4NetworkIterator;
+use crate::ipv6::Ipv6NetworkIterator;
 
 /// Represents a generic network range. This type can have two variants:
 /// the v4 and the v6 case.
@@ -247,6 +249,17 @@ impl IpNetwork {
             IpNetwork::V6(ref ip) => NetworkSize::V6(ip.size()),
         }
     }
+
+    /// Returns an iterator over the addresses contained in the network.
+    ///
+    /// This lists all the addresses in the network range, in ascending order.
+    pub fn iter(&self) -> IpNetworkIterator {
+        let inner = match self {
+            IpNetwork::V4(ip) => IpNetworkIteratorInner::V4(ip.iter()),
+            IpNetwork::V6(ip) => IpNetworkIteratorInner::V6(ip.iter()),
+        };
+        IpNetworkIterator { inner }
+    }
 }
 
 /// Tries to parse the given string into a `IpNetwork`. Will first try to parse
@@ -306,6 +319,34 @@ impl fmt::Display for IpNetwork {
     }
 }
 
+#[derive(Clone, Debug)]
+enum IpNetworkIteratorInner {
+    V4(Ipv4NetworkIterator),
+    V6(Ipv6NetworkIterator),
+}
+
+pub struct IpNetworkIterator {
+    inner: IpNetworkIteratorInner,
+}
+
+impl Iterator for IpNetworkIterator {
+    type Item = IpAddr;
+    fn next(&mut self) -> Option<IpAddr> {
+        match &mut self.inner {
+            IpNetworkIteratorInner::V4(iter) => iter.next().map(IpAddr::V4),
+            IpNetworkIteratorInner::V6(iter) => iter.next().map(IpAddr::V6),
+        }
+    }
+}
+
+impl IntoIterator for &'_ IpNetwork {
+    type IntoIter = IpNetworkIterator;
+    type Item = IpAddr;
+    fn into_iter(self) -> IpNetworkIterator {
+        self.iter()
+    }
+}
+
 /// Converts a `IpAddr` network mask into a prefix.
 /// If the mask is invalid this will return an `IpNetworkError::InvalidPrefix`.
 pub fn ip_mask_to_prefix(mask: IpAddr) -> Result<u8, IpNetworkError> {


Test patch: