test and fix logging callback (#194)

* test and fix logging callback

* `cstr` import

* test `xcoder-*-sys` crates on CI

* satisfy clippy

* gate xcoder tests behind target_os linux

I'm looking forward to <rust-lang/cargo#6179>;
making an entire crate empty or not based on platform is an awkward
workaround.

.github/workflows/rust.yml

@@ -68,7 +68,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --verbose -p xcoder-logan -- --test-threads=1
+          args: --verbose -p xcoder-logan -p xcoder-logan-310-sys -- --test-threads=1
   test_xcoder_logan_v2_compat:
     name: Test xcoder-logan v2-compat
     runs-on:
@@ -91,7 +91,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --verbose -p xcoder-logan --features v2-compat -- --test-threads=1
+          args: --verbose -p xcoder-logan -p xcoder-logan-259-sys --features v2-compat -- --test-threads=1
   test_xcoder_quadra:
     name: Test xcoder-quadra
     runs-on:
@@ -114,4 +114,4 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --verbose -p xcoder-quadra -- --test-threads=1
+          args: --verbose -p xcoder-quadra -p xcoder-quadra-sys -- --test-threads=1

xcoder/logging_shim.c

@@ -11,24 +11,19 @@
 #endif
 
 #ifdef LOGAN
-void rust_netint_logan_callback(int level, char* message);
-#else
-void rust_netint_callback(int level, char* message);
+#define rust_netint_callback rust_netint_logan_callback
+#define netint_log_callback netint_logan_log_callback
+#define ni_log_set_callback ni_logan_log_set_callback
 #endif
 
-void netint_log_callback(int level, const char* format, ...) {
-  va_list args;
-  va_start(args, format);
+void rust_netint_callback(int level, char* message);
+
+static void netint_log_callback(int level, const char* format, va_list args) {
   char buf[2048] = {0};
   size_t buf_len = sizeof(buf) / sizeof(buf[0]);
   int chars_written = vsnprintf(buf, buf_len, format, args);
-  va_end(args);
   if (chars_written > -1 && chars_written + 1 < ((int) buf_len)) {
-      #ifdef LOGAN
-      rust_netint_logan_callback(level, buf);
-      #else
       rust_netint_callback(level, buf);
-      #endif
   }
 }
 

xcoder/xcoder-logan/xcoder-logan-310-sys/Cargo.toml

@@ -11,3 +11,6 @@ log = "0.4"
 # We're very permissive here with bindgen due to https://github.com/rust-lang/cargo/issues/5237
 bindgen = "0.*"
 cc = "1.0"
+
+[dev-dependencies]
+cstr = "0.2.12"

xcoder/xcoder-logan/xcoder-logan-310-sys/tests/logging.rs

@@ -0,0 +1,52 @@
+#[cfg(target_os = "linux")]
+mod linux {
+    use cstr::cstr;
+    use std::sync::Mutex;
+
+    use xcoder_logan_310_sys as sys;
+
+    #[derive(Debug, Eq, PartialEq)]
+    struct LogEntry {
+        level: log::Level,
+        message: String,
+    }
+
+    #[derive(Default)]
+    struct MyLog(Mutex<Vec<LogEntry>>);
+
+    impl log::Log for MyLog {
+        fn enabled(&self, _metadata: &log::Metadata) -> bool {
+            true
+        }
+
+        fn log(&self, record: &log::Record) {
+            let mut l = self.0.lock().expect("not poisoned");
+            l.push(LogEntry {
+                level: record.level(),
+                message: record.args().to_string(),
+            })
+        }
+
+        fn flush(&self) {}
+    }
+
+    #[test]
+    fn blah() {
+        let log = Box::leak(Box::<MyLog>::default());
+        log::set_logger(log).expect("installing logger should succeed");
+        log::set_max_level(log::LevelFilter::Info);
+        unsafe {
+            sys::setup_rust_netint_logging();
+            sys::ni_logan_log(sys::ni_log_level_t_NI_LOG_ERROR, cstr!("foo %s %d").as_ptr(), cstr!("bar").as_ptr(), 1234u32);
+        }
+        let l = log.0.lock().expect("not poisoned");
+        assert_eq!(l.len(), 1);
+        assert_eq!(
+            &LogEntry {
+                level: log::Level::Error,
+                message: "foo bar 1234".to_owned(),
+            },
+            &l[0]
+        );
+    }
+}

xcoder/xcoder-quadra/xcoder-quadra-sys/Cargo.toml

@@ -11,3 +11,6 @@ log = "0.4"
 # We're very permissive here with bindgen due to https://github.com/rust-lang/cargo/issues/5237
 bindgen = "0.*"
 cc = "1.0"
+
+[dev-dependencies]
+cstr = "0.2.12"

xcoder/xcoder-quadra/xcoder-quadra-sys/tests/logging.rs

@@ -0,0 +1,52 @@
+#[cfg(target_os = "linux")]
+mod linux {
+    use cstr::cstr;
+    use std::sync::Mutex;
+
+    use xcoder_quadra_sys as sys;
+
+    #[derive(Debug, Eq, PartialEq)]
+    struct LogEntry {
+        level: log::Level,
+        message: String,
+    }
+
+    #[derive(Default)]
+    struct MyLog(Mutex<Vec<LogEntry>>);
+
+    impl log::Log for MyLog {
+        fn enabled(&self, _metadata: &log::Metadata) -> bool {
+            true
+        }
+
+        fn log(&self, record: &log::Record) {
+            let mut l = self.0.lock().expect("not poisoned");
+            l.push(LogEntry {
+                level: record.level(),
+                message: record.args().to_string(),
+            })
+        }
+
+        fn flush(&self) {}
+    }
+
+    #[test]
+    fn blah() {
+        let log = Box::leak(Box::<MyLog>::default());
+        log::set_logger(log).expect("installing logger should succeed");
+        log::set_max_level(log::LevelFilter::Info);
+        unsafe {
+            sys::setup_rust_netint_logging();
+            sys::ni_log(sys::ni_log_level_t_NI_LOG_ERROR, cstr!("foo %s %d").as_ptr(), cstr!("bar").as_ptr(), 1234u32);
+        }
+        let l = log.0.lock().expect("not poisoned");
+        assert_eq!(l.len(), 1);
+        assert_eq!(
+            &LogEntry {
+                level: log::Level::Error,
+                message: "foo bar 1234".to_owned(),
+            },
+            &l[0]
+        );
+    }
+}