[tor-commits] [tor/master] use tor allocator for string allocation in rust
nickm at torproject.org
nickm at torproject.org
Fri Oct 27 14:07:25 UTC 2017
commit be583a34a3815c2c10e86094ab0610e4b7f9c869
Author: Chelsea Holland Komlo <me at 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
More information about the tor-commits
mailing list