commit be583a34a3815c2c10e86094ab0610e4b7f9c869 Author: Chelsea Holland Komlo me@chelseakomlo.com Date: Wed Oct 11 15:21:20 2017 -0400
use tor allocator for string allocation in rust --- Makefile.am | 1 - configure.ac | 2 - src/common/include.am | 2 - src/common/rust_types.c | 55 -------------------------- src/common/rust_types.h | 22 ----------- src/or/include.am | 2 +- src/or/protover_rust.c | 92 ------------------------------------------- src/rust/Cargo.lock | 15 +++---- src/rust/Cargo.toml | 2 +- src/rust/c_string/Cargo.toml | 13 ------ src/rust/c_string/ffi.rs | 19 --------- src/rust/c_string/include.am | 12 ------ src/rust/external/external.rs | 9 +++-- src/rust/include.am | 1 - src/rust/protover/Cargo.toml | 3 ++ src/rust/protover/ffi.rs | 62 ++++++++++------------------- src/rust/protover/lib.rs | 1 + src/rust/protover/protover.rs | 2 +- 18 files changed, 40 insertions(+), 275 deletions(-)
diff --git a/Makefile.am b/Makefile.am index 9067e9a8a..27ee33e1e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -28,7 +28,6 @@ endif if USE_RUST rust_ldadd=$(top_builddir)/src/rust/target/release/@TOR_RUST_UTIL_STATIC_NAME@ rust_ldadd+=$(top_builddir)/src/rust/target/release/@TOR_RUST_PROTOVER_STATIC_NAME@ -rust_ldadd+=$(top_builddir)/src/rust/target/release/@TOR_RUST_C_STRING_STATIC_NAME@ else rust_ldadd= endif diff --git a/configure.ac b/configure.ac index b2b353f6b..e20a6b990 100644 --- a/configure.ac +++ b/configure.ac @@ -441,11 +441,9 @@ if test "x$enable_rust" = "xyes"; then if test "$bwin32" = "true"; then TOR_RUST_UTIL_STATIC_NAME=tor_util.lib TOR_RUST_PROTOVER_STATIC_NAME=libprotover.lib - TOR_RUST_C_STRING_STATIC_NAME=libc_string.lib else TOR_RUST_UTIL_STATIC_NAME=libtor_util.a TOR_RUST_PROTOVER_STATIC_NAME=libprotover.a - TOR_RUST_C_STRING_STATIC_NAME=libc_string.a fi
AC_SUBST(TOR_RUST_UTIL_STATIC_NAME) diff --git a/src/common/include.am b/src/common/include.am index 7ce84e17c..cd5eea340 100644 --- a/src/common/include.am +++ b/src/common/include.am @@ -94,7 +94,6 @@ LIBOR_A_SRC = \ src/common/util_bug.c \ src/common/util_format.c \ src/common/util_process.c \ - src/common/rust_types.c \ src/common/sandbox.c \ src/common/storagedir.c \ src/common/workqueue.c \ @@ -180,7 +179,6 @@ COMMONHEADERS = \ src/common/procmon.h \ src/common/pubsub.h \ src/common/sandbox.h \ - src/common/rust_types.h \ src/common/storagedir.h \ src/common/testsupport.h \ src/common/timers.h \ diff --git a/src/common/rust_types.c b/src/common/rust_types.c deleted file mode 100644 index 1b6dd3990..000000000 --- a/src/common/rust_types.c +++ /dev/null @@ -1,55 +0,0 @@ -/* Copyright (c) 2017, The Tor Project, Inc. */ -/* See LICENSE for licensing information */ - -/** - * \file rust_types.c - * \brief This file is used for handling types returned from Rust to C. - **/ - -#include "or.h" -#include "rust_types.h" - -#ifdef HAVE_RUST - -void free_rust_str(char *ret); - -/* Because Rust strings can only be freed from Rust, we first copy the string's - * contents to a c pointer, and then free the Rust string. - * This function can be extended to return a success/error value if needed. - */ -void -move_rust_str_to_c_and_free(rust_str_ref_t src, char **dest) -{ - if (!src) { - log_warn(LD_BUG, "Received a null pointer from protover rust."); - return; - } - - if (!dest) { - log_warn(LD_BUG, "Received a null pointer from caller to rust. " - "This results in a memory leak due to not freeing the rust " - "string that was meant to be copied.."); - return; - } - - *dest = tor_strdup(src); - free_rust_str(src); - return; -} - -#else - -/* When Rust is not enabled, this function should never be used. Log a warning - * in the case that it is ever called when Rust is not enabled. - */ -void -move_rust_str_to_c_and_free(rust_str_ref_t src, char **dest) -{ - (void) src; - (void) dest; - log_warn(LD_BUG, "Received a call to free a Rust string when we are " - " not running with Rust enabled."); - return; -} -#endif /* defined(HAVE_RUST) */ - diff --git a/src/common/rust_types.h b/src/common/rust_types.h deleted file mode 100644 index b6d807e65..000000000 --- a/src/common/rust_types.h +++ /dev/null @@ -1,22 +0,0 @@ -/* Copyright (c) 2017, The Tor Project, Inc. */ -/* See LICENSE for licensing information */ - -/** - * \file rust_types.h - * \brief Headers for rust_types.c - **/ - -#include "or.h" - -#ifndef TOR_RUST_TYPES_H -#define TOR_RUST_TYPES_H - -/* This type is used to clearly mark strings that have been allocated in Rust, - * and therefore strictly need to use the free_rust_str method to free. - */ -typedef char *rust_str_ref_t; - -void move_rust_str_to_c_and_free(rust_str_ref_t src, char **dest); - -#endif - diff --git a/src/or/include.am b/src/or/include.am index bf3715e95..3ff71d5ad 100644 --- a/src/or/include.am +++ b/src/or/include.am @@ -78,7 +78,7 @@ LIBTOR_A_SOURCES = \ src/or/parsecommon.c \ src/or/periodic.c \ src/or/protover.c \ - src/or/protover_rust.c \ + src/or/protover_rust.c \ src/or/proto_cell.c \ src/or/proto_control0.c \ src/or/proto_ext_or.c \ diff --git a/src/or/protover_rust.c b/src/or/protover_rust.c index 261555d1e..27f19c5fe 100644 --- a/src/or/protover_rust.c +++ b/src/or/protover_rust.c @@ -9,103 +9,11 @@
#include "or.h" #include "protover.h" -#include "rust_types.h"
#ifdef HAVE_RUST
-int rust_protover_all_supported(const char *s, char **missing); -rust_str_ref_t rust_protover_compute_for_old_tor(const char *version); -rust_str_ref_t rust_protover_compute_vote(const smartlist_t *proto_votes, - int threshold); -rust_str_ref_t rust_protover_get_supported_protocols(void); -int rust_protocol_list_supports_protocol(const char *list, protocol_type_t tp, - uint32_t version); -int rust_protover_is_supported_here(protocol_type_t pr, uint32_t ver); - /* Define for compatibility, used in main.c */ void protover_free_all(void) {};
-/* - * Wrap rust_protover_is_supported_here, located in /src/rust/protover - */ -int -protover_is_supported_here(protocol_type_t pr, uint32_t ver) -{ - return rust_protover_is_supported_here(pr, ver); -} - -/* - * Wrap rust_protover_list_supports_protocol, located in /src/rust/protover - */ -int -protocol_list_supports_protocol(const char *list, protocol_type_t tp, - uint32_t version) -{ - return rust_protocol_list_supports_protocol(list, tp, version); -} - -/* - * Wrap rust_protover_get_supported_protocols, located in /src/rust/protover - */ -const char * -protover_get_supported_protocols(void) -{ - rust_str_ref_t rust_protocols = rust_protover_get_supported_protocols(); - - char *protocols = NULL; - if (rust_protocols != NULL) { - move_rust_str_to_c_and_free(rust_protocols, &protocols); - } - return protocols; -} - -/* - * Wrap rust_protover_compute_vote, located in /src/rust/protover - */ -char * -protover_compute_vote(const smartlist_t *proto_strings, - int threshold) -{ - rust_str_ref_t rust_protocols = rust_protover_compute_vote(proto_strings, - threshold); - - char *protocols = NULL; - if (rust_protocols != NULL) { - move_rust_str_to_c_and_free(rust_protocols, &protocols); - } - return protocols; -} - -/* - * Wrap rust_protover_all_supported, located in /src/rust/protover - */ -int -protover_all_supported(const char *s, char **missing_out) -{ - rust_str_ref_t missing_out_copy = NULL; - int is_supported = rust_protover_all_supported(s, &missing_out_copy); - - if (!is_supported && missing_out_copy != NULL) { - move_rust_str_to_c_and_free(missing_out_copy, missing_out); - } - - return is_supported; -} - -/* - * Wrap rust_compute_for_old_tor, located in /src/rust/protover - */ -const char * -protover_compute_for_old_tor(const char *version) -{ - rust_str_ref_t rust_protocols = rust_protover_compute_for_old_tor(version); - - char *protocols = NULL; - if (rust_protocols != NULL) { - move_rust_str_to_c_and_free(rust_protocols, &protocols); - } - return protocols; -} - #endif
diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock index aa91ea355..56cb9d76b 100644 --- a/src/rust/Cargo.lock +++ b/src/rust/Cargo.lock @@ -6,13 +6,6 @@ dependencies = [ ]
[[package]] -name = "c_string" -version = "0.0.1" -dependencies = [ - "libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)", -] - -[[package]] name = "external" version = "0.0.1" dependencies = [ @@ -31,6 +24,7 @@ dependencies = [ "external 0.0.1", "libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)", "smartlist 0.0.1", + "tor_allocate 0.0.1", "tor_util 0.0.1", ]
@@ -41,5 +35,12 @@ dependencies = [ "libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)", ]
+[[package]] +name = "tor_allocate" +version = "0.0.1" +dependencies = [ + "libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)", +] + [metadata] "checksum libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)" = "babb8281da88cba992fa1f4ddec7d63ed96280a1a53ec9b919fd37b53d71e502" diff --git a/src/rust/Cargo.toml b/src/rust/Cargo.toml index 6943627e3..5d4292bbd 100644 --- a/src/rust/Cargo.toml +++ b/src/rust/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = ["tor_util", "protover", "smartlist", "external", "c_string"] +members = ["tor_util", "protover", "smartlist", "external", "tor_allocate"]
[profile.release] debug = true diff --git a/src/rust/c_string/Cargo.toml b/src/rust/c_string/Cargo.toml deleted file mode 100644 index dc7504856..000000000 --- a/src/rust/c_string/Cargo.toml +++ /dev/null @@ -1,13 +0,0 @@ -[package] -authors = ["The Tor Project"] -version = "0.0.1" -name = "c_string" - -[dependencies] -libc = "0.2.22" - -[lib] -name = "c_string" -path = "ffi.rs" -crate_type = ["rlib", "staticlib"] - diff --git a/src/rust/c_string/ffi.rs b/src/rust/c_string/ffi.rs deleted file mode 100644 index edce25082..000000000 --- a/src/rust/c_string/ffi.rs +++ /dev/null @@ -1,19 +0,0 @@ -//! FFI functions, only to be called from C. -//! -//! This module provides the ability for C to free strings that have been -//! allocated in Rust. - -extern crate libc; - -use libc::c_char; -use std::ffi::CString; - -/// This allows strings allocated in Rust to be freed in Rust. Every string -/// sent across the Rust/C FFI boundary should utilize this function for -/// freeing strings allocated in Rust. -#[no_mangle] -pub extern "C" fn free_rust_str(ptr: *mut c_char) { - if !ptr.is_null() { - unsafe { CString::from_raw(ptr) }; - } -} diff --git a/src/rust/c_string/include.am b/src/rust/c_string/include.am deleted file mode 100644 index 8e9229ae6..000000000 --- a/src/rust/c_string/include.am +++ /dev/null @@ -1,12 +0,0 @@ -EXTRA_DIST +=\ - src/rust/c_string/Cargo.toml \ - src/rust/c_string/ffi.rs - -src/rust/target/release/@TOR_RUST_C_STRING_STATIC_NAME@: FORCE - ( cd "$(abs_top_srcdir)/src/rust/c_string" ; \ - CARGO_TARGET_DIR="$(abs_top_builddir)/src/rust/target" \ - CARGO_HOME="$(abs_top_builddir)/src/rust" \ - $(CARGO) build --release --quiet $(CARGO_ONLINE) ) - -FORCE: - diff --git a/src/rust/external/external.rs b/src/rust/external/external.rs index f3267949c..09d80cb2b 100644 --- a/src/rust/external/external.rs +++ b/src/rust/external/external.rs @@ -16,14 +16,15 @@ pub fn c_tor_version_as_new_as(platform: &str, cutoff: &str) -> bool { Ok(n) => n, Err(_) => return false, }; + let c_cutoff = match CString::new(cutoff) { Ok(n) => n, Err(_) => return false, };
- let result: c_int; - unsafe { - result = tor_version_as_new_as(c_platform.as_ptr(), c_cutoff.as_ptr()); - } + let result: c_int = unsafe { + tor_version_as_new_as(c_platform.as_ptr(), c_cutoff.as_ptr()) + }; + result == 1 } diff --git a/src/rust/include.am b/src/rust/include.am index cba92c28a..716d46f4c 100644 --- a/src/rust/include.am +++ b/src/rust/include.am @@ -1,6 +1,5 @@ include src/rust/tor_util/include.am include src/rust/protover/include.am -include src/rust/c_string/include.am
EXTRA_DIST +=\ src/rust/Cargo.toml \ diff --git a/src/rust/protover/Cargo.toml b/src/rust/protover/Cargo.toml index a8f794f83..04d2f2ed7 100644 --- a/src/rust/protover/Cargo.toml +++ b/src/rust/protover/Cargo.toml @@ -15,6 +15,9 @@ path = "../external" [dependencies.tor_util] path = "../tor_util"
+[dependencies.tor_allocate] +path = "../tor_allocate" + [lib] name = "protover" path = "lib.rs" diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 7365d7cd8..539b6c0ba 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -8,6 +8,7 @@ use std::ffi::CString;
use protover::*; use smartlist::*; +use tor_allocate::allocate_string;
/// Translate C enums to Rust Proto enums, using the integer value of the C /// enum to map to its associated Rust enum @@ -32,7 +33,7 @@ fn translate_to_rust(c_proto: uint32_t) -> Result<Proto, &'static str> { /// Provide an interface for C to translate arguments and return types for /// protover::all_supported #[no_mangle] -pub extern "C" fn rust_protover_all_supported( +pub extern "C" fn protover_all_supported( c_relay_version: *const c_char, missing_out: *mut *mut c_char, ) -> c_int { @@ -43,10 +44,7 @@ pub extern "C" fn rust_protover_all_supported(
// Require an unsafe block to read the version from a C string. The pointer // is checked above to ensure it is not null. - let c_str: &CStr; - unsafe { - c_str = CStr::from_ptr(c_relay_version); - } + let c_str: &CStr = unsafe { CStr::from_ptr(c_relay_version) };
let relay_version = match c_str.to_str() { Ok(n) => n, @@ -71,7 +69,7 @@ pub extern "C" fn rust_protover_all_supported( /// Provide an interface for C to translate arguments and return types for /// protover::list_supports_protocol #[no_mangle] -pub extern "C" fn rust_protocol_list_supports_protocol( +pub extern "C" fn protocol_list_supports_protocol( c_protocol_list: *const c_char, c_protocol: uint32_t, version: uint32_t, @@ -82,10 +80,7 @@ pub extern "C" fn rust_protocol_list_supports_protocol(
// Require an unsafe block to read the version from a C string. The pointer // is checked above to ensure it is not null. - let c_str: &CStr; - unsafe { - c_str = CStr::from_ptr(c_protocol_list); - } + let c_str: &CStr = unsafe { CStr::from_ptr(c_protocol_list) };
let protocol_list = match c_str.to_str() { Ok(n) => n, @@ -106,7 +101,7 @@ pub extern "C" fn rust_protocol_list_supports_protocol( /// Provide an interface for C to translate arguments and return types for /// protover::get_supported_protocols #[no_mangle] -pub extern "C" fn rust_protover_get_supported_protocols() -> *mut c_char { +pub extern "C" fn protover_get_supported_protocols() -> *mut c_char { // Not handling errors when unwrapping as the content is controlled // and is an empty string let empty = CString::new("").unwrap(); @@ -123,38 +118,29 @@ pub extern "C" fn rust_protover_get_supported_protocols() -> *mut c_char { /// Provide an interface for C to translate arguments and return types for /// protover::compute_vote #[no_mangle] -pub extern "C" fn rust_protover_compute_vote( +pub extern "C" fn protover_compute_vote( list: *const Stringlist, threshold: c_int, ) -> *mut c_char { - // Not handling errors when unwrapping as the content is controlled - // and is an empty string - let empty = CString::new("").unwrap();
if list.is_null() { - return empty.into_raw(); + let mut empty = String::new(); + return allocate_string(&mut empty); }
// Dereference of raw pointer requires an unsafe block. The pointer is // checked above to ensure it is not null. - let data: Vec<String>; - unsafe { - data = (*list).get_list(); - } + let data: Vec<String> = unsafe { (*list).get_list() };
- let vote = compute_vote(data, threshold); - let c_vote = match CString::new(vote) { - Ok(n) => n, - Err(_) => return empty.into_raw(), - }; + let mut vote = compute_vote(data, threshold);
- c_vote.into_raw() + allocate_string(&mut vote) }
/// Provide an interface for C to translate arguments and return types for /// protover::is_supported_here #[no_mangle] -pub extern "C" fn rust_protover_is_supported_here( +pub extern "C" fn protover_is_supported_here( c_protocol: uint32_t, version: uint32_t, ) -> c_int { @@ -171,35 +157,27 @@ pub extern "C" fn rust_protover_is_supported_here( /// Provide an interface for C to translate arguments and return types for /// protover::compute_for_old_tor #[no_mangle] -pub extern "C" fn rust_protover_compute_for_old_tor( +pub extern "C" fn protover_compute_for_old_tor( version: *const c_char, ) -> *mut c_char { // Not handling errors when unwrapping as the content is controlled // and is an empty string - let empty = CString::new("").unwrap(); + let mut empty = String::new();
if version.is_null() { - return empty.into_raw(); + return allocate_string(&mut empty); }
// Require an unsafe block to read the version from a C string. The pointer // is checked above to ensure it is not null. - let c_str: &CStr; - unsafe { - c_str = CStr::from_ptr(version); - } + let c_str: &CStr = unsafe { CStr::from_ptr(version) };
let version = match c_str.to_str() { Ok(n) => n, - Err(_) => return empty.into_raw(), + Err(_) => return allocate_string(&mut empty), };
- let supported = compute_for_old_tor(&version); + let mut supported = compute_for_old_tor(&version);
- let c_supported = match CString::new(supported) { - Ok(n) => n, - Err(_) => return empty.into_raw(), - }; - - c_supported.into_raw() + allocate_string(&mut supported) } diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index 89378c7b7..620191f88 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -27,6 +27,7 @@ extern crate libc; extern crate smartlist; extern crate external; +extern crate tor_allocate;
mod protover; pub mod ffi; diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 0893362ce..f85d16b73 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -364,7 +364,7 @@ fn expand_version_range(range: &str) -> Result<Vec<u32>, &'static str> { "cannot parse protocol range upper bound", ))?;
- Ok((lower...higher).collect()) + Ok((lower..=higher).collect()) }
/// Checks to see if there is a continuous range of integers, starting at the