From 53c99f940e3c66e1ec0e7d54456925db1b4c5f07 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Mon, 6 Apr 2020 19:38:33 -0700 Subject: [PATCH] Fix up conventions around unsafe in internal functions --- src/backup.rs | 8 ++++---- src/error.rs | 4 ++-- src/hooks.rs | 16 ++++++++-------- src/inner_connection.rs | 32 +++++++++++++++++--------------- src/raw_statement.rs | 6 +++--- src/statement.rs | 4 ++-- src/unlock_notify.rs | 22 ++++++++++------------ 7 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/backup.rs b/src/backup.rs index 1018ab6..e095146 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -77,8 +77,8 @@ impl Connection { match r { Done => Ok(()), - Busy => Err(error_from_handle(ptr::null_mut(), ffi::SQLITE_BUSY)), - Locked => Err(error_from_handle(ptr::null_mut(), ffi::SQLITE_LOCKED)), + Busy => Err(unsafe { error_from_handle(ptr::null_mut(), ffi::SQLITE_BUSY) }), + Locked => Err(unsafe { error_from_handle(ptr::null_mut(), ffi::SQLITE_LOCKED) }), More => unreachable!(), } } @@ -123,8 +123,8 @@ impl Connection { match r { Done => Ok(()), - Busy => Err(error_from_handle(ptr::null_mut(), ffi::SQLITE_BUSY)), - Locked => Err(error_from_handle(ptr::null_mut(), ffi::SQLITE_LOCKED)), + Busy => Err(unsafe { error_from_handle(ptr::null_mut(), ffi::SQLITE_BUSY) }), + Locked => Err(unsafe { error_from_handle(ptr::null_mut(), ffi::SQLITE_LOCKED) }), More => unreachable!(), } } diff --git a/src/error.rs b/src/error.rs index 50ff32d..34d2ddc 100644 --- a/src/error.rs +++ b/src/error.rs @@ -357,11 +357,11 @@ pub fn error_from_sqlite_code(code: c_int, message: Option) -> Error { Error::SqliteFailure(ffi::Error::new(code), message) } -pub fn error_from_handle(db: *mut ffi::sqlite3, code: c_int) -> Error { +pub unsafe fn error_from_handle(db: *mut ffi::sqlite3, code: c_int) -> Error { let message = if db.is_null() { None } else { - Some(unsafe { errmsg_to_string(ffi::sqlite3_errmsg(db)) }) + Some(errmsg_to_string(ffi::sqlite3_errmsg(db))) }; error_from_sqlite_code(code, message) } diff --git a/src/hooks.rs b/src/hooks.rs index 4b023f5..0691f31 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -102,7 +102,7 @@ impl InnerConnection { // `sqlite3_commit_hook`. so we keep the `xDestroy` function in // `InnerConnection.free_boxed_hook`. let free_commit_hook = if hook.is_some() { - Some(free_boxed_hook:: as fn(*mut c_void)) + Some(free_boxed_hook:: as unsafe fn(*mut c_void)) } else { None }; @@ -122,7 +122,7 @@ impl InnerConnection { }; if !previous_hook.is_null() { if let Some(free_boxed_hook) = self.free_commit_hook { - free_boxed_hook(previous_hook); + unsafe { free_boxed_hook(previous_hook) }; } } self.free_commit_hook = free_commit_hook; @@ -143,7 +143,7 @@ impl InnerConnection { } let free_rollback_hook = if hook.is_some() { - Some(free_boxed_hook:: as fn(*mut c_void)) + Some(free_boxed_hook:: as unsafe fn(*mut c_void)) } else { None }; @@ -163,7 +163,7 @@ impl InnerConnection { }; if !previous_hook.is_null() { if let Some(free_boxed_hook) = self.free_rollback_hook { - free_boxed_hook(previous_hook); + unsafe { free_boxed_hook(previous_hook) }; } } self.free_rollback_hook = free_rollback_hook; @@ -202,7 +202,7 @@ impl InnerConnection { } let free_update_hook = if hook.is_some() { - Some(free_boxed_hook:: as fn(*mut c_void)) + Some(free_boxed_hook:: as unsafe fn(*mut c_void)) } else { None }; @@ -222,15 +222,15 @@ impl InnerConnection { }; if !previous_hook.is_null() { if let Some(free_boxed_hook) = self.free_update_hook { - free_boxed_hook(previous_hook); + unsafe { free_boxed_hook(previous_hook) }; } } self.free_update_hook = free_update_hook; } } -fn free_boxed_hook(p: *mut c_void) { - drop(unsafe { Box::from_raw(p as *mut F) }); +unsafe fn free_boxed_hook(p: *mut c_void) { + drop(Box::from_raw(p as *mut F)); } #[cfg(test)] diff --git a/src/inner_connection.rs b/src/inner_connection.rs index c6694ef..88e6957 100644 --- a/src/inner_connection.rs +++ b/src/inner_connection.rs @@ -27,17 +27,17 @@ pub struct InnerConnection { // interrupt would only acquire the lock after the query's completion. interrupt_lock: Arc>, #[cfg(feature = "hooks")] - pub free_commit_hook: Option, + pub free_commit_hook: Option, #[cfg(feature = "hooks")] - pub free_rollback_hook: Option, + pub free_rollback_hook: Option, #[cfg(feature = "hooks")] - pub free_update_hook: Option, + pub free_update_hook: Option, owned: bool, } impl InnerConnection { #[allow(clippy::mutex_atomic)] - pub fn new(db: *mut ffi::sqlite3, owned: bool) -> InnerConnection { + pub unsafe fn new(db: *mut ffi::sqlite3, owned: bool) -> InnerConnection { InnerConnection { db, interrupt_lock: Arc::new(Mutex::new(db)), @@ -127,10 +127,10 @@ impl InnerConnection { } pub fn decode_result(&mut self, code: c_int) -> Result<()> { - InnerConnection::decode_result_raw(self.db(), code) + unsafe { InnerConnection::decode_result_raw(self.db(), code) } } - fn decode_result_raw(db: *mut ffi::sqlite3, code: c_int) -> Result<()> { + unsafe fn decode_result_raw(db: *mut ffi::sqlite3, code: c_int) -> Result<()> { if code == ffi::SQLITE_OK { Ok(()) } else { @@ -229,9 +229,9 @@ impl InnerConnection { } pub fn prepare<'a>(&mut self, conn: &'a Connection, sql: &str) -> Result> { - let mut c_stmt = MaybeUninit::uninit(); + let mut c_stmt = ptr::null_mut(); let (c_sql, len, _) = str_for_sqlite(sql.as_bytes())?; - let mut c_tail = MaybeUninit::uninit(); + let mut c_tail = ptr::null(); let r = unsafe { if cfg!(feature = "unlock_notify") { let mut rc; @@ -240,8 +240,8 @@ impl InnerConnection { self.db(), c_sql, len, - c_stmt.as_mut_ptr(), - c_tail.as_mut_ptr(), + &mut c_stmt as *mut *mut ffi::sqlite3_stmt, + &mut c_tail as *mut *const c_char, ); if !unlock_notify::is_locked(self.db, rc) { break; @@ -257,8 +257,8 @@ impl InnerConnection { self.db(), c_sql, len, - c_stmt.as_mut_ptr(), - c_tail.as_mut_ptr(), + &mut c_stmt as *mut *mut ffi::sqlite3_stmt, + &mut c_tail as *mut *const c_char, ) } }; @@ -266,11 +266,13 @@ impl InnerConnection { self.decode_result(r)?; // If the input text contains no SQL (if the input is an empty string or a // comment) then *ppStmt is set to NULL. - let c_stmt: *mut ffi::sqlite3_stmt = unsafe { c_stmt.assume_init() }; - let c_tail: *const c_char = unsafe { c_tail.assume_init() }; + let c_stmt: *mut ffi::sqlite3_stmt = c_stmt; + let c_tail: *const c_char = c_tail; // TODO ignore spaces, comments, ... at the end let tail = !c_tail.is_null() && unsafe { c_tail != c_sql.offset(len as isize) }; - Ok(Statement::new(conn, RawStatement::new(c_stmt, tail))) + Ok(Statement::new(conn, unsafe { + RawStatement::new(c_stmt, tail) + })) } pub fn changes(&mut self) -> usize { diff --git a/src/raw_statement.rs b/src/raw_statement.rs index 8d943ba..05634d6 100644 --- a/src/raw_statement.rs +++ b/src/raw_statement.rs @@ -10,7 +10,7 @@ use std::ptr; pub struct RawStatement(*mut ffi::sqlite3_stmt, bool); impl RawStatement { - pub fn new(stmt: *mut ffi::sqlite3_stmt, tail: bool) -> RawStatement { + pub unsafe fn new(stmt: *mut ffi::sqlite3_stmt, tail: bool) -> RawStatement { RawStatement(stmt, tail) } @@ -64,10 +64,10 @@ impl RawStatement { let mut rc; loop { rc = unsafe { ffi::sqlite3_step(self.0) }; - if !unlock_notify::is_locked(db, rc) { + if unsafe { !unlock_notify::is_locked(db, rc) } { break; } - rc = unlock_notify::wait_for_unlock_notify(db); + rc = unsafe { unlock_notify::wait_for_unlock_notify(db) }; if rc != ffi::SQLITE_OK { break; } diff --git a/src/statement.rs b/src/statement.rs index 942fbe9..2c3c14e 100644 --- a/src/statement.rs +++ b/src/statement.rs @@ -619,7 +619,7 @@ impl Statement<'_> { } fn finalize_(&mut self) -> Result<()> { - let mut stmt = RawStatement::new(ptr::null_mut(), false); + let mut stmt = unsafe { RawStatement::new(ptr::null_mut(), false) }; mem::swap(&mut stmt, &mut self.stmt); self.conn.decode_result(stmt.finalize()) } @@ -710,7 +710,7 @@ impl Statement<'_> { impl Into for Statement<'_> { fn into(mut self) -> RawStatement { - let mut stmt = RawStatement::new(ptr::null_mut(), false); + let mut stmt = unsafe { RawStatement::new(ptr::null_mut(), false) }; mem::swap(&mut stmt, &mut self.stmt); stmt } diff --git a/src/unlock_notify.rs b/src/unlock_notify.rs index 8b3d0fc..6f82ae2 100644 --- a/src/unlock_notify.rs +++ b/src/unlock_notify.rs @@ -53,10 +53,10 @@ unsafe extern "C" fn unlock_notify_cb(ap_arg: *mut *mut c_void, n_arg: c_int) { } #[cfg(feature = "unlock_notify")] -pub fn is_locked(db: *mut ffi::sqlite3, rc: c_int) -> bool { +pub unsafe fn is_locked(db: *mut ffi::sqlite3, rc: c_int) -> bool { rc == ffi::SQLITE_LOCKED_SHAREDCACHE || (rc & 0xFF) == ffi::SQLITE_LOCKED - && unsafe { ffi::sqlite3_extended_errcode(db) } == ffi::SQLITE_LOCKED_SHAREDCACHE + && ffi::sqlite3_extended_errcode(db) == ffi::SQLITE_LOCKED_SHAREDCACHE } /// This function assumes that an SQLite API call (either `sqlite3_prepare_v2()` @@ -72,16 +72,14 @@ pub fn is_locked(db: *mut ffi::sqlite3, rc: c_int) -> bool { /// this case the caller should not retry the operation and should roll /// back the current transaction (if any). #[cfg(feature = "unlock_notify")] -pub fn wait_for_unlock_notify(db: *mut ffi::sqlite3) -> c_int { +pub unsafe fn wait_for_unlock_notify(db: *mut ffi::sqlite3) -> c_int { let mut un = UnlockNotification::new(); /* Register for an unlock-notify callback. */ - let rc = unsafe { - ffi::sqlite3_unlock_notify( - db, - Some(unlock_notify_cb), - &mut un as *mut UnlockNotification as *mut c_void, - ) - }; + let rc = ffi::sqlite3_unlock_notify( + db, + Some(unlock_notify_cb), + &mut un as *mut UnlockNotification as *mut c_void, + ); debug_assert!( rc == ffi::SQLITE_LOCKED || rc == ffi::SQLITE_LOCKED_SHAREDCACHE || rc == ffi::SQLITE_OK ); @@ -92,12 +90,12 @@ pub fn wait_for_unlock_notify(db: *mut ffi::sqlite3) -> c_int { } #[cfg(not(feature = "unlock_notify"))] -pub fn is_locked(_db: *mut ffi::sqlite3, _rc: c_int) -> bool { +pub unsafe fn is_locked(_db: *mut ffi::sqlite3, _rc: c_int) -> bool { unreachable!() } #[cfg(not(feature = "unlock_notify"))] -pub fn wait_for_unlock_notify(_db: *mut ffi::sqlite3) -> c_int { +pub unsafe fn wait_for_unlock_notify(_db: *mut ffi::sqlite3) -> c_int { unreachable!() }