move to allocating c strings from rust

src/common/compat_rust.c

@@ -1,39 +0,0 @@
-/* Copyright (c) 2017, The Tor Project, Inc. */
-/* See LICENSE for licensing information */
-
-/**
- * \file rust_compat.c
- * \brief Rust FFI compatibility functions and helpers. This file is only built
- * if Rust is not used.
- **/
-
-#include "compat_rust.h"
-#include "util.h"
-
-/**
- * Free storage pointed to by <b>str</b>, and itself.
- */
-void
-rust_str_free(rust_str_t str)
-{
-    char *s = (char *)str;
-    tor_free(s);
-}
-
-/**
- * Return zero-terminated contained string.
- */
-const char *
-rust_str_get(const rust_str_t str)
-{
-    return (const char *)str;
-}
-
-/* If we were using Rust, we'd say so on startup. */
-rust_str_t
-rust_welcome_string(void)
-{
-    char *s = tor_malloc_zero(1);
-    return (rust_str_t)s;
-}
-

src/common/compat_rust.h

@@ -1,28 +0,0 @@
-/* Copyright (c) 2017, The Tor Project, Inc. */
-/* See LICENSE for licensing information */
-
-/**
- * \file rust_compat.h
- * \brief Headers for rust_compat.c
- **/
-
-#ifndef TOR_RUST_COMPAT_H
-#define TOR_RUST_COMPAT_H
-
-#include "torint.h"
-
-/**
- * Strings allocated in Rust must be freed from Rust code again. Let's make
- * it less likely to accidentally mess up and call tor_free() on it, because
- * currently it'll just work but might break at any time.
- */
-typedef uintptr_t rust_str_t;
-
-void rust_str_free(rust_str_t);
-
-const char *rust_str_get(const rust_str_t);
-
-rust_str_t rust_welcome_string(void);
-
-#endif /* !defined(TOR_RUST_COMPAT_H) */
-

src/common/include.am

@@ -101,11 +101,6 @@ LIBOR_A_SRC = \
   $(threads_impl_source)				\
   $(readpassphrase_source)
 
-if USE_RUST
-else
-LIBOR_A_SRC += src/common/compat_rust.c
-endif
-
 src/common/src_common_libor_testing_a-log.$(OBJEXT) \
   src/common/log.$(OBJEXT): micro-revision.i
 
@@ -156,7 +151,6 @@ COMMONHEADERS = \
   src/common/compat.h				\
   src/common/compat_libevent.h			\
   src/common/compat_openssl.h			\
-  src/common/compat_rust.h			\
   src/common/compat_threads.h			\
   src/common/compat_time.h			\
   src/common/compress.h				\

src/or/main.c

@@ -60,7 +60,6 @@
 #include "circuitlist.h"
 #include "circuituse.h"
 #include "command.h"
-#include "compat_rust.h"
 #include "compress.h"
 #include "config.h"
 #include "confparse.h"
@@ -128,6 +127,10 @@
 
 void evdns_shutdown(int);
 
+// helper function defined in Rust to output a log message indicating if tor is
+// running with Rust enabled. See src/rust/tor_util
+char *rust_welcome_string(void);
+
 /********* PROTOTYPES **********/
 
 static void dumpmemusage(int severity);
@@ -3111,14 +3114,13 @@ tor_init(int argc, char *argv[])
                  "Expect more bugs than usual.");
   }
 
-  {
+#ifdef HAVE_RUST
-    rust_str_t rust_str = rust_welcome_string();
+  char *rust_str = rust_welcome_string();
-    const char *s = rust_str_get(rust_str);
+  if (rust_str != NULL && strlen(rust_str) > 0) {
-    if (strlen(s) > 0) {
+    log_notice(LD_GENERAL, "%s", rust_str);
-      log_notice(LD_GENERAL, "%s", s);
-    }
-    rust_str_free(rust_str);
   }
+  tor_free(rust_str);
+#endif
 
   if (network_init()<0) {
     log_err(LD_BUG,"Error initializing network; exiting.");

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]]

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 };

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"
 

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 libc::c_char;
-use std::ffi::CString;
+use tor_allocate::allocate_and_copy_string;
-
-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
-}
 
 /// Returns a short string to announce Rust support during startup.
 ///
 /// # Examples
 /// ```c
-/// rust_str_t r_s = rust_welcome_string();
+/// char *rust_str = rust_welcome_string();
-/// const char *s = rust_str_get(r_s);
+/// printf("%s", rust_str);
-/// printf("%s", s);
+/// tor_free(rust_str);
-/// rust_str_free(r_s);
 /// ```
 #[no_mangle]
-pub extern "C" fn rust_welcome_string() -> RustString {
+pub extern "C" fn rust_welcome_string() -> *mut c_char {
-    let s = CString::new("Tor is running with Rust integration. Please report \
+    let rust_welcome = String::from("Tor is running with Rust integration. Please report \
-                          any bugs you encouter.")
+                          any bugs you encouter.");
-            .unwrap();
+    allocate_and_copy_string(&rust_welcome)
-    RustString::from(s)
 }

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::*;

src/rust/tor_util/rust_string.rs

@@ -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>())
-    }
-}

src/rust/tor_util/tests/rust_string.rs

@@ -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);
-}

src/test/include.am

@@ -148,7 +148,6 @@ src_test_test_SOURCES = \
 	src/test/test_routerkeys.c \
 	src/test/test_routerlist.c \
 	src/test/test_routerset.c \
-	src/test/test_rust.c \
 	src/test/test_scheduler.c \
 	src/test/test_shared_random.c \
 	src/test/test_socks.c \

src/test/test.c

@@ -1225,7 +1225,6 @@ struct testgroup_t testgroups[] = {
   { "routerkeys/", routerkeys_tests },
   { "routerlist/", routerlist_tests },
   { "routerset/" , routerset_tests },
-  { "rust/", rust_tests },
   { "scheduler/", scheduler_tests },
   { "socks/", socks_tests },
   { "shared-random/", sr_tests },

src/test/test.h

@@ -238,7 +238,6 @@ extern struct testcase_t router_tests[];
 extern struct testcase_t routerkeys_tests[];
 extern struct testcase_t routerlist_tests[];
 extern struct testcase_t routerset_tests[];
-extern struct testcase_t rust_tests[];
 extern struct testcase_t scheduler_tests[];
 extern struct testcase_t storagedir_tests[];
 extern struct testcase_t socks_tests[];

src/test/test_rust.c

@@ -1,31 +0,0 @@
-/* Copyright (c) 2017, The Tor Project, Inc. */
-/* See LICENSE for licensing information */
-
-#include "orconfig.h"
-#include "compat_rust.h"
-#include "test.h"
-#include "util.h"
-
-static void
-test_welcome_string(void *arg)
-{
-  (void)arg;
-  rust_str_t s = rust_welcome_string();
-  const char *c_str = rust_str_get(s);
-  tt_assert(c_str);
-  size_t len = strlen(c_str);
-#ifdef HAVE_RUST
-  tt_assert(len > 0);
-#else
-  tt_assert(len == 0);
-#endif
-
- done:
-  rust_str_free(s);
-}
-
-struct testcase_t rust_tests[] = {
-  { "welcome_string", test_welcome_string, 0, NULL, NULL },
-  END_OF_TESTCASES
-};
-

src/test/test_rust.sh

@@ -1,13 +1,14 @@
 #!/bin/sh
-# Test all the Rust crates we're using
+# Test all Rust crates
 
-crates=tor_util
+crates="protover tor_util smartlist tor_allocate"
 
 exitcode=0
 
 for crate in $crates; do
     cd "${abs_top_srcdir:-.}/src/rust/${crate}"
     CARGO_TARGET_DIR="${abs_top_builddir}/src/rust/target" CARGO_HOME="${abs_top_builddir}/src/rust" "${CARGO:-cargo}" test ${CARGO_ONLINE-"--frozen"} || exitcode=1
+    cd -
 done
 
 exit $exitcode