Merge pull request #878 from thvdveld/modify-ndisc-parsing

NDISC: option parsing is the same for all NDISC types
smoltcp-rs · Dec 19, 2023 · 8b59d57 · 8b59d57
2 parents 8a86a5a + 2d862b2
commit 8b59d57
Showing 1 changed file with 49 additions and 97 deletions.
diff --git a/src/wire/ndisc.rs b/src/wire/ndisc.rs
@@ -230,109 +230,61 @@ impl<'a> Repr<'a> {
     where
         T: AsRef<[u8]> + ?Sized,
     {
-        fn foreach_option<'a>(
-            payload: &'a [u8],
-            mut f: impl FnMut(NdiscOptionRepr<'a>) -> Result<()>,
-        ) -> Result<()> {
-            let mut offset = 0;
-            while payload.len() > offset {
-                let pkt = NdiscOption::new_checked(&payload[offset..])?;
-
-                // If an option doesn't parse, ignore it and still parse the others.
-                if let Ok(opt) = NdiscOptionRepr::parse(&pkt) {
-                    f(opt)?;
+        let (mut src_ll_addr, mut mtu, mut prefix_info, mut target_ll_addr, mut redirected_hdr) =
+            (None, None, None, None, None);
+
+        let mut offset = 0;
+        while packet.payload().len() > offset {
+            let pkt = NdiscOption::new_checked(&packet.payload()[offset..])?;
+
+            // If an option doesn't parse, ignore it and still parse the others.
+            if let Ok(opt) = NdiscOptionRepr::parse(&pkt) {
+                match opt {
+                    NdiscOptionRepr::SourceLinkLayerAddr(addr) => src_ll_addr = Some(addr),
+                    NdiscOptionRepr::TargetLinkLayerAddr(addr) => target_ll_addr = Some(addr),
+                    NdiscOptionRepr::PrefixInformation(prefix) => prefix_info = Some(prefix),
+                    NdiscOptionRepr::RedirectedHeader(redirect) => redirected_hdr = Some(redirect),
+                    NdiscOptionRepr::Mtu(m) => mtu = Some(m),
+                    _ => {}
                 }
+            }
 
-                let len = pkt.data_len() as usize * 8;
-                if len == 0 {
-                    return Err(Error);
-                }
-                offset += len;
+            let len = pkt.data_len() as usize * 8;
+            if len == 0 {
+                return Err(Error);
             }
-            Ok(())
+            offset += len;
         }
 
         match packet.msg_type() {
-            Message::RouterSolicit => {
-                let mut lladdr = None;
-                foreach_option(packet.payload(), |opt| {
-                    match opt {
-                        NdiscOptionRepr::SourceLinkLayerAddr(addr) => lladdr = Some(addr),
-                        _ => {}
-                    }
-                    Ok(())
-                })?;
-                Ok(Repr::RouterSolicit { lladdr })
-            }
-            Message::RouterAdvert => {
-                let (mut lladdr, mut mtu, mut prefix_info) = (None, None, None);
-                foreach_option(packet.payload(), |opt| {
-                    match opt {
-                        NdiscOptionRepr::SourceLinkLayerAddr(addr) => lladdr = Some(addr),
-                        NdiscOptionRepr::Mtu(val) => mtu = Some(val),
-                        NdiscOptionRepr::PrefixInformation(info) => prefix_info = Some(info),
-                        _ => {}
-                    }
-                    Ok(())
-                })?;
-                Ok(Repr::RouterAdvert {
-                    hop_limit: packet.current_hop_limit(),
-                    flags: packet.router_flags(),
-                    router_lifetime: packet.router_lifetime(),
-                    reachable_time: packet.reachable_time(),
-                    retrans_time: packet.retrans_time(),
-                    lladdr,
-                    mtu,
-                    prefix_info,
-                })
-            }
-            Message::NeighborSolicit => {
-                let mut lladdr = None;
-                foreach_option(packet.payload(), |opt| {
-                    match opt {
-                        NdiscOptionRepr::SourceLinkLayerAddr(addr) => lladdr = Some(addr),
-                        _ => {}
-                    }
-                    Ok(())
-                })?;
-                Ok(Repr::NeighborSolicit {
-                    target_addr: packet.target_addr(),
-                    lladdr,
-                })
-            }
-            Message::NeighborAdvert => {
-                let mut lladdr = None;
-                foreach_option(packet.payload(), |opt| {
-                    match opt {
-                        NdiscOptionRepr::TargetLinkLayerAddr(addr) => lladdr = Some(addr),
-                        _ => {}
-                    }
-                    Ok(())
-                })?;
-                Ok(Repr::NeighborAdvert {
-                    flags: packet.neighbor_flags(),
-                    target_addr: packet.target_addr(),
-                    lladdr,
-                })
-            }
-            Message::Redirect => {
-                let (mut lladdr, mut redirected_hdr) = (None, None);
-
-                foreach_option(packet.payload(), |opt| {
-                    match opt {
-                        NdiscOptionRepr::SourceLinkLayerAddr(addr) => lladdr = Some(addr),
-                        NdiscOptionRepr::RedirectedHeader(rh) => redirected_hdr = Some(rh),
-                        _ => {}
-                    }
-                    Ok(())
-                })?;
-                Ok(Repr::Redirect {
-                    target_addr: packet.target_addr(),
-                    dest_addr: packet.dest_addr(),
-                    lladdr,
-                    redirected_hdr,
-                })
-            }
+            Message::RouterSolicit => Ok(Repr::RouterSolicit {
+                lladdr: src_ll_addr,
+            }),
+            Message::RouterAdvert => Ok(Repr::RouterAdvert {
+                hop_limit: packet.current_hop_limit(),
+                flags: packet.router_flags(),
+                router_lifetime: packet.router_lifetime(),
+                reachable_time: packet.reachable_time(),
+                retrans_time: packet.retrans_time(),
+                lladdr: src_ll_addr,
+                mtu,
+                prefix_info,
+            }),
+            Message::NeighborSolicit => Ok(Repr::NeighborSolicit {
+                target_addr: packet.target_addr(),
+                lladdr: src_ll_addr,
+            }),
+            Message::NeighborAdvert => Ok(Repr::NeighborAdvert {
+                flags: packet.neighbor_flags(),
+                target_addr: packet.target_addr(),
+                lladdr: target_ll_addr,
+            }),
+            Message::Redirect => Ok(Repr::Redirect {
+                target_addr: packet.target_addr(),
+                dest_addr: packet.dest_addr(),
+                lladdr: src_ll_addr,
+                redirected_hdr,
+            }),
             _ => Err(Error),
         }
     }