openpgp: Move fields from Common to Container.

6 files changed, 124 insertions, 131 deletions

diff --git a/openpgp/src/message/mod.rs b/openpgp/src/message/mod.rs
index 51c548e0..df38cf12 100644
--- a/openpgp/src/message/mod.rs
+++ b/openpgp/src/message/mod.rs
@@ -454,7 +454,6 @@ mod tests {
     use crate::crypto::mpis::{Ciphertext, MPI};
     use crate::packet::prelude::*;
     use crate::KeyID;
-    use crate::Container;
 
     #[test]
     fn tokens() {
@@ -998,7 +997,6 @@ mod tests {
         //  1: MDC
         // => good.
         let mut seip = SEIP1::new();
-        seip.set_children(Some(Container::new()));
         seip.children_mut().unwrap().push(
             lit.clone().into());
         seip.children_mut().unwrap().push(
diff --git a/openpgp/src/packet/compressed_data.rs b/openpgp/src/packet/compressed_data.rs
index db166ed3..b6914fe9 100644
--- a/openpgp/src/packet/compressed_data.rs
+++ b/openpgp/src/packet/compressed_data.rs
@@ -3,7 +3,6 @@ use std::ops::{Deref, DerefMut};
 
 use crate::packet::{self, Common};
 use crate::Packet;
-use crate::Container;
 use crate::types::CompressionAlgorithm;
 
 /// Holds a compressed data packet.
@@ -29,10 +28,10 @@ impl fmt::Debug for CompressedData {
         f.debug_struct("CompressedData")
             .field("algo", &self.algo)
             .field("children",
-                   &self.common.children.as_ref()
+                   &self.common.children_ref()
                        .map(|c| &c.packets).unwrap_or(&Vec::new()))
             .field("body (bytes)",
-                   &self.common.body.as_ref().unwrap_or(&b"".to_vec()).len())
+                   &self.common.body().unwrap_or(&b"".to_vec()).len())
             .finish()
     }
 }
@@ -58,10 +57,7 @@ impl CompressedData {
 
     /// Adds a new packet to the container.
     pub fn push(mut self, packet: Packet) -> Self {
-        if self.common.children.is_none() {
-            self.common.children = Some(Container::new());
-        }
-        self.common.children.as_mut().unwrap().push(packet);
+        self.common.children_mut().unwrap().packets.push(packet);
         self
     }
 
@@ -70,10 +66,7 @@ impl CompressedData {
     /// container.  If `i` is one, it is inserted after the first
     /// packet, etc.
     pub fn insert(mut self, i: usize, packet: Packet) -> Self {
-        if self.common.children.is_none() {
-            self.common.children = Some(Container::new());
-        }
-        self.common.children.as_mut().unwrap().insert(i, packet);
+        self.common.children_mut().unwrap().packets.insert(i, packet);
         self
     }
 }
diff --git a/openpgp/src/packet/mod.rs b/openpgp/src/packet/mod.rs
index 4cee893d..b0bd705f 100644
--- a/openpgp/src/packet/mod.rs
+++ b/openpgp/src/packet/mod.rs
@@ -103,8 +103,71 @@ impl<'a> DerefMut for Packet {
 }
 
 /// Fields used by multiple packet types.
-#[derive(PartialEq, Eq, Hash, Clone)]
+#[derive(Debug, PartialEq, Eq, Hash, Clone)]
 pub struct Common {
+    /// XXX: Should only be embedded by the container types.
+    container: Container,
+}
+
+impl Default for Common {
+    fn default() -> Common {
+        Common {
+            container: Default::default(),
+        }
+    }
+}
+
+impl Common {
+    pub(crate) // for packet_pile.rs
+    fn children_ref(&self) -> Option<&Container> {
+        self.container.as_ref()
+    }
+
+    pub(crate) // for packet_pile.rs
+    fn children_mut(&mut self) -> Option<&mut Container> {
+        Some(&mut self.container)
+    }
+
+    /// Returns an iterator over the packet's immediate children.
+    pub fn children<'a>(&'a self) -> impl Iterator<Item = &'a Packet> {
+        self.container.children()
+    }
+
+    /// Returns an iterator over all of the packet's descendants, in
+    /// depth-first order.
+    pub fn descendants(&self) -> Iter {
+        self.container.descendants()
+    }
+
+    /// Retrieves the packet's body.
+    ///
+    /// Packets can store a sequence of bytes as body, e.g. if the
+    /// maximum recursion level is reached while parsing a sequence of
+    /// packets, the container's body is stored as is.
+    pub fn body(&self) -> Option<&[u8]> {
+        self.container.body()
+    }
+
+    /// Sets the packet's body.
+    ///
+    /// Setting the body clears the old body, or any of the packet's
+    /// descendants.
+    pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
+        self.container.set_body(data)
+    }
+
+    pub(crate) // For parse.rs
+    fn body_mut(&mut self) -> Option<&mut Vec<u8>> {
+        self.container.body_mut()
+    }
+}
+
+/// Holds zero or more OpenPGP packets.
+///
+/// This is used by OpenPGP container packets, like the compressed
+/// data packet, to store the containing packets.
+#[derive(PartialEq, Eq, Hash, Clone)]
+pub(crate) struct Container {
     /// Used by container packets (such as the encryption and
     /// compression packets) to reference their immediate children.
     /// This results in a tree structure.
@@ -118,7 +181,7 @@ pub struct Common {
     ///   [`PacketPile`]: ../struct.PacketPile.html
     ///   [`PacketPile::from_file`]: ../struct.PacketPile.html#method.from_file
     ///   [`PacketParser`]: ../parse/struct.PacketParser.html
-    children: Option<Container>,
+    pub(crate) packets: Vec<Packet>,
 
     /// Holds a packet's body.
     ///
@@ -168,99 +231,22 @@ pub struct Common {
     body: Option<Vec<u8>>,
 }
 
-impl fmt::Debug for Common {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_struct("Common")
-            .field("children", &self.children)
-            .field("body (bytes)",
-                   &self.body.as_ref().map(|body| body.len()))
-            .finish()
-    }
-}
-
-impl Default for Common {
-    fn default() -> Common {
-        Common {
-            children: None,
+impl Default for Container {
+    fn default() -> Self {
+        Self {
+            packets: Vec::with_capacity(0),
             body: None,
         }
     }
 }
 
-impl Common {
-    pub(crate) // for packet_pile.rs
-    fn children_ref(&self) -> Option<&Container> {
-        self.children.as_ref()
-    }
-
-    pub(crate) // for packet_pile.rs
-    fn children_mut(&mut self) -> Option<&mut Container> {
-        self.children.as_mut()
-    }
-
-    pub(crate) // for packet_pile.rs
-    fn set_children(&mut self, v: Option<Container>) -> Option<Container> {
-        std::mem::replace(&mut self.children, v)
-    }
-
-    fn children_iter<'a>(&'a self) -> slice::Iter<'a, Packet> {
-        if let Some(ref container) = self.children {
-            container.packets.iter()
-        } else {
-            let empty_packet_slice : &[Packet] = &[];
-            empty_packet_slice.iter()
-        }
-    }
-
-    /// Returns an iterator over the packet's immediate children.
-    pub fn children<'a>(&'a self) -> impl Iterator<Item = &'a Packet> {
-        self.children_iter()
-    }
-
-    /// Returns an iterator over all of the packet's descendants, in
-    /// depth-first order.
-    pub fn descendants(&self) -> Iter {
-        return Iter {
-            children: self.children_iter(),
-            child: None,
-            grandchildren: None,
-            depth: 0,
+impl From<Vec<Packet>> for Container {
+    fn from(packets: Vec<Packet>) -> Self {
+        Self {
+            packets,
+            body: None,
         }
     }
-
-    /// Retrieves the packet's body.
-    ///
-    /// Packets can store a sequence of bytes as body, e.g. if the
-    /// maximum recursion level is reached while parsing a sequence of
-    /// packets, the container's body is stored as is.
-    pub fn body(&self) -> Option<&[u8]> {
-        self.body.as_ref().map(|b| b.as_slice())
-    }
-
-    /// Sets the packet's body.
-    ///
-    /// Setting the body clears the old body, or any of the packet's
-    /// descendants.
-    pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
-        self.children = None;
-        ::std::mem::replace(&mut self.body,
-                            if data.len() == 0 { None } else { Some(data) })
-            .unwrap_or(Vec::new())
-    }
-
-    pub(crate) // For parse.rs
-    fn body_mut(&mut self) -> Option<&mut Vec<u8>> {
-        self.body.as_mut()
-    }
-}
-
-/// Holds zero or more OpenPGP packets.
-///
-/// This is used by OpenPGP container packets, like the compressed
-/// data packet, to store the containing packets.
-#[derive(PartialEq, Eq, Hash, Clone)]
-pub(crate) struct Container {
-    pub(crate) packets: Vec<Packet>,
 }
 
 impl fmt::Debug for Container {
@@ -272,8 +258,12 @@ impl fmt::Debug for Container {
 }
 
 impl Container {
-    pub(crate) fn new() -> Container {
-        Container { packets: Vec::with_capacity(8) }
+    pub(crate) fn as_ref(&self) -> Option<&Self> {
+        if ! self.packets.is_empty() {
+            Some(self)
+        } else {
+            None
+        }
     }
 
     // Adds a new packet to the container.
@@ -311,6 +301,31 @@ impl Container {
         self.packets.into_iter()
     }
 
+    /// Retrieves the packet's body.
+    ///
+    /// Packets can store a sequence of bytes as body, e.g. if the
+    /// maximum recursion level is reached while parsing a sequence of
+    /// packets, the container's body is stored as is.
+    pub fn body(&self) -> Option<&[u8]> {
+        self.body.as_ref().map(|b| b.as_slice())
+    }
+
+    /// Sets the packet's body.
+    ///
+    /// Setting the body clears the old body, or any of the packet's
+    /// descendants.
+    pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
+        self.packets.clear();
+        ::std::mem::replace(&mut self.body,
+                            if data.len() == 0 { None } else { Some(data) })
+            .unwrap_or(Vec::new())
+    }
+
+    pub(crate) // For parse.rs
+    fn body_mut(&mut self) -> Option<&mut Vec<u8>> {
+        self.body.as_mut()
+    }
+
     // Converts an indentation level to whitespace.
     fn indent(depth: usize) -> &'static str {
         use std::cmp;
@@ -328,7 +343,7 @@ impl Container {
         for (i, p) in self.packets.iter().enumerate() {
             eprintln!("{}{}: {:?}",
                       Self::indent(indent), i + 1, p);
-            if let Some(ref children) = self.packets[i].children {
+            if let Some(ref children) = self.packets[i].children_ref() {
                 children.pretty_print(indent + 1);
             }
         }
@@ -454,8 +469,8 @@ fn packet_path_iter() {
             v.push(i);
             lpaths.push(v);
 
-            if let Some(ref children) = packet.children {
-                for mut path in paths(children.packets.iter()).into_iter() {
+            if let Some(ref container) = packet.children_ref() {
+                for mut path in paths(container.children()).into_iter() {
                     path.insert(0, i);
                     lpaths.push(path);
                 }
diff --git a/openpgp/src/packet/unknown.rs b/openpgp/src/packet/unknown.rs
index 07cd5e0e..d9bfbfd0 100644
--- a/openpgp/src/packet/unknown.rs
+++ b/openpgp/src/packet/unknown.rs
@@ -41,10 +41,6 @@ impl PartialOrd for Unknown
 impl Ord for Unknown
 {
     fn cmp(&self, other: &Unknown) -> Ordering {
-        // An unknown packet cannot have children.
-        assert!(self.common.children.is_none());
-        assert!(other.common.children.is_none());
-
         match self.tag.cmp(&other.tag) {
             Ordering::Equal => self.common.body().cmp(&other.common.body()),
             o => o,
@@ -110,7 +106,7 @@ impl Unknown {
     /// information, and not encoded using something like OpenPGP's
     /// partial body encoding.
     pub fn body(&self) -> Option<&[u8]> {
-        self.common.body.as_ref().map(|b| b.as_slice())
+        self.common.body()
     }
 
     /// Sets the packet's contents.
@@ -118,8 +114,8 @@ impl Unknown {
     /// This is the raw packet content not include the CTB and length
     /// information, and not encoded using something like OpenPGP's
     /// partial body encoding.
-    pub fn set_body(&mut self, data: Vec<u8>) -> Option<Vec<u8>> {
-        ::std::mem::replace(&mut self.common.body, Some(data))
+    pub fn set_body(&mut self, data: Vec<u8>) -> Vec<u8> {
+        self.common.set_body(data)
     }
 }
 
diff --git a/openpgp/src/packet_pile.rs b/openpgp/src/packet_pile.rs
index a290f841..5b548de1 100644
--- a/openpgp/src/packet_pile.rs
+++ b/openpgp/src/packet_pile.rs
@@ -74,7 +74,7 @@ impl std::str::FromStr for PacketPile {
 
 impl From<Vec<Packet>> for PacketPile {
     fn from(p: Vec<Packet>) -> Self {
-        PacketPile { top_level: Container { packets: p } }
+        PacketPile { top_level: Container::from(p) }
     }
 }
 
@@ -246,11 +246,10 @@ impl PacketPile {
             let p = &mut tmp.packets[i];
             if p.children_ref().is_none() {
                 match p {
-                    Packet::CompressedData(_) | Packet::SEIP(_) => {
-                        // We have a container with no children.
-                        // That's okay.  We can create the container.
-                        p.set_children(Some(Container::new()));
-                    },
+                    Packet::CompressedData(_)
+                        | Packet::SEIP(_)
+                        | Packet::AED(_)
+                        => (), // Ok.
                     _ => {
                         return Err(Error::IndexOutOfRange.into());
                     }
@@ -304,7 +303,7 @@ impl PacketPile {
         }
 
         // Create a top-level container.
-        let mut top_level = Container::new();
+        let mut top_level = Container::default();
 
         let mut last_position = 0;
 
@@ -348,7 +347,6 @@ impl PacketPile {
                     let tmp = container;
                     let i = tmp.packets.len() - 1;
                     assert!(tmp.packets[i].children_ref().is_none());
-                    tmp.packets[i].set_children(Some(Container::new()));
                     container = tmp.packets[i].children_mut().unwrap();
                 }
 
@@ -640,7 +638,6 @@ mod test {
         }
 
         let mut seip = SEIP1::new();
-        seip.set_children(Some(Container::new()));
         seip.children_mut().unwrap().push(cd.into());
         packets.push(seip.into());
 
diff --git a/openpgp/src/parse/packet_pile_parser.rs b/openpgp/src/parse/packet_pile_parser.rs
index 58a93718..223b212b 100644
--- a/openpgp/src/parse/packet_pile_parser.rs
+++ b/openpgp/src/parse/packet_pile_parser.rs
@@ -4,7 +4,6 @@ use std::path::Path;
 use crate::{
     Result,
     Packet,
-    Container,
     PacketPile,
 };
 use crate::parse::{
@@ -109,7 +108,7 @@ impl<'a> PacketPileParser<'a> {
         -> Result<PacketPileParser<'a>>
     {
         Ok(PacketPileParser {
-            pile: PacketPile { top_level: Container::new() },
+            pile: PacketPile { top_level: Default::default() },
             ppr: ppr,
             returned_first: false,
         })
@@ -135,13 +134,8 @@ impl<'a> PacketPileParser<'a> {
             let packets_len = tmp.packets.len();
             let p = &mut tmp.packets[packets_len - 1];
             if p.children().next().is_none() {
-                if i == position - 1 {
-                    // This is the leaf.  Create a new container
-                    // here.
-                    p.set_children(Some(Container::new()));
-                } else {
-                    panic!("Internal inconsistency while building message.");
-                }
+                assert!(i == position - 1,
+                        "Internal inconsistency while building message.");
             }
 
             container = p.children_mut().unwrap();