fixups from code review

src/common/log.c

@@ -54,10 +54,10 @@
 
 /** Defining compile-time constants for Tor log levels (used by the Rust
  * log wrapper at src/rust/tor_log) */
-const int _LOG_WARN = LOG_WARN;
+const int LOG_WARN_ = LOG_WARN;
-const int _LOG_NOTICE = LOG_NOTICE;
+const int LOG_NOTICE_ = LOG_NOTICE;
-const log_domain_mask_t _LD_GENERAL = LD_GENERAL;
+const log_domain_mask_t LD_GENERAL_ = LD_GENERAL;
-const log_domain_mask_t _LD_NET = LD_NET;
+const log_domain_mask_t LD_NET_ = LD_NET;
 
 /** Information for a single logfile; only used in log.c */
 typedef struct logfile_t {

src/common/torlog.h

@@ -36,10 +36,10 @@
  *
  * C_RUST_COUPLED src/rust/tor_log LogSeverity, LogDomain
  */
-extern const int _LOG_WARN;
+extern const int LOG_WARN_;
-extern const int _LOG_NOTICE;
+extern const int LOG_NOTICE_;
-extern const log_domain_mask_t _LD_NET;
+extern const log_domain_mask_t LD_NET_;
-extern const log_domain_mask_t _LD_GENERAL;
+extern const log_domain_mask_t LD_GENERAL_;
 
 /** Debug-level severity: for hyper-verbose messages of no interest to
  * anybody but developers. */

src/or/main.c

@@ -132,7 +132,7 @@ void evdns_shutdown(int);
 #ifdef HAVE_RUST
 // helper function defined in Rust to output a log message indicating if tor is
 // running with Rust enabled. See src/rust/tor_util
-void rust_welcome_string(void);
+void rust_log_welcome_string(void);
 #endif
 
 /********* PROTOTYPES **********/
@@ -3283,7 +3283,7 @@ tor_init(int argc, char *argv[])
   }
 
 #ifdef HAVE_RUST
-  rust_welcome_string();
+  rust_log_welcome_string();
 #endif /* defined(HAVE_RUST) */
 
   if (network_init()<0) {

src/rust/protover/protover.rs

@@ -243,11 +243,11 @@ fn contains_only_supported_protocols(proto_entry: &str) -> bool {
         Err("Too many versions to expand") => {
             tor_log_msg!(
                 LogSeverity::Warn,
-                LogDomain::LdNet,
+                LogDomain::Net,
                 "get_versions",
-                "When expanding a protocol list from an authority, I
+                "When expanding a protocol list from an authority, I \
-                got too many protocols. This is possibly an attack or a bug,
+                got too many protocols. This is possibly an attack or a bug, \
-                unless the Tor network truly has expanded to support over {}
+                unless the Tor network truly has expanded to support over {} \
                 different subprotocol versions. The offending string was: {}",
                 MAX_PROTOCOLS_TO_EXPAND,
                 proto_entry

src/rust/tor_log/tor_log.rs

@@ -9,8 +9,8 @@
 /// general messages can use LdGeneral.
 #[derive(Eq, PartialEq)]
 pub enum LogDomain {
-    LdNet,
+    Net,
-    LdGeneral,
+    General,
 }
 
 /// The severity level at which to log messages.
@@ -98,33 +98,35 @@ pub mod log {
     /// C_RUST_COUPLED: src/common/log.c, log domain types
     extern "C" {
         #[no_mangle]
-        static _LOG_WARN: c_int;
+        static LOG_WARN_: c_int;
-        static _LOG_NOTICE: c_int;
+        static LOG_NOTICE_: c_int;
     }
 
     /// Domain log types. These mirror definitions in /src/common/torlog.h
     /// C_RUST_COUPLED: src/common/log.c, log severity types
     extern "C" {
         #[no_mangle]
-        static _LD_NET: u32;
+        static LD_NET_: u32;
-        static _LD_GENERAL: u32;
+        static LD_GENERAL_: u32;
     }
 
     /// Translate Rust defintions of log domain levels to C. This exposes a 1:1
     /// mapping between types.
+    #[inline]
     pub unsafe fn translate_domain(domain: LogDomain) -> u32 {
         match domain {
-            LogDomain::LdNet => _LD_NET,
+            LogDomain::Net => LD_NET_,
-            LogDomain::LdGeneral => _LD_GENERAL,
+            LogDomain::General => LD_GENERAL_,
         }
     }
 
     /// Translate Rust defintions of log severity levels to C. This exposes a
     /// 1:1 mapping between types.
+    #[inline]
     pub unsafe fn translate_severity(severity: LogSeverity) -> c_int {
         match severity {
-            LogSeverity::Warn => _LOG_WARN,
+            LogSeverity::Warn => LOG_WARN_,
-            LogSeverity::Notice => _LOG_NOTICE,
+            LogSeverity::Notice => LOG_NOTICE_,
         }
     }
 
@@ -194,7 +196,7 @@ mod test {
             fn test_macro() {
                 tor_log_msg!(
                     LogSeverity::Warn,
-                    LogDomain::LdNet,
+                    LogDomain::Net,
                     "test_macro",
                     "test log message {}",
                     "a",
@@ -210,11 +212,12 @@ mod test {
             assert_eq!("test log message a", *message);
         }
 
+        // test multiple inputs into the log message
         {
             fn test_macro() {
                 tor_log_msg!(
                     LogSeverity::Warn,
-                    LogDomain::LdNet,
+                    LogDomain::Net,
                     "next_test_macro",
                     "test log message {} {} {} {} {}",
                     1,
@@ -233,5 +236,37 @@ mod test {
             let message = unsafe { Box::from_raw(LAST_LOGGED_MESSAGE) };
             assert_eq!("test log message 1 2 3 4 5", *message);
         }
+
+        // test how a long log message will be formatted
+        {
+            fn test_macro() {
+                tor_log_msg!(
+                    LogSeverity::Warn,
+                    LogDomain::Net,
+                    "test_macro",
+                    "{}",
+                    "All the world's a stage, and all the men and women \
+                    merely players: they have their exits and their \
+                    entrances; and one man in his time plays many parts, his \
+                    acts being seven ages."
+                );
+            }
+
+            test_macro();
+
+            let expected_string = "All the world's a \
+                stage, and all the men \
+                and women merely players: \
+                they have their exits and \
+                their entrances; and one man \
+                in his time plays many parts, \
+                his acts being seven ages.";
+
+            let function = unsafe { Box::from_raw(LAST_LOGGED_FUNCTION) };
+            assert_eq!("test_macro", *function);
+
+            let message = unsafe { Box::from_raw(LAST_LOGGED_MESSAGE) };
+            assert_eq!(expected_string, *message);
+        }
     }
 }

src/rust/tor_util/ffi.rs

@@ -16,10 +16,10 @@ use tor_log::{LogSeverity, LogDomain};
 /// tor_free(rust_str);
 /// ```
 #[no_mangle]
-pub extern "C" fn rust_welcome_string() {
+pub extern "C" fn rust_log_welcome_string() {
     tor_log_msg!(
         LogSeverity::Notice,
-        LogDomain::LdGeneral,
+        LogDomain::General,
         "rust_welcome_string",
         "Tor is running with Rust integration. Please report \
         any bugs you encounter."