move macro body to inline rust function

5 files changed, 52 insertions, 111 deletions

diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs
index a9e8e1ee64..05687c1fbf 100644
--- a/src/rust/protover/lib.rs
+++ b/src/rust/protover/lib.rs
@@ -26,6 +26,8 @@ extern crate libc;
 extern crate smartlist;
 extern crate external;
 extern crate tor_allocate;
+
+#[macro_use]
 extern crate tor_log;
 
 mod protover;
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index bb2b8f056a..33d49ea485 100644
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -6,7 +6,7 @@ use std::fmt;
 use std::collections::{HashMap, HashSet};
 use std::string::String;
 
-use tor_log::*;
+use tor_log::{LogSeverity, LogDomain};
 use external::c_tor_version_as_new_as;
 
 /// The first version of Tor that included "proto" entries in its descriptors.
diff --git a/src/rust/tor_log/tor_log.rs b/src/rust/tor_log/tor_log.rs
index badbf3a29d..f583ac6dcc 100644
--- a/src/rust/tor_log/tor_log.rs
+++ b/src/rust/tor_log/tor_log.rs
@@ -1,6 +1,8 @@
 // Copyright (c) 2016-2017, The Tor Project, Inc. */
 // See LICENSE for licensing information */
 
+// Note that these functions are untested due to the fact that there are no
+// return variables to test and they are calling into a C API.
 
 /// The related domain which the logging message is relevant. For example,
 /// log messages relevant to networking would use LogDomain::LdNet, whereas
@@ -39,64 +41,65 @@ macro_rules! tor_log_msg {
      $($message:tt)*) =>
     {
         {
-            use std::ffi::CString;
-
-            /// Default function name to log in case of errors when converting
-            /// a function name to a CString
-            const ERR_LOG_FUNCTION: &str = "tor_log_msg";
+            let msg = format!($($message)*);
+            $crate::tor_log_msg_impl($severity, $domain, $function, msg)
+        }
+    };
+}
 
-            /// Default message to log in case of errors when converting a log
-            /// message to a CString
-            const ERR_LOG_MSG: &str = "Unable to log message from Rust
+#[inline]
+pub fn tor_log_msg_impl(
+    severity: LogSeverity,
+    domain: LogDomain,
+    function: &str,
+    message: String,
+) {
+    use std::ffi::CString;
+
+    /// Default function name to log in case of errors when converting
+    /// a function name to a CString
+    const ERR_LOG_FUNCTION: &str = "tor_log_msg";
+
+    /// Default message to log in case of errors when converting a log
+    /// message to a CString
+    const ERR_LOG_MSG: &str = "Unable to log message from Rust
             module due to error when converting to CString";
 
-            let func = match CString::new($function) {
-                Ok(n) => n,
-                Err(_) => CString::new(ERR_LOG_FUNCTION).unwrap(),
-            };
+    let func = match CString::new(function) {
+        Ok(n) => n,
+        Err(_) => CString::new(ERR_LOG_FUNCTION).unwrap(),
+    };
 
-            let msg = match CString::new(format!($($message)*)) {
-                Ok(n) => n,
-                Err(_) => CString::new(ERR_LOG_MSG).unwrap(),
-            };
+    let msg = match CString::new(message) {
+        Ok(n) => n,
+        Err(_) => CString::new(ERR_LOG_MSG).unwrap(),
+    };
 
-            let func_ptr = func.as_ptr();
-            let msg_ptr = msg.as_ptr();
+    // Bind to a local variable to preserve ownership. This is essential so
+    // that ownership is guaranteed until these local variables go out of scope
+    let func_ptr = func.as_ptr();
+    let msg_ptr = msg.as_ptr();
 
-            let c_severity = unsafe { translate_severity($severity) };
-            let c_domain = unsafe { translate_domain($domain) };
+    let c_severity = unsafe { log::translate_severity(severity) };
+    let c_domain = unsafe { log::translate_domain(domain) };
 
-            unsafe {
-                $crate::tor_log_string(c_severity, c_domain, func_ptr, msg_ptr )
-            }
-        }
-    };
+    unsafe { log::tor_log_string(c_severity, c_domain, func_ptr, msg_ptr) }
 }
 
-/// This module exposes no-op functionality purely for the purpose of testing
-/// Rust at the module level.
+/// This module exposes no-op functionality for testing other Rust modules
+/// without linking to C.
 #[cfg(any(test, feature = "testing"))]
 pub mod log {
     use libc::{c_char, c_int};
     use super::LogDomain;
     use super::LogSeverity;
 
-    /// Expose a no-op logging interface purely for testing Rust modules at the
-    /// module level.
-    pub fn tor_log_string<'a>(
-        severity: c_int,
-        domain: u32,
-        function: *const c_char,
-        message: *const c_char,
-    ) -> (c_int, u32, String, String) {
-        use std::ffi::CStr;
-
-        let func = unsafe { CStr::from_ptr(function) }.to_str().unwrap();
-        let func_allocated = String::from(func);
-
-        let msg = unsafe { CStr::from_ptr(message) }.to_str().unwrap();
-        let msg_allocated = String::from(msg);
-        (severity, domain, func_allocated, msg_allocated)
+    pub unsafe fn tor_log_string<'a>(
+        _severity: c_int,
+        _domain: u32,
+        _function: *const c_char,
+        _message: *const c_char,
+    ) {
     }
 
     pub unsafe fn translate_domain(_domain: LogDomain) -> u32 {
@@ -143,7 +146,6 @@ pub mod log {
 
     /// Translate Rust defintions of log severity levels to C. This exposes a
     /// 1:1 mapping between types.
-    #[allow(unreachable_patterns)]
     pub unsafe fn translate_severity(severity: LogSeverity) -> c_int {
         match severity {
             LogSeverity::Warn => _LOG_WARN,
@@ -162,68 +164,3 @@ pub mod log {
         );
     }
 }
-
-#[cfg(test)]
-mod test {
-    use tor_log::*;
-    use tor_log::log::*;
-
-    use libc::c_int;
-
-    #[test]
-    fn test_get_log_message() {
-
-        fn test_macro<'a>() -> (c_int, u32, String, String) {
-            let (x, y, z, a) =
-                tor_log_msg!(
-                    LogSeverity::Warn,
-                    LogDomain::LdNet,
-                    "test_macro",
-                    "test log message {}",
-                    "a",
-                    );
-            (x, y, z, a)
-        }
-
-        let (severity, domain, function_name, log_msg) = test_macro();
-
-        let expected_severity =
-            unsafe { translate_severity(LogSeverity::Warn) };
-        assert_eq!(severity, expected_severity);
-
-        let expected_domain = unsafe { translate_domain(LogDomain::LdNet) };
-        assert_eq!(domain, expected_domain);
-
-        assert_eq!("test_macro", function_name);
-        assert_eq!("test log message a", log_msg);
-    }
-
-    #[test]
-    fn test_get_log_message_multiple_values() {
-        fn test_macro<'a>() -> (c_int, u32, String, String) {
-            let (x, y, z, a) = tor_log_msg!(
-                LogSeverity::Warn,
-                LogDomain::LdNet,
-                "test_macro 2",
-                "test log message {} {} {} {}",
-                10,
-                9,
-                8,
-                7
-            );
-            (x, y, z, a)
-        }
-
-        let (severity, domain, function_name, log_msg) = test_macro();
-
-        let expected_severity =
-            unsafe { translate_severity(LogSeverity::Warn) };
-        assert_eq!(severity, expected_severity);
-
-        let expected_domain = unsafe { translate_domain(LogDomain::LdNet) };
-        assert_eq!(domain, expected_domain);
-
-        assert_eq!("test_macro 2", function_name);
-        assert_eq!("test log message 10 9 8 7", log_msg);
-    }
-}
diff --git a/src/rust/tor_util/ffi.rs b/src/rust/tor_util/ffi.rs
index 866d119740..a9a6cf895c 100644
--- a/src/rust/tor_util/ffi.rs
+++ b/src/rust/tor_util/ffi.rs
@@ -5,7 +5,7 @@
 //! called from C.
 //!
 
-use tor_log::*;
+use tor_log::{LogSeverity, LogDomain};
 
 /// Returns a short string to announce Rust support during startup.
 ///
diff --git a/src/rust/tor_util/lib.rs b/src/rust/tor_util/lib.rs
index 86785c344a..4d5cbb5680 100644
--- a/src/rust/tor_util/lib.rs
+++ b/src/rust/tor_util/lib.rs
@@ -7,6 +7,8 @@
 
 extern crate libc;
 extern crate tor_allocate;
+
+#[macro_use]
 extern crate tor_log;
 
 pub mod ffi;