From 4a7e83f0af8a1e7fab3faa7e348dcd62216409e5 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 1 Aug 2015 17:21:41 +0200 Subject: [PATCH 01/17] Feature sqlite3_{log,trace,profile}. --- Cargo.toml | 1 + libsqlite3-sys/src/lib.rs | 2 + src/lib.rs | 4 +- src/trace_extension.rs | 98 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 src/trace_extension.rs diff --git a/Cargo.toml b/Cargo.toml index 5d05ed5..772b4cc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ name = "rusqlite" [features] load_extension = ["libsqlite3-sys/load_extension"] +trace_extension = [] [dependencies] time = "~0.1.0" diff --git a/libsqlite3-sys/src/lib.rs b/libsqlite3-sys/src/lib.rs index 5d37276..51f33c5 100644 --- a/libsqlite3-sys/src/lib.rs +++ b/libsqlite3-sys/src/lib.rs @@ -92,3 +92,5 @@ pub fn code_to_str(code: c_int) -> &'static str { _ => "Unknown error code", } } + +pub const SQLITE_CONFIG_LOG : c_int = 16; diff --git a/src/lib.rs b/src/lib.rs index f98780b..d96fdfd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,6 +50,7 @@ //! } //! } //! ``` +#![cfg_attr(test, feature(duration))] extern crate libc; extern crate libsqlite3_sys as ffi; #[macro_use] extern crate bitflags; @@ -79,6 +80,7 @@ pub use transaction::{SqliteTransactionBehavior, pub mod types; mod transaction; #[cfg(feature = "load_extension")] mod load_extension_guard; +#[cfg(feature = "trace_extension")] pub mod trace_extension; /// A typedef of the result returned by many methods. pub type SqliteResult = Result; @@ -664,7 +666,7 @@ impl<'conn> SqliteStatement<'conn> { } /// Executes the prepared statement and maps a function over the resulting - /// rows. + /// rows. /// /// Unlike the iterator produced by `query`, the returned iterator does not expose the possibility /// for accessing stale rows. diff --git a/src/trace_extension.rs b/src/trace_extension.rs new file mode 100644 index 0000000..cde1d75 --- /dev/null +++ b/src/trace_extension.rs @@ -0,0 +1,98 @@ +use libc::{c_char, c_int, c_void}; +use std::ffi::CString; +use std::ptr; + +use super::ffi; +use {SqliteError, SqliteResult, SqliteConnection}; + +pub type LogCallback = + Option; + +/// Set up the error logging callback +/// +/// cf [The Error And Warning Log](http://sqlite.org/errlog.html). +pub fn config_log(cb: LogCallback) -> SqliteResult<()> { + let rc = unsafe { + let p_arg: *mut c_void = ptr::null_mut(); + ffi::sqlite3_config(ffi::SQLITE_CONFIG_LOG, cb, p_arg) + }; + 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(); + unsafe { + ffi::sqlite3_log(err_code, msg.as_ptr()); + } +} + +pub type TraceCallback = + Option; +pub type ProfileCallback = + Option; +impl SqliteConnection { + /// Register or clear a callback function that can be used for tracing the execution of SQL statements. + /// 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) { + let c = self.db.borrow_mut(); + unsafe { ffi::sqlite3_trace(c.db(), x_trace, 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. + /// Setting a new profiler clears the old one. + pub fn profile(&mut self, x_profile: ProfileCallback) { + let c = self.db.borrow_mut(); + unsafe { ffi::sqlite3_profile(c.db(), x_profile, ptr::null_mut()); } + } +} + +#[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] + fn test_log() { + if true { // To avoid freezing tests + return + } + 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); + } + } + + #[test] + fn test_profile() { + let mut db = SqliteConnection::open_in_memory().unwrap(); + db.profile(Some(profile_callback)); + db.execute_batch("PRAGMA application_id = 1").unwrap(); + } +} \ No newline at end of file From ef254fdca00d2fbf79830e0f1abb3e6dcef9f702 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 1 Aug 2015 18:58:04 +0200 Subject: [PATCH 02/17] Rename feature to 'trace' --- Cargo.toml | 2 +- src/lib.rs | 2 +- src/{trace_extension.rs => trace.rs} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{trace_extension.rs => trace.rs} (100%) diff --git a/Cargo.toml b/Cargo.toml index 772b4cc..777d50c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ name = "rusqlite" [features] load_extension = ["libsqlite3-sys/load_extension"] -trace_extension = [] +trace = [] [dependencies] time = "~0.1.0" diff --git a/src/lib.rs b/src/lib.rs index d96fdfd..3c03fef 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,7 +80,7 @@ pub use transaction::{SqliteTransactionBehavior, pub mod types; mod transaction; #[cfg(feature = "load_extension")] mod load_extension_guard; -#[cfg(feature = "trace_extension")] pub mod trace_extension; +#[cfg(feature = "trace")] pub mod trace; /// A typedef of the result returned by many methods. pub type SqliteResult = Result; diff --git a/src/trace_extension.rs b/src/trace.rs similarity index 100% rename from src/trace_extension.rs rename to src/trace.rs From 9c415f9c9e433101564767603e0be7aa341c7343 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sun, 2 Aug 2015 12:16:01 +0200 Subject: [PATCH 03/17] Remove usage of unstable library feature 'duration' --- src/lib.rs | 1 - src/trace.rs | 10 ++++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3c03fef..4dfd068 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,7 +50,6 @@ //! } //! } //! ``` -#![cfg_attr(test, feature(duration))] extern crate libc; extern crate libsqlite3_sys as ffi; #[macro_use] extern crate bitflags; diff --git a/src/trace.rs b/src/trace.rs index cde1d75..de03742 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -80,19 +80,17 @@ mod test { 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; + extern "C" fn trace_callback(_: *mut ::libc::c_void, sql: *const ::libc::c_char) { 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(), "TRACE: {:?}", ::std::str::from_utf8(c_slice)); } } #[test] - fn test_profile() { + fn test_trace() { let mut db = SqliteConnection::open_in_memory().unwrap(); - db.profile(Some(profile_callback)); + db.trace(Some(trace_callback)); db.execute_batch("PRAGMA application_id = 1").unwrap(); } } \ No newline at end of file From 59a4c5593a92c61058bff40ed64edd2a07f5423f Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sat, 8 Aug 2015 09:57:07 +0200 Subject: [PATCH 04/17] Improve documentation. --- src/trace.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/trace.rs b/src/trace.rs index de03742..b3c9939 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -1,3 +1,4 @@ +//! Tracing and profiling functions. Error and warning log. use libc::{c_char, c_int, c_void}; use std::ffi::CString; use std::ptr; @@ -30,12 +31,16 @@ pub fn log(err_code: c_int, msg: &str) { } } +/// The trace callback function signature. pub type TraceCallback = Option; +/// The profile callback function signature. pub type ProfileCallback = Option; + impl SqliteConnection { /// Register or clear a callback function that can be used for tracing the execution of SQL statements. + /// /// 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. @@ -44,6 +49,7 @@ impl SqliteConnection { unsafe { ffi::sqlite3_trace(c.db(), x_trace, 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. /// Setting a new profiler clears the old one. pub fn profile(&mut self, x_profile: ProfileCallback) { From 50bfba1e1d5192aa78069bf88fdc57236f2afb38 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 11 Nov 2015 14:29:40 +0100 Subject: [PATCH 05/17] Ignore test_log. --- src/trace.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/trace.rs b/src/trace.rs index b3c9939..29473a2 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -75,11 +75,8 @@ mod test { } } - #[test] + #[test] #[ignore] // To avoid freezing tests fn test_log() { - if true { // To avoid freezing tests - return - } unsafe { ffi::sqlite3_shutdown() }; super::config_log(Some(log_callback)).unwrap(); //super::log(ffi::SQLITE_NOTICE, "message from rusqlite"); @@ -99,4 +96,4 @@ mod test { db.trace(Some(trace_callback)); db.execute_batch("PRAGMA application_id = 1").unwrap(); } -} \ No newline at end of file +} From a2327fb048c96297ecad41243b4b4d04dfb7367b Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 11 Nov 2015 15:00:39 +0100 Subject: [PATCH 06/17] Revert "Remove usage of unstable library feature 'duration'" This reverts commit 9c415f9c9e433101564767603e0be7aa341c7343. --- src/lib.rs | 1 + src/trace.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c639831..c6f82ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,6 +50,7 @@ //! } //! } //! ``` +#![cfg_attr(test, feature(duration))] extern crate libc; extern crate libsqlite3_sys as ffi; #[macro_use] extern crate bitflags; diff --git a/src/trace.rs b/src/trace.rs index 29473a2..b024f13 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -83,17 +83,19 @@ mod test { super::config_log(None).unwrap(); } - extern "C" fn trace_callback(_: *mut ::libc::c_void, sql: *const ::libc::c_char) { + 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 _ = writeln!(::std::io::stderr(), "TRACE: {:?}", ::std::str::from_utf8(c_slice)); + let d = Duration::from_millis(nanoseconds / 1_000_000); + let _ = writeln!(::std::io::stderr(), "PROFILE: {:?} ({})", ::std::str::from_utf8(c_slice), d); } } #[test] - fn test_trace() { + fn test_profile() { let mut db = SqliteConnection::open_in_memory().unwrap(); - db.trace(Some(trace_callback)); + db.profile(Some(profile_callback)); db.execute_batch("PRAGMA application_id = 1").unwrap(); } } From 20c1213482ddd1bf5c19ebc4994b103f76d991f9 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 11 Nov 2015 15:03:07 +0100 Subject: [PATCH 07/17] Remove feature duration. --- src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index c6f82ce..c639831 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,7 +50,6 @@ //! } //! } //! ``` -#![cfg_attr(test, feature(duration))] extern crate libc; extern crate libsqlite3_sys as ffi; #[macro_use] extern crate bitflags; From 0729e195c371fbab0898fdab6327682df9198169 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 11 Nov 2015 15:14:31 +0100 Subject: [PATCH 08/17] Use debug_struct for formatting. --- src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3b1c49e..b46232f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -444,7 +444,9 @@ impl SqliteConnection { impl fmt::Debug for SqliteConnection { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "SqliteConnection( path: {:?} )", &self.path) + f.debug_struct("SqliteConnection") + .field("path", &self.path) + .finish() } } @@ -780,7 +782,11 @@ impl<'conn> fmt::Debug for SqliteStatement<'conn> { let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); str::from_utf8(c_slice) }; - write!(f, "SqliteStatement( conn: {:?}, stmt: {:?}, sql: {:?} )", self.conn, self.stmt, sql) + f.debug_struct("SqliteStatement") + .field("conn", self.conn) + .field("stmt", &self.stmt) + .field("sql", &sql) + .finish() } } From dbfa6ca31f0c839dcc3afd33861d909265fd6f59 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 30 Nov 2015 15:29:50 -0500 Subject: [PATCH 09/17] 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 | 5 ++++ src/trace.rs | 65 ++++++++++++++++++++++++--------------------- tests/config_log.rs | 36 +++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 30 deletions(-) create mode 100644 tests/config_log.rs diff --git a/Cargo.toml b/Cargo.toml index 673044c..0078216 100644 --- a/Cargo.toml +++ b/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 diff --git a/src/trace.rs b/src/trace.rs index b024f13..33b5997 100644 --- a/src/trace.rs +++ b/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 = - Option; - -/// Set up the error logging callback +/// Set up the process-wide SQLite error logging callback. +/// This function is marked unsafe for two reasons: +/// +/// * 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<()> { - let rc = unsafe { - let p_arg: *mut c_void = ptr::null_mut(); - ffi::sqlite3_config(ffi::SQLITE_CONFIG_LOG, cb, p_arg) +pub unsafe fn config_log(callback: Option) -> SqliteResult<()> { + extern "C" fn log_callback(p_arg: *mut c_void, err: c_int, msg: *const c_char) { + let c_slice = unsafe { CStr::from_ptr(msg).to_bytes() }; + 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); } } diff --git a/tests/config_log.rs b/tests/config_log.rs new file mode 100644 index 0000000..e0167bd --- /dev/null +++ b/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> = 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() {} From ace5b1ebdc2e9d31cda1a7bbd928fc09b12f8722 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 30 Nov 2015 16:33:47 -0500 Subject: [PATCH 10/17] 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 { } /// 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, path: Option, 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; /// The profile callback function signature. pub type ProfileCallback = Option; @@ -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) { + 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> = 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 { From 8e31a64dde26a7c404ea75e4d8a603b32c37ce16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krasnoborski?= Date: Tue, 1 Dec 2015 06:18:44 +0000 Subject: [PATCH 11/17] Update Cargo.tomls to libc ~0.2 --- Cargo.toml | 2 +- libsqlite3-sys/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 68e7347..1efbe76 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ load_extension = ["libsqlite3-sys/load_extension"] [dependencies] time = "~0.1.0" bitflags = "~0.1" -libc = "~0.1" +libc = "~0.2" [dev-dependencies] tempdir = "~0.3.4" diff --git a/libsqlite3-sys/Cargo.toml b/libsqlite3-sys/Cargo.toml index ecb20b8..cafa632 100644 --- a/libsqlite3-sys/Cargo.toml +++ b/libsqlite3-sys/Cargo.toml @@ -15,4 +15,4 @@ load_extension = [] pkg-config = "~0.3" [dependencies] -libc = "~0.1" +libc = "~0.2" From e6fef5107dd599bb4dd8dcec12fbb1889bf3334f Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 1 Dec 2015 10:34:18 -0500 Subject: [PATCH 12/17] Change profile() to take a Rust fn instead of an extern "C" fn. --- src/trace.rs | 51 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/src/trace.rs b/src/trace.rs index 1777057..4296ddc 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -5,6 +5,7 @@ use std::ffi::{CStr, CString}; use std::mem; use std::ptr; use std::str; +use std::time::Duration; use super::ffi; use {SqliteError, SqliteResult, SqliteConnection}; @@ -56,10 +57,6 @@ pub fn log(err_code: c_int, msg: &str) { } } -/// The profile callback function signature. -pub type ProfileCallback = - Option; - impl SqliteConnection { /// Register or clear a callback function that can be used for tracing the execution of SQL statements. /// @@ -86,16 +83,31 @@ impl SqliteConnection { /// /// There can only be a single profiler defined for each database connection. /// Setting a new profiler clears the old one. - pub fn profile(&mut self, x_profile: ProfileCallback) { + pub fn profile(&mut self, profile_fn: Option) { + extern "C" fn profile_callback(p_arg: *mut c_void, z_sql: *const c_char, nanoseconds: u64) { + let profile_fn: fn(&str, Duration) = 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) { + const NANOS_PER_SEC: u64 = 1_000_000_000; + + let duration = Duration::new(nanoseconds / NANOS_PER_SEC, + (nanoseconds % NANOS_PER_SEC) as u32); + profile_fn(s, duration); + } + } + let c = self.db.borrow_mut(); - unsafe { ffi::sqlite3_profile(c.db(), x_profile, ptr::null_mut()); } + match profile_fn { + Some(f) => unsafe { ffi::sqlite3_profile(c.db(), Some(profile_callback), mem::transmute(f)) }, + None => unsafe { ffi::sqlite3_profile(c.db(), None, ptr::null_mut()) }, + }; } } #[cfg(test)] mod test { - use std::io::Write; use std::sync::Mutex; + use std::time::Duration; use SqliteConnection; @@ -127,19 +139,24 @@ mod test { 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 { - 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); - } - } - #[test] fn test_profile() { + lazy_static! { + static ref PROFILED: Mutex> = Mutex::new(Vec::new()); + } + fn profiler(s: &str, d: Duration) { + let mut profiled = PROFILED.lock().unwrap(); + profiled.push((s.to_owned(), d)); + } + let mut db = SqliteConnection::open_in_memory().unwrap(); - db.profile(Some(profile_callback)); + db.profile(Some(profiler)); db.execute_batch("PRAGMA application_id = 1").unwrap(); + db.profile(None); + db.execute_batch("PRAGMA application_id = 2").unwrap(); + + let profiled = PROFILED.lock().unwrap(); + assert_eq!(profiled.len(), 1); + assert_eq!(profiled[0].0, "PRAGMA application_id = 1"); } } From cba64a7deee3a372612410b3634dc9e822b91e85 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 1 Dec 2015 10:37:01 -0500 Subject: [PATCH 13/17] Add trace feature to Changelog --- Changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Changelog.md b/Changelog.md index 2987801..5a1eaec 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +# Version UPCOMING (TBD) + +* Adds `trace` feature that allows the use of SQLite's logging, tracing, and profiling hooks. + # Version 0.4.0 (2015-11-03) * Adds `Sized` bound to `FromSql` trait as required by RFC 1214. From 86165725de1b7a0b820f6d3a197f44aace22d561 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 1 Dec 2015 10:43:55 -0500 Subject: [PATCH 14/17] Add krdln to CONTRIBUTORS --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 24b0e7e..eca404b 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -8,3 +8,4 @@ rusqlite contributors (sorted alphabetically) * [Huon Wilson](https://github.com/huonw) * [Patrick Fernie](https://github.com/pfernie) * [Steve Klabnik](https://github.com/steveklabnik) +* [krdln](https://github.com/krdln) From 635616842cea1171dea7a0d4a92fa8d64dec50bd Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 1 Dec 2015 10:55:01 -0500 Subject: [PATCH 15/17] Remove 'static requirement on output of closure given to query_map and query_and_then. The 'static bound was there to prevent callers from being able to save off the `SqliteRow` handles passed into the closure. This PR changes the closure to take `&SqliteRow`s instead, which provides the same feature without restricting the output of the closure. --- Changelog.md | 6 ++++++ README.md | 2 +- src/lib.rs | 20 ++++++++------------ 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/Changelog.md b/Changelog.md index 2987801..0af134a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,9 @@ +# Version UPCOMDING (TBD) + +* Slight change to the closure types passed to `query_map` and `query_and_then`: + * Remove the `'static` requirement on the closure's output type. + * Give the closure a `&SqliteRow` instead of a `SqliteRow`. + # Version 0.4.0 (2015-11-03) * Adds `Sized` bound to `FromSql` trait as required by RFC 1214. diff --git a/README.md b/README.md index 990af8d..1bc6742 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ There are other, less obvious things that may result in a panic as well, such as `collect()` on a `SqliteRows` and then trying to use the collected rows. Strongly consider using the method `query_map()` instead, if you can. -`query_map()` returns an iterator over rows-mapped-to-some-`'static`-type. This +`query_map()` returns an iterator over rows-mapped-to-some-type. This iterator does not have any of the above issues with panics due to attempting to access stale rows. diff --git a/src/lib.rs b/src/lib.rs index b46232f..cae3c59 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -712,8 +712,7 @@ impl<'conn> SqliteStatement<'conn> { /// for accessing stale rows. pub fn query_map<'a, T, F>(&'a mut self, params: &[&ToSql], f: F) -> SqliteResult> - where T: 'static, - F: FnMut(SqliteRow) -> T { + where F: FnMut(&SqliteRow) -> T { let row_iter = try!(self.query(params)); Ok(MappedRows{ @@ -730,9 +729,8 @@ impl<'conn> SqliteStatement<'conn> { /// for accessing stale rows. pub fn query_and_then<'a, T, E, F>(&'a mut self, params: &[&ToSql], f: F) -> SqliteResult> - where T: 'static, - E: convert::From, - F: FnMut(SqliteRow) -> Result { + where E: convert::From, + F: FnMut(&SqliteRow) -> Result { let row_iter = try!(self.query(params)); Ok(AndThenRows{ @@ -804,12 +802,11 @@ pub struct MappedRows<'stmt, F> { } impl<'stmt, T, F> Iterator for MappedRows<'stmt, F> - where T: 'static, - F: FnMut(SqliteRow) -> T { + where F: FnMut(&SqliteRow) -> T { type Item = SqliteResult; fn next(&mut self) -> Option> { - self.rows.next().map(|row_result| row_result.map(|row| (self.map)(row))) + self.rows.next().map(|row_result| row_result.map(|row| (self.map)(&row))) } } @@ -821,15 +818,14 @@ pub struct AndThenRows<'stmt, F> { } impl<'stmt, T, E, F> Iterator for AndThenRows<'stmt, F> - where T: 'static, - E: convert::From, - F: FnMut(SqliteRow) -> Result { + where E: convert::From, + F: FnMut(&SqliteRow) -> Result { type Item = Result; fn next(&mut self) -> Option { self.rows.next().map(|row_result| row_result .map_err(E::from) - .and_then(|row| (self.map)(row))) + .and_then(|row| (self.map)(&row))) } } From 1af3fcd0539a7699ae13d7707d62d6d2f5585491 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 1 Dec 2015 11:47:55 -0500 Subject: [PATCH 16/17] Fix error messages when failing to convert paths and strings to C-compatible versions --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index cae3c59..364d6d9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -128,14 +128,14 @@ impl SqliteError { fn str_to_cstring(s: &str) -> SqliteResult { CString::new(s).map_err(|_| SqliteError{ code: ffi::SQLITE_MISUSE, - message: "Could not convert path to C-combatible string".to_string() + message: format!("Could not convert string {} to C-combatible string", s), }) } fn path_to_cstring(p: &Path) -> SqliteResult { let s = try!(p.to_str().ok_or(SqliteError{ code: ffi::SQLITE_MISUSE, - message: "Could not convert path to UTF-8 string".to_string() + message: format!("Could not convert path {} to UTF-8 string", p.to_string_lossy()), })); str_to_cstring(s) } From 3d654aeed17c22c084764bef48a3d5ad7cd35730 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 1 Dec 2015 12:05:29 -0500 Subject: [PATCH 17/17] Add more documentation for failure modes of functions that return s --- CONTRIBUTORS.md | 1 + Changelog.md | 1 + src/lib.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index eca404b..61121a5 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -9,3 +9,4 @@ rusqlite contributors (sorted alphabetically) * [Patrick Fernie](https://github.com/pfernie) * [Steve Klabnik](https://github.com/steveklabnik) * [krdln](https://github.com/krdln) +* [Ben Striegel](https://github.com/bstrie) diff --git a/Changelog.md b/Changelog.md index 0af134a..f5e1775 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ * Slight change to the closure types passed to `query_map` and `query_and_then`: * Remove the `'static` requirement on the closure's output type. * Give the closure a `&SqliteRow` instead of a `SqliteRow`. +* Add more documentation for failure modes of functions that return `SqliteResult`s. # Version 0.4.0 (2015-11-03) diff --git a/src/lib.rs b/src/lib.rs index 364d6d9..1e77854 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -162,12 +162,21 @@ impl SqliteConnection { /// /// `SqliteConnection::open(path)` is equivalent to `SqliteConnection::open_with_flags(path, /// SQLITE_OPEN_READ_WRITE | SQLITE_OPEN_CREATE)`. + /// + /// # Failure + /// + /// Will return `Err` if `path` cannot be converted to a C-compatible string or if the + /// underlying SQLite open call fails. pub fn open>(path: P) -> SqliteResult { let flags = Default::default(); SqliteConnection::open_with_flags(path, flags) } /// Open a new connection to an in-memory SQLite database. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite open call fails. pub fn open_in_memory() -> SqliteResult { let flags = Default::default(); SqliteConnection::open_in_memory_with_flags(flags) @@ -177,6 +186,11 @@ impl SqliteConnection { /// /// Database Connection](http://www.sqlite.org/c3ref/open.html) for a description of valid /// flag combinations. + /// + /// # Failure + /// + /// Will return `Err` if `path` cannot be converted to a C-compatible string or if the + /// underlying SQLite open call fails. pub fn open_with_flags>(path: P, flags: SqliteOpenFlags) -> SqliteResult { let c_path = try!(path_to_cstring(path.as_ref())); @@ -189,6 +203,10 @@ impl SqliteConnection { /// /// Database Connection](http://www.sqlite.org/c3ref/open.html) for a description of valid /// flag combinations. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite open call fails. pub fn open_in_memory_with_flags(flags: SqliteOpenFlags) -> SqliteResult { let c_memory = try!(str_to_cstring(":memory:")); InnerSqliteConnection::open_with_flags(&c_memory, flags).map(|db| { @@ -216,6 +234,10 @@ impl SqliteConnection { /// tx.commit() /// } /// ``` + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. pub fn transaction<'a>(&'a self) -> SqliteResult> { SqliteTransaction::new(self, SqliteTransactionDeferred) } @@ -223,6 +245,10 @@ impl SqliteConnection { /// Begin a new transaction with a specified behavior. /// /// See `transaction`. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. pub fn transaction_with_behavior<'a>(&'a self, behavior: SqliteTransactionBehavior) -> SqliteResult> { SqliteTransaction::new(self, behavior) @@ -243,6 +269,11 @@ impl SqliteConnection { /// COMMIT;") /// } /// ``` + /// + /// # Failure + /// + /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the + /// underlying SQLite call fails. pub fn execute_batch(&self, sql: &str) -> SqliteResult<()> { self.db.borrow_mut().execute_batch(sql) } @@ -263,6 +294,11 @@ impl SqliteConnection { /// } /// } /// ``` + /// + /// # Failure + /// + /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the + /// underlying SQLite call fails. pub fn execute(&self, sql: &str, params: &[&ToSql]) -> SqliteResult { self.prepare(sql).and_then(|mut stmt| stmt.execute(params)) } @@ -289,6 +325,11 @@ impl SqliteConnection { /// ``` /// /// If the query returns more than one row, all rows except the first are ignored. + /// + /// # Failure + /// + /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the + /// underlying SQLite call fails. pub fn query_row(&self, sql: &str, params: &[&ToSql], f: F) -> SqliteResult where F: FnOnce(SqliteRow) -> T { let mut stmt = try!(self.prepare(sql)); @@ -319,6 +360,11 @@ impl SqliteConnection { /// ``` /// /// If the query returns more than one row, all rows except the first are ignored. + /// + /// # Failure + /// + /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the + /// underlying SQLite call fails. pub fn query_row_and_then(&self, sql: &str, params: &[&ToSql], f: F) -> Result where F: FnOnce(SqliteRow) -> Result, E: convert::From { @@ -371,6 +417,11 @@ impl SqliteConnection { /// Ok(()) /// } /// ``` + /// + /// # Failure + /// + /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the + /// underlying SQLite call fails. pub fn prepare<'a>(&'a self, sql: &str) -> SqliteResult> { self.db.borrow_mut().prepare(self, sql) } @@ -379,6 +430,10 @@ impl SqliteConnection { /// /// This is functionally equivalent to the `Drop` implementation for `SqliteConnection` except /// that it returns any error encountered to the caller. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. pub fn close(self) -> SqliteResult<()> { let mut db = self.db.borrow_mut(); db.close() @@ -398,6 +453,10 @@ impl SqliteConnection { /// conn.load_extension_disable() /// } /// ``` + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. #[cfg(feature = "load_extension")] pub fn load_extension_enable(&self) -> SqliteResult<()> { self.db.borrow_mut().enable_load_extension(1) @@ -406,6 +465,10 @@ impl SqliteConnection { /// Disable loading of SQLite extensions. /// /// See `load_extension_enable` for an example. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. #[cfg(feature = "load_extension")] pub fn load_extension_disable(&self) -> SqliteResult<()> { self.db.borrow_mut().enable_load_extension(0) @@ -428,6 +491,10 @@ impl SqliteConnection { /// /// conn.load_extension("my_sqlite_extension", None) /// } + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. #[cfg(feature = "load_extension")] pub fn load_extension>(&self, dylib_path: P, entry_point: Option<&str>) -> SqliteResult<()> { self.db.borrow_mut().load_extension(dylib_path, entry_point) @@ -653,6 +720,11 @@ impl<'conn> SqliteStatement<'conn> { /// Ok(()) /// } /// ``` + /// + /// # Failure + /// + /// Will return `Err` if binding parameters fails, the executed statement returns rows (in + /// which case `query` should be used instead), or the underling SQLite call fails. pub fn execute(&mut self, params: &[&ToSql]) -> SqliteResult { unsafe { try!(self.bind_parameters(params)); @@ -694,6 +766,10 @@ impl<'conn> SqliteStatement<'conn> { /// Ok(names) /// } /// ``` + /// + /// # Failure + /// + /// Will return `Err` if binding parameters fails. pub fn query<'a>(&'a mut self, params: &[&ToSql]) -> SqliteResult> { self.reset_if_needed(); @@ -710,6 +786,10 @@ impl<'conn> SqliteStatement<'conn> { /// /// Unlike the iterator produced by `query`, the returned iterator does not expose the possibility /// for accessing stale rows. + /// + /// # Failure + /// + /// Will return `Err` if binding parameters fails. pub fn query_map<'a, T, F>(&'a mut self, params: &[&ToSql], f: F) -> SqliteResult> where F: FnMut(&SqliteRow) -> T { @@ -727,6 +807,10 @@ impl<'conn> SqliteStatement<'conn> { /// /// Unlike the iterator produced by `query`, the returned iterator does not expose the possibility /// for accessing stale rows. + /// + /// # Failure + /// + /// Will return `Err` if binding parameters fails. pub fn query_and_then<'a, T, E, F>(&'a mut self, params: &[&ToSql], f: F) -> SqliteResult> where E: convert::From, @@ -743,6 +827,10 @@ impl<'conn> SqliteStatement<'conn> { /// /// Functionally equivalent to the `Drop` implementation, but allows callers to see any errors /// that occur. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. pub fn finalize(mut self) -> SqliteResult<()> { self.finalize_() }