ffi: Use Option<&mut T> for destructors.

  - This reflects the fact that destructors may be called with NULL.

10 files changed, 56 insertions, 45 deletions

diff --git a/ffi/src/core.rs b/ffi/src/core.rs
index bb65d09c..14f11620 100644
--- a/ffi/src/core.rs
+++ b/ffi/src/core.rs
@@ -107,7 +107,7 @@ pub extern "system" fn sq_context_new(domain: *const c_char,
 
 /// Frees a context.
 #[no_mangle]
-pub extern "system" fn sq_context_free(context: *mut Context) {
+pub extern "system" fn sq_context_free(context: Option<&mut Context>) {
     ffi_free!(context)
 }
 
@@ -290,7 +290,7 @@ pub extern "system" fn sq_reader_from_bytes(buf: *const uint8_t,
 
 /// Frees a reader.
 #[no_mangle]
-pub extern "system" fn sq_reader_free(reader: *mut Box<Read>) {
+pub extern "system" fn sq_reader_free(reader: Option<&mut Box<Read>>) {
     ffi_free!(reader)
 }
 
@@ -404,7 +404,7 @@ impl Write for WriterAlloc {
 
 /// Frees a writer.
 #[no_mangle]
-pub extern "system" fn sq_writer_free(writer: *mut Box<Write>) {
+pub extern "system" fn sq_writer_free(writer: Option<&mut Box<Write>>) {
     ffi_free!(writer)
 }
 
diff --git a/ffi/src/error.rs b/ffi/src/error.rs
index 25640556..56a6d27d 100644
--- a/ffi/src/error.rs
+++ b/ffi/src/error.rs
@@ -10,7 +10,7 @@ use sequoia_core as core;
 
 /// Frees an error.
 #[no_mangle]
-pub extern "system" fn sq_error_free(error: *mut failure::Error) {
+pub extern "system" fn sq_error_free(error: Option<&mut failure::Error>) {
     ffi_free!(error)
 }
 
diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs
index 8c73e1b0..a7b95871 100644
--- a/ffi/src/lib.rs
+++ b/ffi/src/lib.rs
@@ -18,8 +18,10 @@
 //! Sequoia objects are opaque objects.  They are created in
 //! constructors, and must be freed when no longer needed.
 //!
-//! Pointers handed to Sequoia must not be `NULL`, destructors are
-//! exempt from this rule.  Freeing `NULL` is a nop.
+//! Pointers handed to Sequoia must not be `NULL`, unless explicitly
+//! stated.  See [references].
+//!
+//! [references]: #references
 //!
 //! Enumeration-like values must be in the valid range.
 //!
@@ -63,10 +65,9 @@
 //!  - All references are valid.
 //!
 //! In this crate we enforce the second rule by asserting that all
-//! pointers handed in are non-`NULL`.  There are two exceptions:
-//!
-//!   - It is explicitly stated by using `Option<&T>` or `Option<&mut T>`.
-//!   - Destructors (`sq_*_free`) may be called with `NULL`.
+//! pointers handed in are non-`NULL`.  If a parameter of an FFI
+//! function uses `Option<&T>` or `Option<&mut T>`, it may be called
+//! with `NULL`.  A notable example are the destructors (`sq_*_free`).
 //!
 //! # Lifetimes
 //!
@@ -134,9 +135,9 @@ use std::hash::BuildHasher;
 /// NOP if called with NULL.
 macro_rules! ffi_free {
     ($name:ident) => {{
-        if ! $name.is_null() {
+        if let Some(ptr) = $name {
             unsafe {
-                drop(Box::from_raw($name))
+                drop(Box::from_raw(ptr))
             }
         }
     }};
diff --git a/ffi/src/net.rs b/ffi/src/net.rs
index 177badea..66f951ce 100644
--- a/ffi/src/net.rs
+++ b/ffi/src/net.rs
@@ -107,7 +107,7 @@ pub extern "system" fn sq_keyserver_sks_pool(ctx: *mut Context)
 
 /// Frees a keyserver object.
 #[no_mangle]
-pub extern "system" fn sq_keyserver_free(ks: *mut KeyServer) {
+pub extern "system" fn sq_keyserver_free(ks: Option<&mut KeyServer>) {
     ffi_free!(ks)
 }
 
diff --git a/ffi/src/openpgp/fingerprint.rs b/ffi/src/openpgp/fingerprint.rs
index 92696601..b2937f43 100644
--- a/ffi/src/openpgp/fingerprint.rs
+++ b/ffi/src/openpgp/fingerprint.rs
@@ -40,7 +40,7 @@ pub extern "system" fn sq_fingerprint_from_hex(hex: *const c_char)
 
 /// Frees a sq_fingerprint_t.
 #[no_mangle]
-pub extern "system" fn sq_fingerprint_free(fp: *mut Fingerprint) {
+pub extern "system" fn sq_fingerprint_free(fp: Option<&mut Fingerprint>) {
     ffi_free!(fp)
 }
 
diff --git a/ffi/src/openpgp/keyid.rs b/ffi/src/openpgp/keyid.rs
index e4e716bb..6df8ca81 100644
--- a/ffi/src/openpgp/keyid.rs
+++ b/ffi/src/openpgp/keyid.rs
@@ -51,7 +51,7 @@ pub extern "system" fn sq_keyid_from_hex(id: *const c_char) -> *mut KeyID {
 
 /// Frees an `KeyID` object.
 #[no_mangle]
-pub extern "system" fn sq_keyid_free(keyid: *mut KeyID) {
+pub extern "system" fn sq_keyid_free(keyid: Option<&mut KeyID>) {
     ffi_free!(keyid)
 }
 
diff --git a/ffi/src/openpgp/mod.rs b/ffi/src/openpgp/mod.rs
index 859d8096..2a663763 100644
--- a/ffi/src/openpgp/mod.rs
+++ b/ffi/src/openpgp/mod.rs
@@ -117,7 +117,7 @@ pub extern "system" fn sq_revocation_status_variant(
 /// Frees a sq_revocation_status_t.
 #[no_mangle]
 pub extern "system" fn sq_revocation_status_free(
-    rs: *mut RevocationStatus)
+    rs: Option<&mut RevocationStatus>)
 {
     ffi_free!(rs)
 }
@@ -151,7 +151,7 @@ pub extern "system" fn sq_tsk_new(ctx: *mut Context,
 
 /// Frees the TSK.
 #[no_mangle]
-pub extern "system" fn sq_tsk_free(tsk: *mut TSK) {
+pub extern "system" fn sq_tsk_free(tsk: Option<&mut TSK>) {
     ffi_free!(tsk)
 }
 
@@ -188,7 +188,7 @@ pub extern "system" fn sq_tsk_serialize(ctx: *mut Context,
 
 /// Frees the Packet.
 #[no_mangle]
-pub extern "system" fn sq_packet_free(p: *mut Packet) {
+pub extern "system" fn sq_packet_free(p: Option<&mut Packet>) {
     ffi_free!(p)
 }
 
@@ -225,7 +225,7 @@ pub extern "system" fn sq_packet_kind(p: *const Packet)
 
 /// Frees the Signature.
 #[no_mangle]
-pub extern "system" fn sq_signature_free(s: *mut Signature) {
+pub extern "system" fn sq_signature_free(s: Option<&mut Signature>) {
     ffi_free!(s)
 }
 
@@ -698,14 +698,14 @@ pub extern "system" fn sq_packet_parser_from_bytes
 /// Frees the packet parser result
 #[no_mangle]
 pub extern "system" fn sq_packet_parser_result_free(
-    ppr: *mut PacketParserResult)
+    ppr: Option<&mut PacketParserResult>)
 {
     ffi_free!(ppr)
 }
 
 /// Frees the packet parser.
 #[no_mangle]
-pub extern "system" fn sq_packet_parser_free(pp: *mut PacketParser) {
+pub extern "system" fn sq_packet_parser_free(pp: Option<&mut PacketParser>) {
     ffi_free!(pp)
 }
 
@@ -721,7 +721,9 @@ pub extern "system" fn sq_packet_parser_eof_is_message(
 
 /// Frees the packet parser EOF object.
 #[no_mangle]
-pub extern "system" fn sq_packet_parser_eof_free(eof: *mut PacketParserEOF) {
+pub extern "system" fn sq_packet_parser_eof_free
+    (eof: Option<&mut PacketParserEOF>)
+{
     ffi_free!(eof)
 }
 
diff --git a/ffi/src/openpgp/packet_pile.rs b/ffi/src/openpgp/packet_pile.rs
index 33a3735f..5f624c36 100644
--- a/ffi/src/openpgp/packet_pile.rs
+++ b/ffi/src/openpgp/packet_pile.rs
@@ -73,7 +73,8 @@ pub extern "system" fn sq_packet_pile_from_bytes(ctx: *mut Context,
 
 /// Frees the packet_pile.
 #[no_mangle]
-pub extern "system" fn sq_packet_pile_free(packet_pile: *mut PacketPile) {
+pub extern "system" fn sq_packet_pile_free(packet_pile: Option<&mut PacketPile>)
+{
     ffi_free!(packet_pile)
 }
 
diff --git a/ffi/src/openpgp/tpk.rs b/ffi/src/openpgp/tpk.rs
index 471f4b93..47172013 100644
--- a/ffi/src/openpgp/tpk.rs
+++ b/ffi/src/openpgp/tpk.rs
@@ -104,7 +104,7 @@ pub extern "system" fn sq_tpk_from_packet_parser(ctx: *mut Context,
 
 /// Frees the TPK.
 #[no_mangle]
-pub extern "system" fn sq_tpk_free(tpk: *mut TPK) {
+pub extern "system" fn sq_tpk_free(tpk: Option<&mut TPK>) {
     ffi_free!(tpk)
 }
 
@@ -478,7 +478,7 @@ pub extern "system" fn sq_tpk_user_id_binding_iter(tpk: *const TPK)
 /// Frees a sq_user_id_binding_iter_t.
 #[no_mangle]
 pub extern "system" fn sq_user_id_binding_iter_free(
-    iter: *mut UserIDBindingIter)
+    iter: Option<&mut UserIDBindingIter>)
 {
     ffi_free!(iter)
 }
@@ -518,7 +518,7 @@ pub extern "system" fn sq_tpk_key_iter(tpk: *const TPK)
 /// Frees a sq_tpk_key_iter_t.
 #[no_mangle]
 pub extern "system" fn sq_tpk_key_iter_free(
-    iter: *mut KeyIterWrapper)
+    iter: Option<&mut KeyIterWrapper>)
 {
     ffi_free!(iter)
 }
@@ -599,7 +599,7 @@ pub extern "system" fn sq_tpk_builder_autocrypt() -> *mut TPKBuilder {
 
 /// Frees an `sq_tpk_builder_t`.
 #[no_mangle]
-pub extern "system" fn sq_tpk_builder_free(tpkb: *mut TPKBuilder)
+pub extern "system" fn sq_tpk_builder_free(tpkb: Option<&mut TPKBuilder>)
 {
     ffi_free!(tpkb)
 }
diff --git a/ffi/src/store.rs b/ffi/src/store.rs
index 593a864d..6ba7e387 100644
--- a/ffi/src/store.rs
+++ b/ffi/src/store.rs
@@ -96,7 +96,7 @@ pub extern "system" fn sq_store_iter_next(iter: *mut StoreIter,
 
 /// Frees a sq_store_iter_t.
 #[no_mangle]
-pub extern "system" fn sq_store_iter_free(iter: *mut StoreIter) {
+pub extern "system" fn sq_store_iter_free(iter: Option<&mut StoreIter>) {
     ffi_free!(iter)
 }
 
@@ -141,7 +141,7 @@ pub extern "system" fn sq_key_iter_next(iter: *mut KeyIter,
 
 /// Frees a sq_key_iter_t.
 #[no_mangle]
-pub extern "system" fn sq_key_iter_free(iter: *mut KeyIter) {
+pub extern "system" fn sq_key_iter_free(iter: Option<&mut KeyIter>) {
     ffi_free!(iter)
 }
 
@@ -183,7 +183,7 @@ pub extern "system" fn sq_log_iter_next(iter: *mut LogIter)
 
 /// Frees a sq_log_iter_t.
 #[no_mangle]
-pub extern "system" fn sq_log_iter_free(iter: *mut LogIter) {
+pub extern "system" fn sq_log_iter_free(iter: Option<&mut LogIter>) {
     ffi_free!(iter)
 }
 
@@ -214,7 +214,7 @@ pub extern "system" fn sq_store_open(ctx: *mut Context,
 
 /// Frees a sq_store_t.
 #[no_mangle]
-pub extern "system" fn sq_store_free(store: *mut Store) {
+pub extern "system" fn sq_store_free(store: Option<&mut Store>) {
     ffi_free!(store)
 }
 
@@ -349,7 +349,7 @@ pub extern "system" fn sq_binding_iter_next(iter: *mut BindingIter,
 
 /// Frees a sq_binding_iter_t.
 #[no_mangle]
-pub extern "system" fn sq_binding_iter_free(iter: *mut BindingIter) {
+pub extern "system" fn sq_binding_iter_free(iter: Option<&mut BindingIter>) {
     ffi_free!(iter)
 }
 
@@ -366,28 +366,35 @@ pub extern "system" fn sq_store_log(ctx: *mut Context,
 
 /// Frees a sq_binding_t.
 #[no_mangle]
-pub extern "system" fn sq_binding_free(binding: *mut Binding) {
+pub extern "system" fn sq_binding_free(binding: Option<&mut Binding>) {
     ffi_free!(binding)
 }
 
 /// Frees a sq_key_t.
 #[no_mangle]
-pub extern "system" fn sq_key_free(key: *mut Key) {
+pub extern "system" fn sq_key_free(key: Option<&mut Key>) {
     ffi_free!(key)
 }
 
 /// Frees a sq_log_t.
 #[no_mangle]
-pub extern "system" fn sq_log_free(log: *mut Log) {
-    if log.is_null() { return };
-    let log = ffi_param_move!(log);
-    sq_store_free(log.store);
-    sq_binding_free(log.binding);
-    sq_key_free(log.key);
-    sq_string_free(log.slug);
-    sq_string_free(log.status);
-    sq_string_free(log.error);
-    drop(log)
+pub extern "system" fn sq_log_free(log: Option<&mut Log>) {
+    if let Some(log) = log {
+        let log = unsafe { Box::from_raw(log) };
+        if ! log.store.is_null() {
+            ffi_param_move!(log.store);
+        }
+        if ! log.binding.is_null() {
+            ffi_param_move!(log.binding);
+        }
+        if ! log.key.is_null() {
+            ffi_param_move!(log.key);
+        }
+        sq_string_free(log.slug);
+        sq_string_free(log.status);
+        sq_string_free(log.error);
+        drop(log)
+    }
 }
 
 /// Returns the `sq_stats_t` of this binding.
@@ -557,7 +564,7 @@ pub extern "system" fn sq_key_log(ctx: *mut Context,
 
 /// Frees a sq_stats_t.
 #[no_mangle]
-pub extern "system" fn sq_stats_free(stats: *mut Stats) {
+pub extern "system" fn sq_stats_free(stats: Option<&mut Stats>) {
     ffi_free!(stats)
 }