move to allocating c strings from rust

7 files changed, 20 insertions, 194 deletions

diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock
index a5686979fa..224d2135bf 100644
--- a/src/rust/Cargo.lock
+++ b/src/rust/Cargo.lock
@@ -3,6 +3,7 @@ name = "tor_util"
 version = "0.0.1"
 dependencies = [
  "libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)",
+ "tor_allocate 0.0.1",
 ]
 
 [[package]]
diff --git a/src/rust/tor_allocate/tor_allocate.rs b/src/rust/tor_allocate/tor_allocate.rs
index de1d13942e..e2fc3ea36c 100644
--- a/src/rust/tor_allocate/tor_allocate.rs
+++ b/src/rust/tor_allocate/tor_allocate.rs
@@ -26,9 +26,9 @@ extern "C" fn tor_malloc_ ( size: usize) ->  *mut c_void {
 /// A `String` that should be freed by tor_free in C
 ///
 pub fn allocate_and_copy_string(src: &String) -> *mut c_char {
-    let bytes = s.as_bytes();
+    let bytes = src.as_bytes();
 
-    let size = s.len();
+    let size = bytes.len();
     let size_with_null_byte = size + 1;
 
     let dest = unsafe { tor_malloc_(size_with_null_byte) as *mut u8 };
diff --git a/src/rust/tor_util/Cargo.toml b/src/rust/tor_util/Cargo.toml
index 906833ce24..d7379a5988 100644
--- a/src/rust/tor_util/Cargo.toml
+++ b/src/rust/tor_util/Cargo.toml
@@ -8,6 +8,9 @@ name = "tor_util"
 path = "lib.rs"
 crate_type = ["rlib", "staticlib"]
 
+[dependencies.tor_allocate]
+path = "../tor_allocate"
+
 [dependencies]
 libc = "0.2.22"
 
diff --git a/src/rust/tor_util/ffi.rs b/src/rust/tor_util/ffi.rs
index af4bfc41af..9a56309365 100644
--- a/src/rust/tor_util/ffi.rs
+++ b/src/rust/tor_util/ffi.rs
@@ -1,56 +1,21 @@
-//! FFI functions, only to be called from C.
+//! FFI functions to announce Rust support during tor startup, only to be
+//! called from C.
 //!
-//! Equivalent C versions of these live in `src/common/compat_rust.c`
 
-use std::mem::forget;
-use std::ffi::CString;
-
-use libc;
-use rust_string::RustString;
-
-/// Free the passed `RustString` (`rust_str_t` in C), to be used in place of
-/// `tor_free`().
-///
-/// # Examples
-/// ```c
-/// rust_str_t r_s = rust_welcome_string();
-/// rust_str_free(r_s);
-/// ```
-#[no_mangle]
-#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))]
-pub unsafe extern "C" fn rust_str_free(_str: RustString) {
-    // Empty body: Just drop _str and we're done (Drop takes care of it).
-}
-
-/// Lends an immutable, NUL-terminated C String.
-///
-/// # Examples
-/// ```c
-/// rust_str_t r_s = rust_welcome_string();
-/// const char *s = rust_str_get(r_s);
-/// printf("%s", s);
-/// rust_str_free(r_s);
-/// ```
-#[no_mangle]
-pub unsafe extern "C" fn rust_str_get(str: RustString) -> *const libc::c_char {
-    let res = str.as_ptr();
-    forget(str);
-    res
-}
+use libc::c_char;
+use tor_allocate::allocate_and_copy_string;
 
 /// Returns a short string to announce Rust support during startup.
 ///
 /// # Examples
 /// ```c
-/// rust_str_t r_s = rust_welcome_string();
-/// const char *s = rust_str_get(r_s);
-/// printf("%s", s);
-/// rust_str_free(r_s);
+/// char *rust_str = rust_welcome_string();
+/// printf("%s", rust_str);
+/// tor_free(rust_str);
 /// ```
 #[no_mangle]
-pub extern "C" fn rust_welcome_string() -> RustString {
-    let s = CString::new("Tor is running with Rust integration. Please report \
-                          any bugs you encouter.")
-            .unwrap();
-    RustString::from(s)
+pub extern "C" fn rust_welcome_string() -> *mut c_char {
+    let rust_welcome = String::from("Tor is running with Rust integration. Please report \
+                          any bugs you encouter.");
+    allocate_and_copy_string(&rust_welcome)
 }
diff --git a/src/rust/tor_util/lib.rs b/src/rust/tor_util/lib.rs
index 79d583d1ae..9c863e39bd 100644
--- a/src/rust/tor_util/lib.rs
+++ b/src/rust/tor_util/lib.rs
@@ -1,13 +1,8 @@
-//! C <-> Rust compatibility helpers and types.
+//! Small module to announce Rust support during startup for demonstration
+//! purposes.
 //!
-//! Generically useful, small scale helpers should go here. This goes for both
-//! the C side (in the form of the ffi module) as well as the Rust side
-//! (individual modules per functionality). The corresponding C stuff lives in
-//! `src/common/compat_rust.{c,h}`.
 
 extern crate libc;
+extern crate tor_allocate;
 
-mod rust_string;
 pub mod ffi;
-
-pub use rust_string::*;
diff --git a/src/rust/tor_util/rust_string.rs b/src/rust/tor_util/rust_string.rs
deleted file mode 100644
index 46ec3fd7a8..0000000000
--- a/src/rust/tor_util/rust_string.rs
+++ /dev/null
@@ -1,101 +0,0 @@
-use std::ffi::CString;
-use std::mem::forget;
-use libc;
-
-/// Compatibility wrapper for strings allocated in Rust and passed to C.
-///
-/// Rust doesn't ensure the safety of freeing memory across an FFI boundary, so
-/// we need to take special care to ensure we're not accidentally calling
-/// `tor_free`() on any string allocated in Rust. To more easily differentiate
-/// between strings that possibly (if Rust support is enabled) were allocated
-/// in Rust, C has the `rust_str_t` helper type. The equivalent on the Rust
-/// side is `RustString`.
-///
-/// Note: This type must not be used for strings allocated in C.
-#[repr(C)]
-#[derive(Debug)]
-pub struct RustString(*mut libc::c_char);
-
-impl RustString {
-    /// Returns a pointer to the underlying NUL-terminated byte array.
-    ///
-    /// Note that this function is not typically useful for Rust callers,
-    /// except in a direct FFI context.
-    ///
-    /// # Examples
-    /// ```
-    /// # use tor_util::RustString;
-    /// use std::ffi::CString;
-    ///
-    /// let r = RustString::from(CString::new("asdf").unwrap());
-    /// let c_str = r.as_ptr();
-    /// assert_eq!(b'a', unsafe { *c_str as u8});
-    /// ```
-    pub fn as_ptr(&self) -> *const libc::c_char {
-        self.0 as *const libc::c_char
-    }
-}
-
-impl From<CString> for RustString {
-    /// Constructs a new `RustString`
-    ///
-    /// # Examples
-    /// ```
-    /// # use tor_util::RustString;
-    /// use std::ffi::CString;
-    ///
-    /// let r = RustString::from(CString::new("asdf").unwrap());
-    /// ```
-    fn from(str: CString) -> RustString {
-        RustString(str.into_raw())
-    }
-}
-
-impl Into<CString> for RustString {
-    /// Reconstructs a `CString` from this `RustString`.
-    ///
-    /// Useful to take ownership back from a `RustString` that was given to C
-    /// code.
-    ///
-    /// # Examples
-    /// ```
-    /// # use tor_util::RustString;
-    /// use std::ffi::CString;
-    ///
-    /// let cs = CString::new("asdf").unwrap();
-    /// let r = RustString::from(cs.clone());
-    /// let cs2 = r.into();
-    /// assert_eq!(cs, cs2);
-    /// ```
-    fn into(self) -> CString {
-        // Calling from_raw is always OK here: We only construct self using
-        // valid CStrings and don't expose anything that could mutate it
-        let ret = unsafe { CString::from_raw(self.0) };
-        forget(self);
-        ret
-    }
-}
-
-impl Drop for RustString {
-    fn drop(&mut self) {
-        // Don't use into() here, because we would need to move out of
-        // self. Same safety consideration. Immediately drop the created
-        // CString, which takes care of freeing the wrapped string.
-        unsafe { CString::from_raw(self.0) };
-    }
-}
-
-#[cfg(test)]
-mod test {
-    use std::mem;
-    use super::*;
-
-    use libc;
-
-    /// Ensures we're not adding overhead by using RustString.
-    #[test]
-    fn size_of() {
-        assert_eq!(mem::size_of::<*mut libc::c_char>(),
-                   mem::size_of::<RustString>())
-    }
-}
diff --git a/src/rust/tor_util/tests/rust_string.rs b/src/rust/tor_util/tests/rust_string.rs
deleted file mode 100644
index 1ff605a43c..0000000000
--- a/src/rust/tor_util/tests/rust_string.rs
+++ /dev/null
@@ -1,37 +0,0 @@
-extern crate tor_util;
-extern crate libc;
-
-use std::ffi::CString;
-use tor_util::RustString;
-
-#[test]
-fn rust_string_conversions_preserve_c_string() {
-    let s = CString::new("asdf foo").unwrap();
-    let r = RustString::from(s.clone());
-    let r2 = RustString::from(s.clone());
-    let c = r2.as_ptr();
-    assert_eq!(unsafe { libc::strlen(c) }, 8);
-    let c_str = r.into();
-    assert_eq!(s, c_str);
-}
-
-#[test]
-fn empty_string() {
-    let s = CString::new("").unwrap();
-    let r = RustString::from(s.clone());
-    let c = r.as_ptr();
-    assert_eq!(unsafe { libc::strlen(c) }, 0);
-    let c_str = r.into();
-    assert_eq!(s, c_str);
-}
-
-#[test]
-fn c_string_with_unicode() {
-    // The euro sign is three bytes
-    let s = CString::new("asd€asd").unwrap();
-    let r = RustString::from(s.clone());
-    let c = r.as_ptr();
-    assert_eq!(unsafe { libc::strlen(c) }, 9);
-    let c_str = r.into();
-    assert_eq!(s, c_str);
-}