openpgp: Specialize key iterator to return Key<SecretParts, _>.

  - Once KeyIter::secret or KeyIter::unencrypted_secret is called,
    change the iterator type to iterate over &Key<SecretParts, _>.

  - Fixes #384.

7 files changed, 95 insertions, 48 deletions

diff --git a/openpgp-ffi/include/sequoia/openpgp.h b/openpgp-ffi/include/sequoia/openpgp.h
index fccc79d4..fe003422 100644
--- a/openpgp-ffi/include/sequoia/openpgp.h
+++ b/openpgp-ffi/include/sequoia/openpgp.h
@@ -699,21 +699,19 @@ void pgp_tpk_key_iter_alive_at (pgp_tpk_key_iter_t iter, time_t when);
 void pgp_tpk_key_iter_revoked (pgp_tpk_key_iter_t iter, bool revoked);
 
 /*/
-/// Changes the iterator to only return keys that have secret keys (or
-/// not).
+/// Changes the iterator to only return keys that have secret keys.
 ///
 /// Note: you may not call this function after starting to iterate.
 /*/
-void pgp_tpk_key_iter_secret (pgp_tpk_key_iter_t iter, bool secret);
+void pgp_tpk_key_iter_secret (pgp_tpk_key_iter_t iter);
 
 /*/
 /// Changes the iterator to only return keys that have unencrypted
-/// secret keys (or not).
+/// secret keys.
 ///
 /// Note: you may not call this function after starting to iterate.
 /*/
-void pgp_tpk_key_iter_unencrypted_secret (pgp_tpk_key_iter_t iter,
-                                          bool unencrypted_secret);
+void pgp_tpk_key_iter_unencrypted_secret (pgp_tpk_key_iter_t iter);
 
 /*/
 /// Returns a reference to the next key.  Returns NULL if there are no
diff --git a/openpgp-ffi/src/tpk.rs b/openpgp-ffi/src/tpk.rs
index ed834a90..92464175 100644
--- a/openpgp-ffi/src/tpk.rs
+++ b/openpgp-ffi/src/tpk.rs
@@ -647,14 +647,12 @@ pub extern "C" fn pgp_tpk_key_iter_revoked<'a>(
     iter_wrapper.iter = tmp.revoked(Some(revoked));
 }
 
-/// Changes the iterator to only return keys that have secret keys (or
-/// not).
+/// Changes the iterator to only return keys that have secret keys.
 ///
 /// Note: you may not call this function after starting to iterate.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
 pub extern "C" fn pgp_tpk_key_iter_secret<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>,
-    secret: bool)
+    iter_wrapper: *mut KeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -663,17 +661,16 @@ pub extern "C" fn pgp_tpk_key_iter_secret<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty());
-    iter_wrapper.iter = tmp.secret(Some(secret));
+    iter_wrapper.iter = unsafe { std::mem::transmute(tmp.secret()) };
 }
 
 /// Changes the iterator to only return keys that have unencrypted
-/// secret keys (or not).
+/// secret keys.
 ///
 /// Note: you may not call this function after starting to iterate.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
 pub extern "C" fn pgp_tpk_key_iter_unencrypted_secret<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>,
-    unencrypted_secret: bool)
+    iter_wrapper: *mut KeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -682,7 +679,8 @@ pub extern "C" fn pgp_tpk_key_iter_unencrypted_secret<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty());
-    iter_wrapper.iter = tmp.unencrypted_secret(Some(unencrypted_secret));
+    iter_wrapper.iter =
+        unsafe { std::mem::transmute(tmp.unencrypted_secret()) };
 }
 
 /// Returns the next key.  Returns NULL if there are no more elements.
diff --git a/openpgp/examples/notarize.rs b/openpgp/examples/notarize.rs
index fc161373..93ca53e1 100644
--- a/openpgp/examples/notarize.rs
+++ b/openpgp/examples/notarize.rs
@@ -28,7 +28,7 @@ fn main() {
             .expect("Failed to read key");
         let mut n = 0;
 
-        for (_, _, key) in tsk.keys_valid().signing_capable().secret(true) {
+        for (_, _, key) in tsk.keys_valid().signing_capable().secret() {
             keys.push({
                 let mut key = key.clone();
                 if key.secret().expect("filtered").is_encrypted() {
@@ -41,7 +41,7 @@ fn main() {
                         .expect("decryption failed");
                 }
                 n += 1;
-                key.mark_parts_secret().unwrap().into_keypair().unwrap()
+                key.into_keypair().unwrap()
             });
         }
 
diff --git a/openpgp/examples/sign-detached.rs b/openpgp/examples/sign-detached.rs
index 54c9ceef..8d5f9cfa 100644
--- a/openpgp/examples/sign-detached.rs
+++ b/openpgp/examples/sign-detached.rs
@@ -24,7 +24,7 @@ fn main() {
             .expect("Failed to read key");
         let mut n = 0;
 
-        for (_, _, key) in tsk.keys_valid().signing_capable().secret(true) {
+        for (_, _, key) in tsk.keys_valid().signing_capable().secret() {
             keys.push({
                 let mut key = key.clone();
                 if key.secret().expect("filtered").is_encrypted() {
@@ -37,7 +37,7 @@ fn main() {
                         .expect("decryption failed");
                 }
                 n += 1;
-                key.mark_parts_secret().unwrap().into_keypair().unwrap()
+                key.into_keypair().unwrap()
             });
         }
 
diff --git a/openpgp/examples/sign.rs b/openpgp/examples/sign.rs
index ec80e87f..8ba0b018 100644
--- a/openpgp/examples/sign.rs
+++ b/openpgp/examples/sign.rs
@@ -23,7 +23,7 @@ fn main() {
             .expect("Failed to read key");
         let mut n = 0;
 
-        for (_, _, key) in tsk.keys_valid().signing_capable().secret(true) {
+        for (_, _, key) in tsk.keys_valid().signing_capable().secret() {
             keys.push({
                 let mut key = key.clone();
                 if key.secret().expect("filtered").is_encrypted() {
@@ -36,7 +36,7 @@ fn main() {
                         .expect("decryption failed");
                 }
                 n += 1;
-                key.mark_parts_secret().unwrap().into_keypair().unwrap()
+                key.into_keypair().unwrap()
             });
         }
 
diff --git a/openpgp/src/serialize/tpk.rs b/openpgp/src/serialize/tpk.rs
index bf111561..5a8f5fec 100644
--- a/openpgp/src/serialize/tpk.rs
+++ b/openpgp/src/serialize/tpk.rs
@@ -316,7 +316,7 @@ impl<'a> TSK<'a> {
     /// # f().unwrap();
     /// # fn f() -> Result<()> {
     /// let (tpk, _) = TPKBuilder::new().add_signing_subkey().generate()?;
-    /// assert_eq!(tpk.keys_valid().secret(true).count(), 2);
+    /// assert_eq!(tpk.keys_valid().secret().count(), 2);
     ///
     /// // Only write out the primary key's secret.
     /// let mut buf = Vec::new();
@@ -328,7 +328,7 @@ impl<'a> TSK<'a> {
     ///     .serialize(&mut buf)?;
     ///
     /// let tpk_ = TPK::from_bytes(&buf)?;
-    /// assert_eq!(tpk_.keys_valid().secret(true).count(), 1);
+    /// assert_eq!(tpk_.keys_valid().secret().count(), 1);
     /// assert!(tpk_.primary().secret().is_some());
     /// # Ok(()) }
     pub fn set_filter<P>(mut self, predicate: P) -> Self
diff --git a/openpgp/src/tpk/keyiter.rs b/openpgp/src/tpk/keyiter.rs
index 25e94903..50f59cbe 100644
--- a/openpgp/src/tpk/keyiter.rs
+++ b/openpgp/src/tpk/keyiter.rs
@@ -67,14 +67,49 @@ impl<'a, P: key::KeyParts, R: key::KeyRole> fmt::Debug
     }
 }
 
-impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> Iterator
-    for KeyIter<'a, P, R>
-    where &'a Key<P, R>: From<&'a Key<key::PublicParts,
-                                      key::UnspecifiedRole>>
+// Very carefully implement Iterator for
+// Key<{PublicParts,UnspecifiedParts}, _>.  We cannot just abstract
+// over the parts, because then we cannot specialize the
+// implementation for Key<SecretParts, _> below.
+macro_rules! impl_iterator {
+    ($parts:path) => {
+        impl<'a, R: 'a + key::KeyRole> Iterator for KeyIter<'a, $parts, R>
+            where &'a Key<$parts, R>: From<&'a Key<key::PublicParts,
+                                                   key::UnspecifiedRole>>
+        {
+            type Item = (Option<&'a Signature>, RevocationStatus<'a>,
+                         &'a Key<$parts, R>);
+
+            fn next(&mut self) -> Option<Self::Item> {
+                self.next_common().map(|(s, r, k)| (s, r, k.into()))
+            }
+        }
+    }
+}
+impl_iterator!(key::PublicParts);
+impl_iterator!(key::UnspecifiedParts);
+
+impl<'a, R: 'a + key::KeyRole> Iterator for KeyIter<'a, key::SecretParts, R>
+    where &'a Key<key::SecretParts, R>: From<&'a Key<key::SecretParts,
+                                                     key::UnspecifiedRole>>
 {
-    type Item = (Option<&'a Signature>, RevocationStatus<'a>, &'a Key<P, R>);
+    type Item = (Option<&'a Signature>, RevocationStatus<'a>,
+                 &'a Key<key::SecretParts, R>);
 
     fn next(&mut self) -> Option<Self::Item> {
+        self.next_common()
+            .map(|(s, r, k)|
+                 (s, r,
+                  k.mark_parts_secret_ref().expect("has secret parts").into()))
+    }
+}
+
+impl <'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> {
+    fn next_common(&mut self)
+                   -> Option<(Option<&'a Signature>,
+                              RevocationStatus<'a>,
+                              &'a Key<key::PublicParts, key::UnspecifiedRole>)>
+    {
         tracer!(false, "KeyIter::next", 0);
         t!("KeyIter: {:?}", self);
 
@@ -186,7 +221,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> Iterator
                 }
             }
 
-            return Some((sigo, revoked, key.into()));
+            return Some((sigo, revoked, key));
         }
     }
 }
@@ -333,27 +368,43 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R>
         self
     }
 
-    /// If not None, filters by whether a key has secret key material.
-    ///
-    /// If you call this function multiple times, only the last value
-    /// is used.
-    pub fn secret<T>(mut self, secret: T) -> Self
-        where T: Into<Option<bool>>
-    {
-        self.secret = secret.into();
-        self
+    /// Changes the filter to only return keys with secret key material.
+    pub fn secret(self) -> KeyIter<'a, key::SecretParts, R> {
+        KeyIter {
+            tpk: self.tpk,
+            primary: self.primary,
+            subkey_iter: self.subkey_iter,
+
+            // The filters.
+            flags: self.flags,
+            alive_at: self.alive_at,
+            revoked: self.revoked,
+            secret: Some(true),
+            unencrypted_secret: self.unencrypted_secret,
+
+            _p: std::marker::PhantomData,
+            _r: std::marker::PhantomData,
+        }
     }
 
-    /// If not None, filters by whether a key has an unencrypted
-    /// secret.
-    ///
-    /// If you call this function multiple times, only the last value
-    /// is used.
-    pub fn unencrypted_secret<T>(mut self, unencrypted_secret: T) -> Self
-        where T: Into<Option<bool>>
-    {
-        self.unencrypted_secret = unencrypted_secret.into();
-        self
+    /// Changes the filter to only return keys with unencrypted secret
+    /// key material.
+    pub fn unencrypted_secret(self)  -> KeyIter<'a, key::SecretParts, R> {
+        KeyIter {
+            tpk: self.tpk,
+            primary: self.primary,
+            subkey_iter: self.subkey_iter,
+
+            // The filters.
+            flags: self.flags,
+            alive_at: self.alive_at,
+            revoked: self.revoked,
+            secret: self.secret,
+            unencrypted_secret: Some(true),
+
+            _p: std::marker::PhantomData,
+            _r: std::marker::PhantomData,
+        }
     }
 }