Change config_log to take a Rust fn instead of an extern "C" fn.

Moves the unit test for config_log out of #[ignore] and into its own
test file since it affects the entire process.

Cargo.toml

@@ -23,7 +23,12 @@ libc = "~0.1"
 
 [dev-dependencies]
 tempdir = "~0.3.4"
+lazy_static = "~0.1"
 
 [dependencies.libsqlite3-sys]
 path = "libsqlite3-sys"
 version = "0.2.0"
+
+[[test]]
+name = "config_log"
+harness = false

src/trace.rs

@@ -1,31 +1,55 @@
 //! Tracing and profiling functions. Error and warning log.
+
 use libc::{c_char, c_int, c_void};
-use std::ffi::CString;
+use std::ffi::{CStr, CString};
 use std::ptr;
+use std::str;
 
 use super::ffi;
 use {SqliteError, SqliteResult, SqliteConnection};
 
-pub type LogCallback =
+/// Set up the process-wide SQLite error logging callback.
-    Option<extern "C" fn (udp: *mut c_void, err: c_int, msg: *const c_char)>;
+/// This function is marked unsafe for two reasons:
-
+///
-/// Set up the error logging callback
+/// * The function is not threadsafe. No other SQLite calls may be made while
+///   `config_log` is running, and multiple threads may not call `config_log`
+///   simultaneously.
+/// * The provided `callback` itself function has two requirements:
+///     * It must not invoke any SQLite calls.
+///     * It must be threadsafe if SQLite is used in a multithreaded way.
 ///
 /// cf [The Error And Warning Log](http://sqlite.org/errlog.html).
-pub fn config_log(cb: LogCallback) -> SqliteResult<()> {
+pub unsafe fn config_log(callback: Option<fn(c_int, &str)>) -> SqliteResult<()> {
-    let rc = unsafe {
+    extern "C" fn log_callback(p_arg: *mut c_void, err: c_int, msg: *const c_char) {
-        let p_arg: *mut c_void = ptr::null_mut();
+        let c_slice = unsafe { CStr::from_ptr(msg).to_bytes() };
-        ffi::sqlite3_config(ffi::SQLITE_CONFIG_LOG, cb, p_arg)
+        let callback: fn(c_int, &str) = unsafe { mem::transmute(p_arg) };
+
+        if let Ok(s) = str::from_utf8(c_slice) {
+            callback(err, s);
+        }
+    }
+
+    let rc = match callback {
+        Some(f) => {
+            let p_arg: *mut c_void = mem::transmute(f);
+            ffi::sqlite3_config(ffi::SQLITE_CONFIG_LOG, Some(log_callback), p_arg)
+        },
+        None => {
+            let nullptr: *mut c_void = ptr::null_mut();
+            ffi::sqlite3_config(ffi::SQLITE_CONFIG_LOG, nullptr, nullptr)
+        }
     };
+
     if rc != ffi::SQLITE_OK {
         return Err(SqliteError{ code: rc, message: "sqlite3_config(SQLITE_CONFIG_LOG, ...)".to_string() });
     }
+
     Ok(())
 }
 
 /// Write a message into the error log established by `config_log`.
 pub fn log(err_code: c_int, msg: &str) {
-    let msg = CString::new(msg).unwrap();
+    let msg = CString::new(msg).expect("SQLite log messages cannot contain embedded zeroes");
     unsafe {
         ffi::sqlite3_log(err_code, msg.as_ptr());
     }
@@ -60,35 +84,16 @@ impl SqliteConnection {
 
 #[cfg(test)]
 mod test {
-    use libc::{c_char, c_int, c_void};
-    use std::ffi::CStr;
     use std::io::Write;
-    use std::str;
 
-    use ffi;
     use SqliteConnection;
 
-    extern "C" fn log_callback(_: *mut c_void, err: c_int, msg: *const c_char) {
-        unsafe {
-            let c_slice = CStr::from_ptr(msg).to_bytes();
-            let _ = writeln!(::std::io::stderr(), "{}: {:?}", err, str::from_utf8(c_slice));
-        }
-    }
-
-    #[test] #[ignore] // To avoid freezing tests
-    fn test_log() {
-        unsafe { ffi::sqlite3_shutdown() };
-        super::config_log(Some(log_callback)).unwrap();
-        //super::log(ffi::SQLITE_NOTICE, "message from rusqlite");
-        super::config_log(None).unwrap();
-    }
-
     extern "C" fn profile_callback(_: *mut ::libc::c_void, sql: *const ::libc::c_char, nanoseconds: u64) {
         use std::time::Duration;
         unsafe {
             let c_slice = ::std::ffi::CStr::from_ptr(sql).to_bytes();
             let d = Duration::from_millis(nanoseconds / 1_000_000);
-            let _ = writeln!(::std::io::stderr(), "PROFILE: {:?} ({})", ::std::str::from_utf8(c_slice), d);
+            let _ = writeln!(::std::io::stderr(), "PROFILE: {:?} ({:?})", ::std::str::from_utf8(c_slice), d);
         }
     }
 

tests/config_log.rs

@@ -0,0 +1,36 @@
+//! This file contains unit tests for rusqlite::trace::config_log. This function affects
+//! SQLite process-wide and so is not safe to run as a normal #[test] in the library.
+
+#[macro_use] extern crate lazy_static;
+extern crate libc;
+extern crate rusqlite;
+
+#[cfg(feature = "trace")]
+fn main() {
+    use libc::c_int;
+    use std::sync::Mutex;
+
+    lazy_static! {
+        static ref LOGS_RECEIVED: Mutex<Vec<(c_int, String)>> = Mutex::new(Vec::new());
+    }
+
+    fn log_handler(err: c_int, message: &str) {
+        let mut logs_received = LOGS_RECEIVED.lock().unwrap();
+        logs_received.push((err, message.to_owned()));
+    }
+
+    use rusqlite::trace;
+
+    unsafe { trace::config_log(Some(log_handler)) }.unwrap();
+    trace::log(10, "First message from rusqlite");
+    unsafe { trace::config_log(None) }.unwrap();
+    trace::log(11, "Second message from rusqlite");
+
+    let logs_received = LOGS_RECEIVED.lock().unwrap();
+    assert_eq!(logs_received.len(), 1);
+    assert_eq!(logs_received[0].0, 10);
+    assert_eq!(logs_received[0].1, "First message from rusqlite");
+}
+
+#[cfg(not(feature = "trace"))]
+fn main() {}