[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] move to allocating c strings from rust



commit 91bca5c31b9eefe4d07645f690f69914c89a5594
Author: Chelsea Holland Komlo <me@xxxxxxxxxxxxxxxx>
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



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits