ffi: Allocate returned strings using libc.

  - Allocate all returned strings using libc's allocator.  This has
    the advantage that the user can easily use strings and free them
    using free(3).

  - Fixes #157.

12 files changed, 76 insertions, 71 deletions

diff --git a/ffi/examples/configure.c b/ffi/examples/configure.c
index 45c56ff1..f0192f87 100644
--- a/ffi/examples/configure.c
+++ b/ffi/examples/configure.c
@@ -3,6 +3,7 @@
 #include <error.h>
 #include <errno.h>
 #include <stdio.h>
+#include <stdlib.h>
 
 #include <sequoia.h>
 
@@ -29,7 +30,7 @@ main (int argc, char **argv)
       char *msg = sq_error_string (err);
       error (0, 0, "Initializing KeyServer failed as expected: %s",
              msg);
-      sq_string_free (msg);
+      free (msg);
       sq_error_free (err);
     }
   else
diff --git a/ffi/include/sequoia/core.h b/ffi/include/sequoia/core.h
index 0e5dd6aa..829d7104 100644
--- a/ffi/include/sequoia/core.h
+++ b/ffi/include/sequoia/core.h
@@ -28,11 +28,6 @@ typedef struct sq_context *sq_context_t;
 sq_error_t sq_context_last_error (sq_context_t ctx);
 
 /*/
-/// Frees a string returned from Sequoia.
-/*/
-void sq_string_free (char *s);
-
-/*/
 /// Represents a `Context` configuration.
 /*/
 typedef struct sq_config *sq_config_t;
diff --git a/ffi/include/sequoia/error.h b/ffi/include/sequoia/error.h
index 647fd674..dd42753c 100644
--- a/ffi/include/sequoia/error.h
+++ b/ffi/include/sequoia/error.h
@@ -135,7 +135,7 @@ void sq_error_free (sq_error_t error);
 /*/
 /// Returns the error message.
 ///
-/// The returned value must be freed with `sq_string_free`.
+/// The returned value must be freed with `free(3)`.
 /*/
 char *sq_error_string (const sq_error_t err);
 
diff --git a/ffi/lang/python/sequoia/glue.py b/ffi/lang/python/sequoia/glue.py
index 4b74d609..799c550c 100644
--- a/ffi/lang/python/sequoia/glue.py
+++ b/ffi/lang/python/sequoia/glue.py
@@ -84,7 +84,7 @@ class SQObject(object):
 
 def sq_str(s):
     t = ffi.string(s).decode()
-    lib.sq_string_free(s)
+    lib.free(s)
     return t
 _str = sq_str
 
diff --git a/ffi/lang/python/sequoia/sequoia_build.py b/ffi/lang/python/sequoia/sequoia_build.py
index 5295243a..07dd5bb6 100644
--- a/ffi/lang/python/sequoia/sequoia_build.py
+++ b/ffi/lang/python/sequoia/sequoia_build.py
@@ -22,6 +22,9 @@ ffibuilder.set_source('_sequoia',
 # cffi magic to make time_t work.
 ffibuilder.cdef('typedef int... time_t;')
 
+# free(3)
+ffibuilder.cdef('void free (void *ptr);')
+
 try:
     ffibuilder.cdef(defs, override=True)
 except error.CDefError as e:
diff --git a/ffi/src/core.rs b/ffi/src/core.rs
index 609b65f9..88ee0570 100644
--- a/ffi/src/core.rs
+++ b/ffi/src/core.rs
@@ -40,7 +40,6 @@
 
 use failure;
 use std::fs::File;
-use std::ffi::CString;
 use std::io::{self, Read, Write, Cursor};
 use std::path::Path;
 use std::ptr;
@@ -76,14 +75,6 @@ pub extern "system" fn sq_context_last_error(ctx: *mut Context)
     maybe_box_raw!(ctx.e.take())
 }
 
-/// Frees a string returned from Sequoia.
-#[no_mangle]
-pub extern "system" fn sq_string_free(s: *mut c_char) {
-    if ! s.is_null() {
-        unsafe { drop(CString::from_raw(s)) }
-    }
-}
-
 /// Creates a Context with reasonable defaults.
 ///
 /// `domain` should uniquely identify your application, it is strongly
diff --git a/ffi/src/error.rs b/ffi/src/error.rs
index 56a6d27d..e6ae201b 100644
--- a/ffi/src/error.rs
+++ b/ffi/src/error.rs
@@ -2,7 +2,6 @@
 
 use failure;
 use std::io;
-use std::ffi::CString;
 use libc::c_char;
 
 extern crate sequoia_openpgp as openpgp;
@@ -16,15 +15,12 @@ pub extern "system" fn sq_error_free(error: Option<&mut failure::Error>) {
 
 /// Returns the error message.
 ///
-/// The returned value must be freed with `sq_string_free`.
+/// The returned value must be freed with `free(3)`.
 #[no_mangle]
 pub extern "system" fn sq_error_string(error: *const failure::Error)
                                        -> *mut c_char {
     let error = ffi_param_ref!(error);
-    CString::new(format!("{}", error))
-        .map(|s| s.into_raw())
-        .unwrap_or(CString::new("Failed to convert error into string")
-                   .unwrap().into_raw())
+    ffi_return_string!(&format!("{}", error))
 }
 
 /// Returns the error status code.
diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs
index dd87e6fa..236f8695 100644
--- a/ffi/src/lib.rs
+++ b/ffi/src/lib.rs
@@ -209,6 +209,53 @@ macro_rules! ffi_param_cstr {
     }};
 }
 
+/* Return value handling.  */
+
+/// Duplicates a string similar to strndup(3).
+fn strndup(src: &[u8]) -> Option<*mut libc::c_char> {
+    if src.contains(&0) {
+        return None;
+    }
+
+    let l = src.len() + 1;
+    let s = unsafe {
+        ::std::slice::from_raw_parts_mut(libc::malloc(l) as *mut u8, l)
+    };
+    &mut s[..l - 1].copy_from_slice(src);
+    s[l - 1] = 0;
+
+    Some(s.as_mut_ptr() as *mut libc::c_char)
+}
+
+/// Transfers a string from Rust to C, allocating it using malloc.
+///
+/// # Panics
+///
+/// Panics if the given string contains a 0.
+macro_rules! ffi_return_string {
+    ($name:expr) => {{
+        let string = $name;
+        let bytes: &[u8] = string.as_ref();
+        ::strndup(bytes).expect(
+            &format!("Returned string {} contains a 0 byte.", stringify!($name))
+        )
+    }};
+}
+
+/// Transfers a string from Rust to C, allocating it using malloc.
+///
+/// # Panics
+///
+/// Does *NOT* panic if the given string contains a 0, but returns
+/// `NULL`.
+macro_rules! ffi_return_maybe_string {
+    ($name:expr) => {{
+        let string = $name;
+        let bytes: &[u8] = string.as_ref();
+        ::strndup(bytes).unwrap_or(::std::ptr::null_mut())
+    }};
+}
+
 /// Like try! for ffi glue.
 ///
 /// Evaluates the given expression.  On success, evaluate to
diff --git a/ffi/src/openpgp/fingerprint.rs b/ffi/src/openpgp/fingerprint.rs
index 0a8ed08b..8e94886a 100644
--- a/ffi/src/openpgp/fingerprint.rs
+++ b/ffi/src/openpgp/fingerprint.rs
@@ -4,7 +4,6 @@
 //!
 //! [`sequoia-openpgp::Fingerprint`]: ../../../sequoia_openpgp/enum.Fingerprint.html
 
-use std::ffi::CString;
 use std::hash::{Hash, Hasher};
 use std::ptr;
 use std::slice;
@@ -81,9 +80,7 @@ pub extern "system" fn sq_fingerprint_as_bytes(fp: *const Fingerprint,
 pub extern "system" fn sq_fingerprint_to_string(fp: *const Fingerprint)
                                                 -> *mut c_char {
     let fp = ffi_param_ref!(fp);
-    CString::new(fp.to_string())
-        .unwrap() // Errors only on internal nul bytes.
-        .into_raw()
+    ffi_return_string!(fp.to_string())
 }
 
 /// Converts the fingerprint to a hexadecimal number.
@@ -91,9 +88,7 @@ pub extern "system" fn sq_fingerprint_to_string(fp: *const Fingerprint)
 pub extern "system" fn sq_fingerprint_to_hex(fp: *const Fingerprint)
                                              -> *mut c_char {
     let fp = ffi_param_ref!(fp);
-    CString::new(fp.to_hex())
-        .unwrap() // Errors only on internal nul bytes.
-        .into_raw()
+    ffi_return_string!(fp.to_hex())
 }
 
 /// Converts the fingerprint to a key ID.
diff --git a/ffi/src/openpgp/keyid.rs b/ffi/src/openpgp/keyid.rs
index 045e5133..bdb93b2d 100644
--- a/ffi/src/openpgp/keyid.rs
+++ b/ffi/src/openpgp/keyid.rs
@@ -4,7 +4,6 @@
 //!
 //! [`sequoia-openpgp::KeyID`]: ../../../sequoia_openpgp/enum.KeyID.html
 
-use std::ffi::CString;
 use std::hash::{Hash, Hasher};
 use std::ptr;
 use std::slice;
@@ -21,6 +20,7 @@ use build_hasher;
 ///
 /// ```c
 /// #include <assert.h>
+/// #include <stdlib.h>
 /// #include <string.h>
 /// #include <sequoia.h>
 ///
@@ -30,7 +30,7 @@ use build_hasher;
 /// assert (strcmp (mr_b_as_string, "BBBB BBBB BBBB BBBB") == 0);
 ///
 /// sq_keyid_free (mr_b);
-/// sq_string_free (mr_b_as_string);
+/// free (mr_b_as_string);
 /// ```
 #[no_mangle]
 pub extern "system" fn sq_keyid_from_bytes(id: *const uint8_t) -> *mut KeyID {
@@ -77,9 +77,7 @@ pub extern "system" fn sq_keyid_hash(id: *const KeyID)
 pub extern "system" fn sq_keyid_to_string(id: *const KeyID)
                                           -> *mut c_char {
     let id = ffi_param_ref!(id);
-    CString::new(id.to_string())
-        .unwrap() // Errors only on internal nul bytes.
-        .into_raw()
+    ffi_return_string!(id.to_string())
 }
 
 /// Converts the KeyID to a hexadecimal number.
@@ -87,9 +85,7 @@ pub extern "system" fn sq_keyid_to_string(id: *const KeyID)
 pub extern "system" fn sq_keyid_to_hex(id: *const KeyID)
                                        -> *mut c_char {
     let id = ffi_param_ref!(id);
-    CString::new(id.to_hex())
-        .unwrap() // Errors only on internal nul bytes.
-        .into_raw()
+    ffi_return_string!(id.to_hex())
 }
 
 /// Compares KeyIDs.
diff --git a/ffi/src/openpgp/tpk.rs b/ffi/src/openpgp/tpk.rs
index 66f38a5e..1a88f46b 100644
--- a/ffi/src/openpgp/tpk.rs
+++ b/ffi/src/openpgp/tpk.rs
@@ -5,7 +5,6 @@
 //! [`sequoia-openpgp::TPK`]: ../../../sequoia_openpgp/struct.TPK.html
 //! [related functionality]: ../../../sequoia_openpgp/tpk/index.html
 
-use std::ffi::CString;
 use std::ptr;
 use std::slice;
 use std::io::{Read, Write};
@@ -441,9 +440,7 @@ pub extern "system" fn sq_tpk_primary_user_id(tpk: *const TPK)
 {
     let tpk = ffi_param_ref!(tpk);
     if let Some(binding) = tpk.userids().nth(0) {
-        CString::new(binding.userid().userid())
-            .unwrap() // Errors only on internal nul bytes.
-            .into_raw()
+        ffi_return_string!(binding.userid().userid())
     } else {
         ptr::null_mut()
     }
@@ -465,11 +462,7 @@ pub extern "system" fn sq_user_id_binding_user_id(
 {
     let binding = ffi_param_ref!(binding);
 
-    if let Ok(c_str) = CString::new(binding.userid().userid()) {
-        c_str.into_raw()
-    } else {
-        ptr::null_mut()
-    }
+    ffi_return_maybe_string!(binding.userid().userid())
 }
 
 /// Returns a reference to the self-signature, if any.
diff --git a/ffi/src/store.rs b/ffi/src/store.rs
index 71cb20a0..ac5fea0e 100644
--- a/ffi/src/store.rs
+++ b/ffi/src/store.rs
@@ -24,7 +24,6 @@
 
 
 use libc::{uint8_t, uint64_t, c_char};
-use std::ffi::CString;
 use std::ptr;
 
 extern crate sequoia_openpgp as openpgp;
@@ -39,7 +38,7 @@ use sequoia_store::{
 };
 
 use super::error::Status;
-use super::core::{Context, sq_string_free};
+use super::core::Context;
 
 
 /// Lists all stores with the given prefix.
@@ -69,15 +68,11 @@ pub extern "system" fn sq_store_iter_next(iter: *mut StoreIter,
     match iter.next() {
         Some((domain, name, policy, store)) => {
             if domainp.is_some() {
-                *domainp.unwrap() = CString::new(domain)
-                    .map(|c| c.into_raw())
-                    .unwrap_or(ptr::null_mut());
+                *domainp.unwrap() = ffi_return_maybe_string!(domain);
             }
 
             if namep.is_some() {
-                *namep.unwrap() = CString::new(name)
-                    .map(|c| c.into_raw())
-                    .unwrap_or(ptr::null_mut());
+                *namep.unwrap() = ffi_return_maybe_string!(name);
             }
 
             if policyp.is_some() {
@@ -142,13 +137,6 @@ pub extern "system" fn sq_key_iter_free(iter: Option<&mut KeyIter>) {
 }
 
 
-fn cstring(s: &str) -> *mut c_char {
-    CString::new(s)
-        .map(|c| c.into_raw())
-        // Fails only on internal nul bytes.
-        .unwrap_or(ptr::null_mut())
-}
-
 /// Returns the next log entry.
 ///
 /// Returns `NULL` on exhaustion.
@@ -159,8 +147,8 @@ pub extern "system" fn sq_log_iter_next(iter: *mut LogIter)
     match iter.next() {
         Some(e) => {
             let (status, error) = match e.status {
-                Ok(s) => (cstring(&s), ptr::null_mut()),
-                Err((s, e)) => (cstring(&s), cstring(&e)),
+                Ok(s) => (ffi_return_string!(&s), ptr::null_mut()),
+                Err((s, e)) => (ffi_return_string!(&s), ffi_return_string!(&e)),
             };
 
             box_raw!(Log{
@@ -168,7 +156,7 @@ pub extern "system" fn sq_log_iter_next(iter: *mut LogIter)
                 store: maybe_box_raw!(e.store),
                 binding: maybe_box_raw!(e.binding),
                 key: maybe_box_raw!(e.key),
-                slug: cstring(&e.slug),
+                slug: ffi_return_string!(&e.slug),
                 status: status,
                 error: error,
             })
@@ -315,9 +303,7 @@ pub extern "system" fn sq_binding_iter_next(iter: *mut BindingIter,
     match iter.next() {
         Some((label, fp, binding)) => {
             if labelp.is_some() {
-                *labelp.unwrap() = CString::new(label)
-                    .map(|c| c.into_raw())
-                    .unwrap_or(ptr::null_mut());
+                *labelp.unwrap() = ffi_return_maybe_string!(label);
             }
 
             if fpp.is_some() {
@@ -373,9 +359,11 @@ pub extern "system" fn sq_log_free(log: Option<&mut Log>) {
         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);
+        unsafe {
+            libc::free(log.slug as *mut libc::c_void);
+            libc::free(log.status as *mut libc::c_void);
+            libc::free(log.error as *mut libc::c_void);
+        }
         drop(log)
     }
 }