From ace5b1ebdc2e9d31cda1a7bbd928fc09b12f8722 Mon Sep 17 00:00:00 2001
From: John Gallagher <jgallagher@bignerdranch.com>
Date: Mon, 30 Nov 2015 16:33:47 -0500
Subject: [PATCH] Change trace() to take a Rust fn instead of an extern "C" fn.

---
 src/lib.rs   | 10 +---------
 src/trace.rs | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index d11b98a..cd24ad9 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -53,6 +53,7 @@
 extern crate libc;
 extern crate libsqlite3_sys as ffi;
 #[macro_use] extern crate bitflags;
+#[cfg(test)] #[macro_use] extern crate lazy_static;
 
 use std::default::Default;
 use std::convert;
@@ -142,15 +143,6 @@ fn path_to_cstring(p: &Path) -> SqliteResult<CString> {
 }
 
 /// A connection to a SQLite database.
-///
-/// ## Warning
-///
-/// Note that despite the fact that most `SqliteConnection` methods take an immutable reference to
-/// `self`, `SqliteConnection` is NOT threadsafe, and using it from multiple threads may result in
-/// runtime panics or data races. The SQLite connection handle has at least two pieces of internal
-/// state (the last insertion ID and the last error message) that rusqlite uses, but wrapping these
-/// APIs in a safe way from Rust would be too restrictive (for example, you would not be able to
-/// prepare multiple statements at the same time).
 pub struct SqliteConnection {
     db: RefCell<InnerSqliteConnection>,
     path: Option<PathBuf>,
diff --git a/src/trace.rs b/src/trace.rs
index 33b5997..1777057 100644
--- a/src/trace.rs
+++ b/src/trace.rs
@@ -2,6 +2,7 @@
 
 use libc::{c_char, c_int, c_void};
 use std::ffi::{CStr, CString};
+use std::mem;
 use std::ptr;
 use std::str;
 
@@ -55,9 +56,6 @@ pub fn log(err_code: c_int, msg: &str) {
     }
 }
 
-/// The trace callback function signature.
-pub type TraceCallback =
-    Option<extern "C" fn (p_arg: *mut c_void, z_sql: *const c_char)>;
 /// The profile callback function signature.
 pub type ProfileCallback =
     Option<extern "C" fn (p_arg: *mut c_void, z_sql: *const c_char, nanoseconds: u64)>;
@@ -68,10 +66,22 @@ impl SqliteConnection {
     /// Prepared statement placeholders are replaced/logged with their assigned values.
     /// There can only be a single tracer defined for each database connection.
     /// Setting a new tracer clears the old one.
-    pub fn trace(&mut self, x_trace: TraceCallback) {
+    pub fn trace(&mut self, trace_fn: Option<fn(&str)>) {
+        extern "C" fn trace_callback (p_arg: *mut c_void, z_sql: *const c_char) {
+            let trace_fn: fn(&str) = unsafe { mem::transmute(p_arg) };
+            let c_slice = unsafe { CStr::from_ptr(z_sql).to_bytes() };
+            if let Ok(s) = str::from_utf8(c_slice) {
+                trace_fn(s);
+            }
+        }
+
         let c = self.db.borrow_mut();
-        unsafe { ffi::sqlite3_trace(c.db(), x_trace, ptr::null_mut()); }
+        match trace_fn {
+            Some(f) => unsafe { ffi::sqlite3_trace(c.db(), Some(trace_callback), mem::transmute(f)); },
+            None    => unsafe { ffi::sqlite3_trace(c.db(), None, ptr::null_mut()); },
+        }
     }
+
     /// Register or clear a callback function that can be used for profiling the execution of SQL statements.
     ///
     /// There can only be a single profiler defined for each database connection.
@@ -85,9 +95,38 @@ impl SqliteConnection {
 #[cfg(test)]
 mod test {
     use std::io::Write;
+    use std::sync::Mutex;
 
     use SqliteConnection;
 
+    #[test]
+    fn test_trace() {
+        lazy_static! {
+            static ref TRACED_STMTS: Mutex<Vec<String>> = Mutex::new(Vec::new());
+        }
+        fn tracer(s: &str) {
+            let mut traced_stmts = TRACED_STMTS.lock().unwrap();
+            traced_stmts.push(s.to_owned());
+        }
+
+        let mut db = SqliteConnection::open_in_memory().unwrap();
+        db.trace(Some(tracer));
+        {
+            let _ = db.query_row("SELECT ?", &[&1i32], |_| {});
+            let _ = db.query_row("SELECT ?", &[&"hello"], |_| {});
+        }
+        db.trace(None);
+        {
+            let _ = db.query_row("SELECT ?", &[&2i32], |_| {});
+            let _ = db.query_row("SELECT ?", &[&"goodbye"], |_| {});
+        }
+
+        let traced_stmts = TRACED_STMTS.lock().unwrap();
+        assert_eq!(traced_stmts.len(), 2);
+        assert_eq!(traced_stmts[0], "SELECT 1");
+        assert_eq!(traced_stmts[1], "SELECT 'hello'");
+    }
+
     extern "C" fn profile_callback(_: *mut ::libc::c_void, sql: *const ::libc::c_char, nanoseconds: u64) {
         use std::time::Duration;
         unsafe {