commit 91bca5c31b9eefe4d07645f690f69914c89a5594 Author: Chelsea Holland Komlo me@chelseakomlo.com Date: Sun Oct 22 00:07:16 2017 -0400

move to allocating c strings from rust --- src/common/compat_rust.c | 39 ------------- src/common/compat_rust.h | 28 --------- src/common/include.am | 6 -- src/or/main.c | 18 +++--- src/rust/Cargo.lock | 1 + src/rust/tor_allocate/tor_allocate.rs | 4 +- src/rust/tor_util/Cargo.toml | 3 + src/rust/tor_util/ffi.rs | 57 ++++--------------- src/rust/tor_util/lib.rs | 11 +--- src/rust/tor_util/rust_string.rs | 101 --------------------------------- src/rust/tor_util/tests/rust_string.rs | 37 ------------ src/test/include.am | 1 - src/test/test.c | 1 - src/test/test.h | 1 - src/test/test_rust.c | 31 ---------- src/test/test_rust.sh | 5 +- 16 files changed, 33 insertions(+), 311 deletions(-)

diff --git a/src/common/compat_rust.c b/src/common/compat_rust.c deleted file mode 100644 index 366fd4037..000000000 --- a/src/common/compat_rust.c +++ /dev/null @@ -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; -} - diff --git a/src/common/compat_rust.h b/src/common/compat_rust.h deleted file mode 100644 index 72fde3929..000000000 --- a/src/common/compat_rust.h +++ /dev/null @@ -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) */ - diff --git a/src/common/include.am b/src/common/include.am index cd5eea340..2856c40fd 100644 --- a/src/common/include.am +++ b/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 \ diff --git a/src/or/main.c b/src/or/main.c index 65b0b8f4d..e9f636aa5 100644 --- a/src/or/main.c +++ b/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."); }

- { - rust_str_t rust_str = rust_welcome_string(); - const char *s = rust_str_get(rust_str); - if (strlen(s) > 0) { - log_notice(LD_GENERAL, "%s", s); - } - rust_str_free(rust_str); +#ifdef HAVE_RUST + char *rust_str = rust_welcome_string(); + if (rust_str != NULL && strlen(rust_str) > 0) { + log_notice(LD_GENERAL, "%s", rust_str); } + tor_free(rust_str); +#endif

if (network_init()<0) { log_err(LD_BUG,"Error initializing network; exiting."); diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock index a5686979f..224d2135b 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 de1d13942..e2fc3ea36 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 906833ce2..d7379a598 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 af4bfc41a..9a5630936 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 79d583d1a..9c863e39b 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 46ec3fd7a..000000000 --- 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 1ff605a43..000000000 --- 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); -} diff --git a/src/test/include.am b/src/test/include.am index 95efe4e79..f66986316 100644 --- a/src/test/include.am +++ b/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 \ diff --git a/src/test/test.c b/src/test/test.c index 27c776b30..95cab99b5 100644 --- a/src/test/test.c +++ b/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 }, diff --git a/src/test/test.h b/src/test/test.h index ba7b767c3..454573c2f 100644 --- a/src/test/test.h +++ b/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[]; diff --git a/src/test/test_rust.c b/src/test/test_rust.c deleted file mode 100644 index 6ad57d6fc..000000000 --- a/src/test/test_rust.c +++ /dev/null @@ -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 -}; - diff --git a/src/test/test_rust.sh b/src/test/test_rust.sh index d559f94ce..50740fd18 100755 --- a/src/test/test_rust.sh +++ b/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