restructure code and introduce builders

before the structs used a `&mut` directly, this was kind of unflexible.
The code is changed to use builders for the `mutable` part. Parsing can
only be done on the immutable struct versions.

5 files changed, 219 insertions, 139 deletions

diff --git a/src/eth.rs b/src/eth.rs
index 3c5292d..b13d707 100644
--- a/src/eth.rs
+++ b/src/eth.rs
@@ -8,7 +8,7 @@ use crate::error::ParseError;
 pub struct Header<'a>(&'a mut [u8]);
 
 impl<'a> Header<'a> {
-    pub fn from_buffer(buffer: &'a mut [u8]) -> Result<Self, ParseError> {
+    pub fn with_buffer(buffer: &'a mut [u8]) -> Result<Self, ParseError> {
         if buffer.len() < 14 {
             return Err(ParseError::BufferTooSmall(buffer.len()));
         }
@@ -20,31 +20,50 @@ impl<'a> Header<'a> {
         (&self.0[6..12]).try_into().unwrap()
     }
 
-    pub fn set_src_address(&mut self, addr: [u8; 6]) {
-        self.0[6..12].copy_from_slice(&addr);
-    }
-
     pub fn dst_address(&self) -> &[u8; 6] {
         (&self.0[..6]).try_into().unwrap()
     }
 
-    pub fn set_dst_address(&mut self, addr: [u8; 6]) {
-        self.0[..6].copy_from_slice(&addr);
-    }
-
     pub fn ether_type(&self) -> u16 {
         NE::read_u16(&self.0[12..])
     }
 
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.0
+    }
+}
+
+pub struct HeaderBuilder<'a>(&'a mut [u8]);
+
+impl<'a> HeaderBuilder<'a> {
+    pub fn with_buffer(buffer: &'a mut [u8]) -> Result<Self, ParseError> {
+        if buffer.len() < 14 {
+            return Err(ParseError::BufferTooSmall(buffer.len()));
+        }
+        Ok(Self(buffer))
+    }
+
+    pub fn set_src_address(&mut self, addr: [u8; 6]) {
+        self.0[6..12].copy_from_slice(&addr);
+    }
+
+    pub fn set_dst_address(&mut self, addr: [u8; 6]) {
+        self.0[..6].copy_from_slice(&addr);
+    }
+
     pub fn set_ether_type(&mut self, ether_type: u16) {
         NE::write_u16(&mut self.0[12..], ether_type);
     }
 
-    pub fn payload(&self) -> &[u8] {
-        &self.0[14..]
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.0
+    }
+
+    pub fn as_mut_bytes(&mut self) -> &mut [u8] {
+        &mut self.0
     }
 
-    pub fn payload_mut(&mut self) -> &mut [u8] {
-        &mut self.0[14..]
+    pub fn build(self) -> Result<Header<'a>, ParseError> {
+        Header::with_buffer(self.0)
     }
 }
diff --git a/src/header.rs b/src/header.rs
index 8e2bae8..91b6ef0 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -35,19 +35,26 @@ impl Code {
     }
 }
 
+fn ensure_minimal_buffer_length(buffer: &[u8]) -> Result<(), ParseError> {
+    if buffer.len() < 6 {
+        return Err(ParseError::BufferTooSmall(buffer.len()));
+    }
+    Ok(())
+}
+
 #[derive(Debug)]
-pub struct Header<'a>(&'a mut [u8]);
+pub struct Header<'a>(&'a [u8]);
 
 impl<'a> Header<'a> {
-    pub fn from_buffer(buffer: &mut [u8]) -> Result<Header, ParseError> {
-        Self::from_buffer_with_code(buffer, None)
+    pub fn with_buffer(buffer: &'a [u8]) -> Result<Self, ParseError> {
+        Self::with_buffer_and_code(buffer, None)
     }
 
-    pub fn from_buffer_with_code(
-        buffer: &mut [u8],
+    pub fn with_buffer_and_code(
+        buffer: &'a [u8],
         expected_code: Option<Code>,
-    ) -> Result<Header, ParseError> {
-        Self::ensure_minimal_buffer_length(buffer)?;
+    ) -> Result<Header<'a>, ParseError> {
+        ensure_minimal_buffer_length(buffer)?;
         if buffer[0] != 0x11 {
             let version = buffer[0] >> 4;
             let r#type = buffer[0] & 0x0f;
@@ -75,36 +82,37 @@ impl<'a> Header<'a> {
             return Err(ParseError::MissingServiceName);
         }
 
-        Self::validate_tags(&mut buffer[6..6 + length])?;
+        Self::validate_tags(&buffer[6..6 + length])?;
 
         Ok(Header(buffer))
     }
 
-    pub fn padi_from_buffer(buffer: &mut [u8]) -> Result<Header, ParseError> {
-        Self::from_buffer_with_code(buffer, Some(Code::Padi))
+    pub fn padi_with_buffer(buffer: &'a [u8]) -> Result<Self, ParseError> {
+        Self::with_buffer_and_code(buffer, Some(Code::Padi))
     }
 
-    pub fn pado_from_buffer(buffer: &mut [u8]) -> Result<Header, ParseError> {
-        Self::from_buffer_with_code(buffer, Some(Code::Pado))
+    pub fn pado_with_buffer(buffer: &'a [u8]) -> Result<Header<'a>, ParseError> {
+        Self::with_buffer_and_code(buffer, Some(Code::Pado))
     }
 
-    pub fn padr_from_buffer(buffer: &mut [u8]) -> Result<Header, ParseError> {
-        Self::from_buffer_with_code(buffer, Some(Code::Padr))
+    pub fn padr_with_buffer(buffer: &'a [u8]) -> Result<Header<'a>, ParseError> {
+        Self::with_buffer_and_code(buffer, Some(Code::Padr))
     }
 
-    pub fn pads_from_buffer(buffer: &mut [u8]) -> Result<Header, ParseError> {
-        Self::from_buffer_with_code(buffer, Some(Code::Pads))
+    pub fn pads_with_buffer(buffer: &'a [u8]) -> Result<Header<'a>, ParseError> {
+        Self::with_buffer_and_code(buffer, Some(Code::Pads))
     }
 
-    pub fn padt_from_buffer(buffer: &mut [u8]) -> Result<Header, ParseError> {
-        Self::from_buffer_with_code(buffer, Some(Code::Padt))
+    pub fn padt_with_buffer(buffer: &'a [u8]) -> Result<Header<'a>, ParseError> {
+        Self::with_buffer_and_code(buffer, Some(Code::Padt))
     }
 
-    fn ensure_minimal_buffer_length(buffer: &mut [u8]) -> Result<(), ParseError> {
-        if buffer.len() < 6 {
-            return Err(ParseError::BufferTooSmall(buffer.len()));
-        }
-        Ok(())
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.0
+    }
+
+    pub fn get_ref(&self) -> &[u8] {
+        &self.0
     }
 
     fn check_duplicate(tag: u16, exists: &mut bool) -> Result<(), ParseError> {
@@ -192,8 +200,34 @@ impl<'a> Header<'a> {
         self.0[1]
     }
 
-    pub fn set_code(&mut self, code: Code) {
-        self.0[1] = code as u8;
+    pub fn session_id(&self) -> u16 {
+        NE::read_u16(&self.0[2..])
+    }
+
+    pub fn len(&self) -> usize {
+        usize::from(6 + NE::read_u16(&self.0[4..]))
+    }
+
+    pub fn is_empty(&mut self) -> bool {
+        self.len() == 6
+    }
+
+    pub fn payload(&self) -> &[u8] {
+        &self.0[6..]
+    }
+
+    pub fn tag_iter(&self) -> TagIterator {
+        TagIterator {
+            payload: &self.0[6..self.len()],
+        }
+    }
+}
+
+pub struct HeaderBuilder<'a>(&'a mut [u8]);
+
+impl<'a> HeaderBuilder<'a> {
+    pub fn code(&self) -> u8 {
+        self.0[1]
     }
 
     pub fn session_id(&self) -> u16 {
@@ -204,16 +238,28 @@ impl<'a> Header<'a> {
         usize::from(6 + NE::read_u16(&self.0[4..]))
     }
 
-    unsafe fn set_len(&mut self, new_length: u16) {
-        NE::write_u16(&mut self.0[4..], new_length)
+    pub fn is_empty(&mut self) -> bool {
+        self.len() == 6
     }
 
     pub fn payload(&self) -> &[u8] {
         &self.0[6..]
     }
 
+    pub fn tag_iter(&self) -> TagIterator {
+        TagIterator {
+            payload: &self.0[6..self.len()],
+        }
+    }
+    pub fn set_code(&mut self, code: Code) {
+        self.0[1] = code as u8;
+    }
+
+    unsafe fn set_len(&mut self, new_length: u16) {
+        NE::write_u16(&mut self.0[4..], new_length)
+    }
+
     pub fn clear_payload(&mut self) {
-        //  NE::write_u16(&mut self.0[4..], 0)
         unsafe { self.set_len(0) };
     }
 
@@ -223,16 +269,12 @@ impl<'a> Header<'a> {
         }
     }
 
-    pub fn is_empty(&mut self) -> bool {
-        self.len() == 6
-    }
-
     pub fn create_packet(
-        buffer: &mut [u8],
+        buffer: &'a mut [u8],
         code: Code,
         session_id: u16,
-    ) -> Result<Header, ParseError> {
-        Self::ensure_minimal_buffer_length(buffer)?;
+    ) -> Result<Self, ParseError> {
+        ensure_minimal_buffer_length(buffer)?;
 
         // set version and type
         buffer[0] = 0x11;
@@ -240,35 +282,35 @@ impl<'a> Header<'a> {
         NE::write_u16(&mut buffer[2..], session_id);
         NE::write_u16(&mut buffer[4..], 0);
 
-        Ok(Header(buffer))
+        Ok(HeaderBuilder(buffer))
     }
 
-    pub fn create_padi(buffer: &mut [u8]) -> Result<Header, ParseError> {
+    pub fn create_padi(buffer: &'a mut [u8]) -> Result<Self, ParseError> {
         Self::create_packet(buffer, Code::Padi, 0)
     }
 
-    pub fn create_pado(buffer: &mut [u8]) -> Result<Header, ParseError> {
+    pub fn create_pado(buffer: &'a mut [u8]) -> Result<Self, ParseError> {
         Self::create_packet(buffer, Code::Pado, 0)
     }
 
-    pub fn create_padr(buffer: &mut [u8]) -> Result<Header, ParseError> {
+    pub fn create_padr(buffer: &'a mut [u8]) -> Result<Self, ParseError> {
         Self::create_packet(buffer, Code::Padr, 0)
     }
 
-    pub fn create_pads(buffer: &mut [u8], session_id: NonZeroU16) -> Result<Header, ParseError> {
+    pub fn create_pads(buffer: &'a mut [u8], session_id: NonZeroU16) -> Result<Self, ParseError> {
         Self::create_packet(buffer, Code::Pads, u16::from(session_id))
     }
 
-    pub fn create_padt(buffer: &mut [u8], session_id: NonZeroU16) -> Result<Header, ParseError> {
+    pub fn create_padt(buffer: &'a mut [u8], session_id: NonZeroU16) -> Result<Self, ParseError> {
         Self::create_packet(buffer, Code::Padt, u16::from(session_id))
     }
 
     pub fn create_padr_from_pado(
         buffer: &'a mut [u8],
-        pado: &Self,
+        pado: &Header,
         expected_service_name: Option<&[u8]>,
         expected_ac_name: Option<&[u8]>,
-    ) -> Result<Header<'a>, ParseError> {
+    ) -> Result<Self, ParseError> {
         let mut padr = Self::create_padr(buffer)?;
 
         let mut tag_iterator = pado.tag_iter();
@@ -315,12 +357,6 @@ impl<'a> Header<'a> {
         Ok(padr)
     }
 
-    pub fn tag_iter(&self) -> TagIterator {
-        TagIterator {
-            payload: &self.0[6..self.len()],
-        }
-    }
-
     pub fn add_tag(&mut self, tag: Tag) -> Result<(), ParseError> {
         let packet_length = self.len();
 
@@ -350,28 +386,12 @@ impl<'a> Header<'a> {
         self.add_tag(Tag::EndOfList)
     }
 
-    pub fn as_bytes(&self) -> &[u8] {
-        self.as_ref()
-    }
-
     pub fn get_ref_mut(&mut self) -> &mut [u8] {
         &mut self.0
     }
 
-    pub fn get_ref(&self) -> &[u8] {
-        &self.0
-    }
-}
-
-impl<'a> AsRef<[u8]> for Header<'a> {
-    fn as_ref(&self) -> &[u8] {
-        self.0
-    }
-}
-
-impl<'a> Into<&'a [u8]> for Header<'a> {
-    fn into(self) -> &'a [u8] {
-        self.0
+    pub fn build(self) -> Result<Header<'a>, ParseError> {
+        Header::with_buffer(self.0)
     }
 }
 
@@ -392,34 +412,34 @@ mod tests {
         }}
     }
 
-    fn minimal_header<'a>(buffer: &'a mut [u8], service_name: Option<&[u8]>) -> Header<'a> {
-        let mut header = Header::create_padi(&mut buffer[..]).unwrap();
+    fn minimal_header<'a>(buffer: &'a mut [u8], service_name: Option<&[u8]>) -> HeaderBuilder<'a> {
+        let mut builder = HeaderBuilder::create_padi(&mut buffer[..]).unwrap();
         if let Some(service_name) = service_name {
-            header.add_tag(Tag::ServiceName(service_name)).unwrap();
+            builder.add_tag(Tag::ServiceName(service_name)).unwrap();
         } else {
-            header.add_tag(Tag::ServiceName(b"")).unwrap();
+            builder.add_tag(Tag::ServiceName(b"")).unwrap();
         }
-        header
+        builder
     }
 
     fn minimal_header_with_eol<'a>(
         buffer: &'a mut [u8],
         service_name: Option<&[u8]>,
-    ) -> Header<'a> {
-        let mut header = minimal_header(buffer, service_name);
-        header.add_tag(Tag::EndOfList).unwrap();
-        header
+    ) -> HeaderBuilder<'a> {
+        let mut builder = minimal_header(buffer, service_name);
+        builder.add_tag(Tag::EndOfList).unwrap();
+        builder
     }
 
-    fn expect_parse_error(buffer: &mut [u8]) -> ParseError {
-        let should_be_error = Header::from_buffer(&mut buffer[..]);
+    fn expect_parse_error(buffer: &[u8]) -> ParseError {
+        let should_be_error = Header::with_buffer(&buffer[..]);
         assert!(should_be_error.is_err());
         should_be_error.unwrap_err()
     }
 
     #[test]
     fn duplicate_tag_detection() {
-        let buffer = &mut [0u8; 200];
+        let buffer = &mut [0u8; 200][..];
         let content = b"abcdef123";
         let tags = create_tags!(
             content,
@@ -430,7 +450,7 @@ mod tests {
         );
 
         for (id, tag) in tags {
-            let mut header = Header::create_padi(buffer).unwrap();
+            let mut header = HeaderBuilder::create_padi(buffer).unwrap();
             header.add_tag(tag).unwrap();
             header.add_tag(tag).unwrap();
 
@@ -441,7 +461,7 @@ mod tests {
 
     #[test]
     fn reported_length_bigger_than_packet() {
-        let buffer = &mut [0u8; 200];
+        let buffer = &mut [0u8; 200][..];
         let mut header = minimal_header_with_eol(buffer, None);
         unsafe { header.set_len(555) };
 
@@ -465,7 +485,7 @@ mod tests {
     #[test]
     fn buffer_less_than_minimal_required_size_for_creation() {
         let buffer = &mut [0u8; 4];
-        assert!(Header::create_padi(buffer).is_err());
+        assert!(HeaderBuilder::create_padi(buffer).is_err());
     }
 
     #[test]
@@ -519,7 +539,7 @@ mod tests {
             _ => false,
         });
 
-        Header::create_padi(buffer)
+        HeaderBuilder::create_padi(buffer)
             .unwrap()
             .add_tag(Tag::EndOfList)
             .unwrap();
@@ -533,17 +553,17 @@ mod tests {
 
     #[test]
     fn pppoe_code_conditional_parsing() {
-        let buffer = &mut [0u8; 200];
+        let buffer = &mut [0u8; 200][..];
         let codes = [Code::Padi, Code::Pado, Code::Padr, Code::Pads, Code::Padt];
         let session_ids = [0, 0, 0, 1, 1];
 
         for (i, code) in codes.iter().cloned().enumerate() {
-            Header::create_packet(buffer, code, session_ids[i])
+            HeaderBuilder::create_packet(buffer, code, session_ids[i])
                 .unwrap()
                 .add_tag(Tag::ServiceName(b"abc"))
                 .unwrap();
             for (j, code) in codes.iter().cloned().enumerate() {
-                let header = Header::from_buffer_with_code(buffer, Some(code));
+                let header = Header::with_buffer_and_code(buffer, Some(code));
                 if j == i {
                     assert!(header.is_ok())
                 } else {
diff --git a/src/lib.rs b/src/lib.rs
index de1015e..823838d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -4,10 +4,10 @@ pub mod socket;
 pub use socket::Socket;
 
 pub mod header;
-pub use header::{Code, Header};
+pub use header::{Code, Header, HeaderBuilder};
 
 pub mod packet;
-pub use packet::Packet;
+pub use packet::{Packet, PacketBuilder};
 
 pub mod error;
 pub mod eth;
@@ -29,7 +29,7 @@ mod tests {
 
         let mut receive_buffer = [0u8; 1450];
         let mut buffer = [0u8; 1450];
-        let mut packet = Packet::new_discovery_packet(
+        let mut packet = PacketBuilder::new_discovery_packet(
             &mut buffer[..],
             [0xfe, 0xb9, 0x04, 0x2a, 0xb2, 0x35],
             [0xff, 0xff, 0xff, 0xff, 0xff, 0xff],
@@ -37,15 +37,19 @@ mod tests {
         .unwrap();
 
         {
-            let pppoe_header = packet.pppoe_header_mut();
+            let pppoe_header = packet.pppoe_header();
             pppoe_header.add_tag(Tag::PppMaxMtu(2000)).unwrap();
             pppoe_header.add_tag(Tag::ServiceName(b"\0")).unwrap();
             pppoe_header.add_tag(Tag::RelaySessionId(b"abc")).unwrap();
-            pppoe_header.add_tag(Tag::HostUniq(b"abcanretadi\0arnedt")).unwrap();
-            pppoe_header.add_vendor_tag_with_callback(|buffer| {
-                Tr101Information::with_both_ids("circuit", "remoteid")
-                    .and_then(|tr101| tr101.write(buffer))
-            }).unwrap();
+            pppoe_header
+                .add_tag(Tag::HostUniq(b"abcanretadi\0arnedt"))
+                .unwrap();
+            pppoe_header
+                .add_vendor_tag_with_callback(|buffer| {
+                    Tr101Information::with_both_ids("circuit", "remoteid")
+                        .and_then(|tr101| tr101.write(buffer))
+                })
+                .unwrap();
             pppoe_header.add_tag(Tag::EndOfList).unwrap();
         }
 
@@ -53,12 +57,12 @@ mod tests {
         assert!(ret.is_ok());
 
         let len = sock.recv(&mut receive_buffer[..]).unwrap();
-        let pado = Packet::from_buffer(&mut receive_buffer[..len]).unwrap();
+        let pado = Packet::with_buffer(&mut receive_buffer[..len]).unwrap();
 
         {
             let dst = pado.ethernet_header().src_address();
-            packet.ethernet_header_mut().set_dst_address(dst);
-            let pppoe_header = packet.pppoe_header_mut();
+            packet.ethernet_header().set_dst_address(dst);
+            let pppoe_header = packet.pppoe_header();
             pppoe_header.set_code(Code::Padr);
             pppoe_header.clear_eol();
 
@@ -74,6 +78,6 @@ mod tests {
         let ret = sock.send(packet.as_bytes());
         assert!(ret.is_ok());
 
-        let len = sock.recv(&mut receive_buffer[..]).unwrap();
+        let _len = sock.recv(&mut receive_buffer[..]).unwrap();
     }
 }
diff --git a/src/packet.rs b/src/packet.rs
index 99f0bab..ecf2611 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -6,6 +6,14 @@ use std::slice;
 pub const PPPOE_DISCOVERY: u16 = 0x8863;
 pub const PPPOE_SESSION: u16 = 0x8864;
 
+fn ensure_minimal_buffer_size(buffer: &mut [u8]) -> Result<(), ParseError> {
+    // minimal eth + pppoe header size
+    if buffer.len() < 20 {
+        return Err(ParseError::BufferTooSmall(buffer.len()));
+    }
+    Ok(())
+}
+
 #[derive(Debug)]
 pub struct Packet<'a> {
     ethernet: eth::Header<'a>,
@@ -13,65 +21,94 @@ pub struct Packet<'a> {
 }
 
 impl<'a> Packet<'a> {
-    pub fn from_buffer(buffer: &'a mut [u8]) -> Result<Self, Error> {
-        Self::ensure_minimal_buffer_size(buffer)?;
+    pub fn with_buffer(buffer: &'a mut [u8]) -> Result<Self, Error> {
+        ensure_minimal_buffer_size(buffer)?;
         let (eth_buf, pppoe_buf) = buffer.split_at_mut(14);
 
         Ok(Self {
-            ethernet: eth::Header::from_buffer(eth_buf)?,
-            pppoe: pppoe::Header::from_buffer(pppoe_buf)?,
+            ethernet: eth::Header::with_buffer(eth_buf)?,
+            pppoe: pppoe::Header::with_buffer(pppoe_buf)?,
         })
     }
 
-    fn ensure_minimal_buffer_size(buffer: &mut [u8]) -> Result<(), ParseError> {
-        // minimal eth + pppoe header size
-        if buffer.len() < 20 {
-            return Err(ParseError::BufferTooSmall(buffer.len()).into());
-        }
-        Ok(())
+    pub fn pppoe_header(&self) -> &pppoe::Header {
+        &self.pppoe
+    }
+
+    pub fn ethernet_header(&self) -> &eth::Header<'a> {
+        &self.ethernet
+    }
+
+    pub fn len(&self) -> usize {
+        14 + self.pppoe.len()
+    }
+
+    pub fn is_empty(&self) -> bool {
+        false
+    }
+
+    pub fn as_bytes(&self) -> &[u8] {
+        let ptr = self.ethernet.dst_address().as_ptr();
+        unsafe { slice::from_raw_parts(ptr, self.len()) }
     }
+}
 
+pub struct PacketBuilder<'a> {
+    ethernet: eth::HeaderBuilder<'a>,
+    pppoe: pppoe::HeaderBuilder<'a>,
+}
+
+impl<'a> PacketBuilder<'a> {
     pub fn new_discovery_packet(
         buffer: &'a mut [u8],
         src_mac: [u8; 6],
         dst_mac: [u8; 6],
     ) -> Result<Self, Error> {
-        Self::ensure_minimal_buffer_size(buffer)?;
+        ensure_minimal_buffer_size(buffer)?;
 
         let (eth_buf, pppoe_buf) = buffer.split_at_mut(14);
-        let mut ethernet = eth::Header::from_buffer(eth_buf)?;
+        let mut ethernet = eth::HeaderBuilder::with_buffer(eth_buf)?;
         ethernet.set_src_address(src_mac);
         ethernet.set_dst_address(dst_mac);
         ethernet.set_ether_type(PPPOE_DISCOVERY);
 
         Ok(Self {
             ethernet,
-            pppoe: pppoe::Header::create_padi(pppoe_buf)?,
+            pppoe: pppoe::HeaderBuilder::create_padi(pppoe_buf)?,
         })
     }
 
-    pub fn pppoe_header(&self) -> &pppoe::Header {
-        &self.pppoe
+    pub fn len(&self) -> usize {
+        14 + self.pppoe.len()
     }
 
-    pub fn pppoe_header_mut(&mut self) -> &mut pppoe::Header<'a> {
-        &mut self.pppoe
+    pub fn is_empty(&self) -> bool {
+        false
     }
 
-    pub fn ethernet_header(&self) -> &eth::Header {
-        &self.ethernet
+    pub fn pppoe_header(&mut self) -> &mut pppoe::HeaderBuilder<'a> {
+        &mut self.pppoe
     }
 
-    pub fn ethernet_header_mut(&mut self) -> &mut eth::Header<'a> {
+    pub fn ethernet_header(&mut self) -> &mut eth::HeaderBuilder<'a> {
         &mut self.ethernet
     }
 
-    pub fn len(&self) -> usize {
-        14 + self.pppoe.len()
-    }
-
     pub fn as_bytes(&self) -> &[u8] {
-        let ptr = self.ethernet.dst_address().as_ptr();
+        let ptr = self.ethernet.as_bytes().as_ptr();
         unsafe { slice::from_raw_parts(ptr, self.len()) }
     }
+
+    pub fn as_mut_bytes(&mut self) -> &mut [u8] {
+        let ptr = self.ethernet.as_mut_bytes().as_mut_ptr();
+        // TODO: this could be undefined behaviour... need to check
+        unsafe { slice::from_raw_parts_mut(ptr, self.len()) }
+    }
+
+    pub fn build(self) -> Result<Packet<'a>, Error> {
+        Ok(Packet {
+            ethernet: self.ethernet.build()?,
+            pppoe: self.pppoe.build()?,
+        })
+    }
 }
diff --git a/src/tags/tag.rs b/src/tags/tag.rs
index 4067d4e..a1bb109 100644
--- a/src/tags/tag.rs
+++ b/src/tags/tag.rs
@@ -169,7 +169,7 @@ impl<'a> Tag<'a> {
             | Tag::AcSystemError(msg)
             | Tag::GenericError(msg)
             | Tag::Unknown((_, msg)) => {
-                str::from_utf8(msg).map(|msg| if msg.len() == 0 { None } else { Some(msg) })
+                str::from_utf8(msg).map(|msg| if msg.is_empty() { None } else { Some(msg) })
             }
             _ => Ok(None),
         }
@@ -235,7 +235,7 @@ impl<'a> Iterator for TagIterator<'a> {
     type Item = Tag<'a>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        if self.payload.len() == 0 {
+        if self.payload.is_empty() {
             return None;
         }