use tor allocator for string allocation in rust

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

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)

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				\

src/common/rust_types.c

@@ -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) */
-

src/common/rust_types.h

@@ -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
-

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				\

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
 

src/rust/Cargo.lock

@@ -5,13 +5,6 @@ dependencies = [
  "libc 0.2.22 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
-[[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"
@@ -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"

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

src/rust/c_string/Cargo.toml

@@ -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"]
-

src/rust/c_string/ffi.rs

@@ -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) };
-    }
-}

src/rust/c_string/include.am

@@ -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:
-

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
 }

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 \

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"

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)
 }

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;

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