Merge pull request #23 from josephlr/format

mathstuf · web-flow · commit e73f49060040 · 2019-06-26T19:27:38.000-04:00
Cleanup payload formatting to avoid allocation
diff --git a/Cargo.toml b/Cargo.toml
@@ -16,7 +16,6 @@ members = ["libkeyutils-sys"]
 [dependencies]
 bitflags = "1.0.4"
 errno = "0.2"
-itertools = "0.8"
 libkeyutils-sys = { path = "libkeyutils-sys" }
 log = "0.4.4"
 
diff --git a/src/keytypes/blacklist.rs b/src/keytypes/blacklist.rs
@@ -28,7 +28,7 @@
 
 use std::borrow::Cow;
 
-use super::AsciiHex;
+use super::ByteBuf;
 use crate::keytype::*;
 
 /// Blacklist hashes.
@@ -81,10 +81,6 @@ pub struct Description {
 
 impl KeyDescription for Description {
     fn description(&self) -> Cow<str> {
-        Cow::Owned(format!(
-            "{}:{}",
-            self.hash_type.name(),
-            AsciiHex::convert(&self.hash),
-        ))
+        format!("{}:{:x}", self.hash_type.name(), ByteBuf(&self.hash)).into()
     }
 }
diff --git a/src/keytypes/dns_resolver.rs b/src/keytypes/dns_resolver.rs
@@ -59,11 +59,11 @@ pub enum QueryType {
 impl QueryType {
     /// The name of the DNS record.
     fn name(&self) -> &str {
-        match *self {
+        match self {
             QueryType::A => "a",
             QueryType::AAAA => "aaaa",
             QueryType::AFSDB => "afsdb",
-            QueryType::Other(ref s) => s,
+            QueryType::Other(s) => s,
         }
     }
 }
@@ -87,10 +87,9 @@ pub struct Description {
 
 impl KeyDescription for Description {
     fn description(&self) -> Cow<str> {
-        if let Some(ref query_type) = self.query_type {
-            Cow::Owned(format!("{}:{}", query_type.name(), self.name))
-        } else {
-            self.name.clone()
+        match &self.query_type {
+            Some(ref query_type) => format!("{}:{}", query_type.name(), self.name).into(),
+            _ => self.name.clone(),
         }
     }
 }
diff --git a/src/keytypes/encrypted.rs b/src/keytypes/encrypted.rs
@@ -28,7 +28,7 @@
 
 use std::borrow::Cow;
 
-use super::AsciiHex;
+use super::ByteBuf;
 use crate::keytype::*;
 
 /// Encrypted keys.
@@ -96,7 +96,7 @@ pub enum MasterKeyType {
 impl MasterKeyType {
     /// The name of the master key type.
     fn name(&self) -> &str {
-        match *self {
+        match self {
             MasterKeyType::Trusted => "trusted",
             MasterKeyType::User => "user",
         }
@@ -142,12 +142,12 @@ pub enum Payload {
 
 impl KeyPayload for Payload {
     fn payload(&self) -> Cow<[u8]> {
-        let payload = match *self {
+        match self {
             Payload::New {
-                ref format,
-                ref keytype,
-                ref description,
-                ref keylen,
+                format,
+                keytype,
+                description,
+                keylen,
             } => {
                 format!(
                     "new {} {}:{} {}",
@@ -158,14 +158,14 @@ impl KeyPayload for Payload {
                 )
             },
             Payload::Load {
-                ref blob,
-            } => format!("load {}", AsciiHex::convert(&blob)),
+                blob,
+            } => format!("load {:x}", ByteBuf(&blob)),
             Payload::Update {
-                ref keytype,
-                ref description,
+                keytype,
+                description,
             } => format!("update {}:{}", keytype.name(), description),
-        };
-
-        payload.bytes().collect()
+        }
+        .into_bytes()
+        .into()
     }
 }
diff --git a/src/keytypes/mod.rs b/src/keytypes/mod.rs
@@ -29,7 +29,7 @@
 //! The Linux kernel supports many types of keys. They may be compiled out or available as
 //! modules. The types provided here try to make it easier to use these keys.
 
-use std::char;
+use std::fmt;
 
 pub mod asymmetric;
 pub use self::asymmetric::Asymmetric;
@@ -64,36 +64,24 @@ pub use self::trusted::Trusted;
 pub mod user;
 pub use self::user::User;
 
-/// A structure for assisting in coverting binary data into hexadecimal.
-///
-/// Many key types take in ASCII hexadecimal input instead of raw binary.
-pub struct AsciiHex;
+/// A structure for assisting in display binary data.
+struct ByteBuf<'a>(&'a [u8]);
 
-/// The mask for a nibble.
-const NIBBLE_MASK: u8 = 0x0f;
-
-impl AsciiHex {
-    /// Convert binary data into an ASCII hexadecimal string.
-    pub fn convert(data: &[u8]) -> String {
-        data.iter()
-            .fold(String::with_capacity(data.len() * 2), |mut string, byte| {
-                let hi = (byte >> 4) & NIBBLE_MASK;
-                let lo = byte & NIBBLE_MASK;
-
-                string.push(char::from_digit(u32::from(hi), 16).unwrap());
-                string.push(char::from_digit(u32::from(lo), 16).unwrap());
-
-                string
-            })
+impl fmt::LowerHex for ByteBuf<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        for byte in self.0 {
+            write!(f, "{:02x}", byte)?;
+        }
+        Ok(())
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::*;
+    use super::ByteBuf;
 
     fn check(input: &[u8], expected: &str) {
-        assert_eq!(AsciiHex::convert(input), expected);
+        assert_eq!(format!("{:x}", ByteBuf(input)), expected);
     }
 
     #[test]
diff --git a/src/keytypes/rxrpc.rs b/src/keytypes/rxrpc.rs
@@ -75,6 +75,6 @@ impl KeyPayload for Payload {
         payload.extend(self.session_key.iter());
         payload.extend(self.ticket.iter());
 
-        Cow::Owned(payload)
+        payload.into()
     }
 }
diff --git a/src/keytypes/rxrpc_s.rs b/src/keytypes/rxrpc_s.rs
@@ -54,7 +54,7 @@ pub struct Description {
 
 impl KeyDescription for Description {
     fn description(&self) -> Cow<str> {
-        Cow::Owned(format!("{}:{}", self.service_id, self.security_index))
+        format!("{}:{}", self.service_id, self.security_index).into()
     }
 }
 
diff --git a/src/keytypes/trusted.rs b/src/keytypes/trusted.rs
@@ -27,10 +27,9 @@
 //! Trusted keys
 
 use std::borrow::Cow;
+use std::fmt;
 
-use itertools::Itertools;
-
-use super::AsciiHex;
+use super::ByteBuf;
 use crate::keytype::*;
 
 /// Trusted keys are rooted in the TPM.
@@ -111,71 +110,55 @@ pub struct TrustedOptions {
     pub policyhandle: Option<u32>,
 }
 
-impl TrustedOptions {
-    fn payload_string(&self) -> String {
-        let parts = [
+impl fmt::Display for TrustedOptions {
+    /// Formats the options that are present. Starts with a leading space.
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if let Some(keyhandle) = self.keyhandle {
             // keyhandle=    ascii hex value of sealing key; default 40000000 (SRK)
-            (
-                "keyhandle",
-                self.keyhandle
-                    .as_ref()
-                    .map(|v| AsciiHex::convert(&v.to_be().to_be_bytes())),
-            ),
+            write!(f, " keyhandle={:x}", keyhandle)?;
+        }
+        if let Some(keyauth) = self.keyauth.as_ref() {
             // keyauth=      ascii hex auth for sealing key; default 00...
             //               (40 ascii zeros)
-            (
-                "keyauth",
-                self.keyauth.as_ref().map(|v| AsciiHex::convert(v)),
-            ),
+            write!(f, " keyauth={:x}", ByteBuf(keyauth))?;
+        }
+        if let Some(blobauth) = self.blobauth.as_ref() {
             // blobauth=     ascii hex auth for sealed data; default 00...
             //               (40 ascii zeros)
-            (
-                "blobauth",
-                self.blobauth.as_ref().map(|v| AsciiHex::convert(v)),
-            ),
+            write!(f, " blobauth={:x}", ByteBuf(blobauth))?;
+        }
+        if let Some(pcrinfo) = self.pcrinfo.as_ref() {
             // pcrinfo=      ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
-            (
-                "pcrinfo",
-                self.pcrinfo.as_ref().map(|v| AsciiHex::convert(&v)),
-            ),
+            write!(f, " pcrinfo={:x}", ByteBuf(pcrinfo))?;
+        }
+        if let Some(pcrlock) = self.pcrlock {
             // pcrlock=      pcr number to be extended to "lock" blob
-            ("pcrlock", self.pcrlock.as_ref().map(|v| format!("{}", v))),
+            write!(f, " pcrlock={}", pcrlock)?;
+        }
+        if let Some(migratable) = self.migratable {
             // migratable=   0|1 indicating permission to reseal to new PCR values,
             //               default 1 (resealing allowed)
-            (
-                "migratable",
-                self.migratable
-                    .as_ref()
-                    .map(|&v| if v { "1" } else { "0" }.to_string()),
-            ),
+            write!(f, " migratable={}", migratable as u8)?;
+        }
+        if let Some(hash) = self.hash {
             // hash=         hash algorithm name as a string. For TPM 1.x the only
             //               allowed value is sha1. For TPM 2.x the allowed values
             //               are sha1, sha256, sha384, sha512 and sm3-256.
-            ("hash", self.hash.as_ref().map(|v| v.name().to_string())),
+            write!(f, " hash={}", hash.name())?;
+        }
+        if let Some(policydigest) = self.policydigest.as_ref() {
             // policydigest= digest for the authorization policy. must be calculated
             //               with the same hash algorithm as specified by the 'hash='
             //               option.
-            (
-                "policydigest",
-                self.policydigest.as_ref().map(|v| AsciiHex::convert(&v)),
-            ),
+            write!(f, " policydigest={:x}", ByteBuf(policydigest))?;
+        }
+        if let Some(policyhandle) = self.policyhandle {
             // policyhandle= handle to an authorization policy session that defines the
             //               same policy and with the same hash algorithm as was used to
             //               seal the key.
-            (
-                "policyhandle",
-                self.policyhandle
-                    .as_ref()
-                    .map(|v| AsciiHex::convert(&v.to_be().to_be_bytes())),
-            ),
-        ];
-
-        let options = parts
-            .iter()
-            .filter_map(|(key, value)| value.as_ref().map(|value| format!("{}={}", key, value)))
-            .format(" ");
-
-        format!("{}", options)
+            write!(f, " policyhandle={:x}", policyhandle)?;
+        }
+        Ok(())
     }
 }
 
@@ -211,22 +194,20 @@ pub enum Payload {
 
 impl KeyPayload for Payload {
     fn payload(&self) -> Cow<[u8]> {
-        let (command, blob, options) = match *self {
+        match self {
             Payload::New {
-                ref keylen,
-                ref options,
-            } => ("new", format!("{}", keylen), options),
+                keylen,
+                options,
+            } => format!("new {}{}", keylen, options),
             Payload::Load {
-                ref blob,
-                ref options,
-            } => ("load", AsciiHex::convert(&blob), options),
+                blob,
+                options,
+            } => format!("load {:x}{}", ByteBuf(blob), options),
             Payload::Update {
-                ref options,
-            } => ("update", String::new(), options),
-        };
-
-        format!("{} {} {}", command, blob, options.payload_string())
-            .bytes()
-            .collect()
+                options,
+            } => format!("update{}", options),
+        }
+        .into_bytes()
+        .into()
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -59,11 +59,11 @@ pub enum QueryType {`
`59`	`59`	`impl QueryType {`
`60`	`60`	`/// The name of the DNS record.`
`61`	`61`	`fn name(&self) -> &str {`
`62`		`- match *self {`
	`62`	`+ match self {`
`63`	`63`	`QueryType::A => "a",`
`64`	`64`	`QueryType::AAAA => "aaaa",`
`65`	`65`	`QueryType::AFSDB => "afsdb",`
`66`		`- QueryType::Other(ref s) => s,`
	`66`	`+ QueryType::Other(s) => s,`
`67`	`67`	`}`
`68`	`68`	`}`
`69`	`69`	`}`
`@@ -87,10 +87,9 @@ pub struct Description {`
`87`	`87`
`88`	`88`	`impl KeyDescription for Description {`
`89`	`89`	`fn description(&self) -> Cow<str> {`
`90`		`- if let Some(ref query_type) = self.query_type {`
`91`		`- Cow::Owned(format!("{}:{}", query_type.name(), self.name))`
`92`		`- } else {`
`93`		`- self.name.clone()`
	`90`	`+ match &self.query_type {`
	`91`	`+ Some(ref query_type) => format!("{}:{}", query_type.name(), self.name).into(),`
	`92`	`+ _ => self.name.clone(),`
`94`	`93`	`}`
`95`	`94`	`}`
`96`	`95`	`}`
Original file line number	Diff line number	Diff line change
`@@ -75,6 +75,6 @@ impl KeyPayload for Payload {`
`75`	`75`	`payload.extend(self.session_key.iter());`
`76`	`76`	`payload.extend(self.ticket.iter());`
`77`	`77`
`78`		`- Cow::Owned(payload)`
	`78`	`+ payload.into()`
`79`	`79`	`}`
`80`	`80`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ pub struct Description {`
`54`	`54`
`55`	`55`	`impl KeyDescription for Description {`
`56`	`56`	`fn description(&self) -> Cow<str> {`
`57`		`- Cow::Owned(format!("{}:{}", self.service_id, self.security_index))`
	`57`	`+ format!("{}:{}", self.service_id, self.security_index).into()`
`58`	`58`	`}`
`59`	`59`	`}`
`60`	`60`