[tor-commits] [tor/master] move to allocating c strings from rust
nickm at torproject.org
nickm at torproject.org
Fri Oct 27 14:07:26 UTC 2017
commit 91bca5c31b9eefe4d07645f690f69914c89a5594
Author: Chelsea Holland Komlo <me at 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
More information about the tor-commits
mailing list