openpgp: Make fields of SubpacketValue private.

5 files changed, 41 insertions, 30 deletions

diff --git a/openpgp/examples/statistics.rs b/openpgp/examples/statistics.rs
index e2078bcf..138497e3 100644
--- a/openpgp/examples/statistics.rs
+++ b/openpgp/examples/statistics.rs
@@ -118,13 +118,13 @@ fn main() {
                         .chain(sig.unhashed_area().iter())
                     {
                         use crate::openpgp::packet::signature::subpacket::*;
-                        let i = u8::from(sub.tag) as usize;
+                        let i = u8::from(sub.tag()) as usize;
                         sigs_subpacket_tags_count[i] += 1;
                         tpk.sigs_subpacket_tags_count[i] += 1;
                         signature.subpacket_tags_count[i] += 1;
-                        if let SubpacketValue::Unknown(_) = sub.value {
+                        if let SubpacketValue::Unknown(_) = sub.value() {
                             sigs_subpacket_tags_unknown
-                                [u8::from(sub.tag) as usize] += 1;
+                                [u8::from(sub.tag()) as usize] += 1;
                         } else {
                             sigs_subpacket_tags_size_bytes[i] += len;
                             sigs_subpacket_tags_size_count[i] += 1;
@@ -136,7 +136,7 @@ fn main() {
                                 sigs_subpacket_tags_size_max[i] = len;
                             }
 
-                            match sub.value {
+                            match sub.value() {
                                 SubpacketValue::Unknown(_) => unreachable!(),
                                 SubpacketValue::KeyFlags(k) =>
                                     if let Some(count) = key_flags.get_mut(&k) {
@@ -146,34 +146,34 @@ fn main() {
                                     },
                                 SubpacketValue::PreferredSymmetricAlgorithms(a)
                                     =>
-                                    if let Some(count) = p_sym.get_mut(&a) {
+                                    if let Some(count) = p_sym.get_mut(a) {
                                         *count += 1;
                                     } else {
                                         p_sym.insert(a.clone(), 1);
                                     },
                                 SubpacketValue::PreferredHashAlgorithms(a)
                                     =>
-                                    if let Some(count) = p_hashes.get_mut(&a) {
+                                    if let Some(count) = p_hashes.get_mut(a) {
                                         *count += 1;
                                     } else {
                                         p_hashes.insert(a.clone(), 1);
                                     },
                                 SubpacketValue::PreferredCompressionAlgorithms(a)
                                     =>
-                                    if let Some(count) = p_comp.get_mut(&a) {
+                                    if let Some(count) = p_comp.get_mut(a) {
                                         *count += 1;
                                     } else {
                                         p_comp.insert(a.clone(), 1);
                                     },
                                 SubpacketValue::PreferredAEADAlgorithms(a)
                                     =>
-                                    if let Some(count) = p_aead.get_mut(&a) {
+                                    if let Some(count) = p_aead.get_mut(a) {
                                         *count += 1;
                                     } else {
                                         p_aead.insert(a.clone(), 1);
                                     },
                                 SubpacketValue::ExportableCertification(v) =>
-                                    if v {
+                                    if *v {
                                         sigs_subpacket_exportable_true += 1;
                                     } else {
                                         sigs_subpacket_exportable_false += 1;
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index 4bd659bc..e5c5e21f 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -526,7 +526,7 @@ impl Signature4 {
             // Second, re-add the EmbeddedSignature, if present.
             if let Some(embedded_sig) =
                 self.unhashed_area().iter().find_map(|(_, _, v)| {
-                    if v.tag == SubpacketTag::EmbeddedSignature {
+                    if v.tag() == SubpacketTag::EmbeddedSignature {
                         Some(v)
                     } else {
                         None
@@ -1427,7 +1427,7 @@ mod test {
         assert_eq!(sig.unhashed_area().iter().nth(0).unwrap().2,
                    Subpacket::new(SubpacketValue::Issuer(keyid.clone()),
                                   false).unwrap());
-        assert_eq!(sig.unhashed_area().iter().nth(1).unwrap().2.tag,
+        assert_eq!(sig.unhashed_area().iter().nth(1).unwrap().2.tag(),
                    SubpacketTag::EmbeddedSignature);
 
         // Now, make sure that an Issuer subpacket is synthesized from
diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs
index 9d0d0c92..c6f27e5c 100644
--- a/openpgp/src/packet/signature/subpacket.rs
+++ b/openpgp/src/packet/signature/subpacket.rs
@@ -871,11 +871,11 @@ impl<'a> SubpacketValue<'a> {
 #[derive(PartialEq, Clone)]
 pub struct Subpacket<'a> {
     /// Critical flag.
-    pub critical: bool,
+    critical: bool,
     /// Packet type.
-    pub tag: SubpacketTag,
+    tag: SubpacketTag,
     /// Packet value, must match packet type.
-    pub value: SubpacketValue<'a>,
+    value: SubpacketValue<'a>,
 }
 
 impl<'a> fmt::Debug for Subpacket<'a> {
@@ -900,6 +900,21 @@ impl<'a> Subpacket<'a> {
         })
     }
 
+    /// Returns whether this subpacket is critical.
+    pub fn critical(&self) -> bool {
+        self.critical
+    }
+
+    /// Returns the subpacket tag.
+    pub fn tag(&self) -> SubpacketTag {
+        self.tag
+    }
+
+    /// Returns the subpackets value.
+    pub fn value(&self) -> &SubpacketValue<'a> {
+        &self.value
+    }
+
     /// Returns the length of the serialized subpacket.
     pub fn len(&self) -> usize {
         let value_len = self.value.len();
diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs
index 609a3b28..9a810c42 100644
--- a/openpgp/src/serialize/mod.rs
+++ b/openpgp/src/serialize/mod.rs
@@ -953,19 +953,19 @@ impl SerializeInto for Unknown {
 
 impl<'a> Serialize for Subpacket<'a> {
     fn serialize(&self, o: &mut dyn std::io::Write) -> Result<()> {
-        let tag = u8::from(self.tag)
-            | if self.critical { 1 << 7 } else { 0 };
-        let len = 1 + self.value.len();
+        let tag = u8::from(self.tag())
+            | if self.critical() { 1 << 7 } else { 0 };
+        let len = 1 + self.value().len();
 
         len.serialize(o)?;
         o.write_all(&[tag])?;
-        self.value.serialize(o)
+        self.value().serialize(o)
     }
 }
 
 impl<'a> SerializeInto for Subpacket<'a> {
     fn serialized_len(&self) -> usize {
-        (1 + self.value.len()).len() + 1 + self.value.serialized_len()
+        (1 + self.value().len()).len() + 1 + self.value().serialized_len()
     }
 
     fn serialize_into(&self, buf: &mut [u8]) -> Result<usize> {
diff --git a/openpgp/src/tpk/builder.rs b/openpgp/src/tpk/builder.rs
index 281819bd..12109d6c 100644
--- a/openpgp/src/tpk/builder.rs
+++ b/openpgp/src/tpk/builder.rs
@@ -408,7 +408,7 @@ impl TPKBuilder {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::packet::signature::subpacket::{SubpacketTag, Subpacket, SubpacketValue};
+    use crate::packet::signature::subpacket::{SubpacketTag, SubpacketValue};
     use crate::constants::PublicKeyAlgorithm;
 
     #[test]
@@ -526,11 +526,9 @@ mod tests {
             .generate().unwrap();
         let sig_pkts = &tpk1.primary_key_signature(None).unwrap().hashed_area();
 
-        match sig_pkts.lookup(SubpacketTag::KeyFlags) {
-            Some(Subpacket{ value: SubpacketValue::KeyFlags(ref ks),.. }) => {
-                assert!(ks.can_certify());
-            }
-            _ => {}
+        match sig_pkts.lookup(SubpacketTag::KeyFlags).unwrap().value() {
+            SubpacketValue::KeyFlags(ref ks) => assert!(ks.can_certify()),
+            v => panic!("Unexpected subpacket: {:?}", v),
         }
 
         assert_eq!(tpk1.subkeys().count(), 1);
@@ -545,11 +543,9 @@ mod tests {
             .generate().unwrap();
         let sig_pkts = tpk1.subkeys().next().unwrap().self_signatures[0].hashed_area();
 
-        match sig_pkts.lookup(SubpacketTag::KeyFlags) {
-            Some(Subpacket{ value: SubpacketValue::KeyFlags(ref ks),.. }) => {
-                assert!(ks.can_certify());
-            }
-            _ => {}
+        match sig_pkts.lookup(SubpacketTag::KeyFlags).unwrap().value() {
+            SubpacketValue::KeyFlags(ref ks) => assert!(ks.can_certify()),
+            v => panic!("Unexpected subpacket: {:?}", v),
         }
 
         assert_eq!(tpk1.subkeys().count(), 1);