Refactor hooks

src/hooks.rs

@@ -1,6 +1,7 @@
 //! Commit, Data Change and Rollback Notification Callbacks
 #![allow(non_camel_case_types)]
 
+use std::mem::size_of;
 use std::ptr;
 use std::os::raw::{c_int, c_char, c_void};
 
@@ -94,8 +95,9 @@ impl Connection {
     /// Register a callback function to be invoked whenever a transaction is committed.
     ///
     /// The callback returns `true` to rollback.
-    pub fn commit_hook<F>(&self, hook: F)
+    pub fn commit_hook<F>(&self, hook: Option<F>)
-        where F: FnMut() -> bool
+    where
+        F: FnMut() -> bool,
     {
         self.db.borrow_mut().commit_hook(hook);
     }
@@ -103,8 +105,9 @@ impl Connection {
     /// Register a callback function to be invoked whenever a transaction is committed.
     ///
     /// The callback returns `true` to rollback.
-    pub fn rollback_hook<F>(&self, hook: F)
+    pub fn rollback_hook<F>(&self, hook: Option<F>)
-        where F: FnMut()
+    where
+        F: FnMut(),
     {
         self.db.borrow_mut().rollback_hook(hook);
     }
@@ -118,99 +121,104 @@ impl Connection {
     ///   - the name of the database ("main", "temp", ...),
     ///   - the name of the table that is updated,
     ///   - the ROWID of the row that is updated.
-    pub fn update_hook<F>(&self, hook: F)
+    pub fn update_hook<F>(&self, hook: Option<F>)
-        where F: FnMut(Action, &str, &str, i64)
+    where
+        F: FnMut(Action, &str, &str, i64),
     {
         self.db.borrow_mut().update_hook(hook);
     }
-
-    /// Remove hook installed by `update_hook`.
-    pub fn remove_update_hook(&self) {
-        self.db.borrow_mut().remove_update_hook();
-    }
-
-    /// Remove hook installed by `commit_hook`.
-    pub fn remove_commit_hook(&self) {
-        self.db.borrow_mut().remove_commit_hook();
-    }
-
-    /// Remove hook installed by `rollback_hook`.
-    pub fn remove_rollback_hook(&self) {
-        self.db.borrow_mut().remove_rollback_hook();
-    }
 }
 
 impl InnerConnection {
     pub fn remove_hooks(&mut self) {
-        self.remove_update_hook();
+        self.update_hook(None::<fn(Action, &str, &str, i64)>);
-        self.remove_commit_hook();
+        self.commit_hook(None::<fn() -> bool>);
-        self.remove_rollback_hook();
+        self.rollback_hook(None::<fn()>);
     }
 
-    fn commit_hook<F>(&self, hook: F)
+    fn commit_hook<F>(&self, hook: Option<F>)
-        where F: FnMut() -> bool
+    where
+        F: FnMut() -> bool,
     {
+        assert_eq!(size_of::<*mut F>(), size_of::<*mut c_void>());
+
         unsafe extern "C" fn call_boxed_closure<F>(p_arg: *mut c_void) -> c_int
-            where F: FnMut() -> bool
+        where
+            F: FnMut() -> bool,
         {
             let boxed_hook: *mut F = p_arg as *mut F;
-            assert!(!boxed_hook.is_null(),
+            if (*boxed_hook)() {
-                    "Internal error - null function pointer");
+                1
-
+            } else {
-            if (*boxed_hook)() { 1 } else { 0 }
+                0
+            }
         }
 
-        let previous_hook = {
+        let previous_hook = match hook {
+            Some(hook) => {
                 let boxed_hook: *mut F = Box::into_raw(Box::new(hook));
                 unsafe {
-                ffi::sqlite3_commit_hook(self.db(),
+                    ffi::sqlite3_commit_hook(
+                        self.db(),
                         Some(call_boxed_closure::<F>),
-                                         boxed_hook as *mut _)
+                        boxed_hook as *mut _,
+                    )
                 }
+            }
+            _ => unsafe { ffi::sqlite3_commit_hook(self.db(), None, ptr::null_mut()) },
         };
         free_boxed_hook(previous_hook);
     }
 
-    fn rollback_hook<F>(&self, hook: F)
+    fn rollback_hook<F>(&self, hook: Option<F>)
-        where F: FnMut()
+    where
+        F: FnMut(),
     {
+        assert_eq!(size_of::<*mut F>(), size_of::<*mut c_void>());
+
         unsafe extern "C" fn call_boxed_closure<F>(p_arg: *mut c_void)
-            where F: FnMut()
+        where
+            F: FnMut(),
         {
             let boxed_hook: *mut F = p_arg as *mut F;
-            assert!(!boxed_hook.is_null(),
-                    "Internal error - null function pointer");
-
             (*boxed_hook)();
         }
 
-        let previous_hook = {
+        let previous_hook = match hook {
+            Some(hook) => {
                 let boxed_hook: *mut F = Box::into_raw(Box::new(hook));
                 unsafe {
-                ffi::sqlite3_rollback_hook(self.db(),
+                    ffi::sqlite3_rollback_hook(
+                        self.db(),
                         Some(call_boxed_closure::<F>),
-                                           boxed_hook as *mut _)
+                        boxed_hook as *mut _,
+                    )
                 }
+            }
+            _ => unsafe { ffi::sqlite3_rollback_hook(self.db(), None, ptr::null_mut()) },
         };
         free_boxed_hook(previous_hook);
     }
 
-    fn update_hook<F>(&mut self, hook: F)
+    fn update_hook<F>(&mut self, hook: Option<F>)
-        where F: FnMut(Action, &str, &str, i64)
+    where
+        F: FnMut(Action, &str, &str, i64),
     {
-        unsafe extern "C" fn call_boxed_closure<F>(p_arg: *mut c_void,
+        assert_eq!(size_of::<*mut F>(), size_of::<*mut c_void>());
+
+        unsafe extern "C" fn call_boxed_closure<F>(
+            p_arg: *mut c_void,
             action_code: c_int,
             db_str: *const c_char,
             tbl_str: *const c_char,
-                                                   row_id: i64)
+            row_id: i64,
-            where F: FnMut(Action, &str, &str, i64)
+        ) where
+            F: FnMut(Action, &str, &str, i64),
         {
             use std::ffi::CStr;
             use std::str;
 
             let boxed_hook: *mut F = p_arg as *mut F;
-            assert!(!boxed_hook.is_null(),
-                    "Internal error - null function pointer");
 
             let action = Action::from(action_code);
             let db_name = {
@@ -225,36 +233,26 @@ impl InnerConnection {
             (*boxed_hook)(action, db_name, tbl_name, row_id);
         }
 
-        let previous_hook = {
+        let previous_hook = match hook {
+            Some(hook) => {
                 let boxed_hook: *mut F = Box::into_raw(Box::new(hook));
                 unsafe {
-                ffi::sqlite3_update_hook(self.db(),
+                    ffi::sqlite3_update_hook(
+                        self.db(),
                         Some(call_boxed_closure::<F>),
-                                         boxed_hook as *mut _)
+                        boxed_hook as *mut _,
+                    )
                 }
+            }
+            _ => unsafe { ffi::sqlite3_update_hook(self.db(), None, ptr::null_mut()) },
         };
         free_boxed_hook(previous_hook);
     }
-
-    fn remove_update_hook(&mut self) {
-        let previous_hook = unsafe { ffi::sqlite3_update_hook(self.db(), None, ptr::null_mut()) };
-        free_boxed_hook(previous_hook);
-    }
-
-    fn remove_commit_hook(&mut self) {
-        let previous_hook = unsafe { ffi::sqlite3_commit_hook(self.db(), None, ptr::null_mut()) };
-        free_boxed_hook(previous_hook);
-    }
-
-    fn remove_rollback_hook(&mut self) {
-        let previous_hook = unsafe { ffi::sqlite3_rollback_hook(self.db(), None, ptr::null_mut()) };
-        free_boxed_hook(previous_hook);
-    }
 }
 
 fn free_boxed_hook(hook: *mut c_void) {
     if !hook.is_null() {
-        // TODO make sure that size_of::<*mut F>() is always equal to size_of::<*mut c_void>()
+        // make sure that size_of::<*mut F>() is always equal to size_of::<*mut c_void>()
         let _: Box<*mut c_void> = unsafe { Box::from_raw(hook as *mut _) };
     }
 }
@@ -269,10 +267,10 @@ mod test {
         let db = Connection::open_in_memory().unwrap();
 
         let mut called = false;
-        db.commit_hook(|| {
+        db.commit_hook(Some(|| {
             called = true;
             false
-                       });
+        }));
         db.execute_batch("BEGIN; CREATE TABLE foo (t TEXT); COMMIT;")
             .unwrap();
         assert!(called);
@@ -283,7 +281,9 @@ mod test {
         let db = Connection::open_in_memory().unwrap();
 
         let mut called = false;
-        db.rollback_hook(|| { called = true; });
+        db.rollback_hook(Some(|| {
+            called = true;
+        }));
         db.execute_batch("BEGIN; CREATE TABLE foo (t TEXT); ROLLBACK;")
             .unwrap();
         assert!(called);
@@ -294,13 +294,13 @@ mod test {
         let db = Connection::open_in_memory().unwrap();
 
         let mut called = false;
-        db.update_hook(|action, db, tbl, row_id| {
+        db.update_hook(Some(|action, db: &str, tbl: &str, row_id| {
             assert_eq!(Action::SQLITE_INSERT, action);
             assert_eq!("main", db);
             assert_eq!("foo", tbl);
             assert_eq!(1, row_id);
             called = true;
-                       });
+        }));
         db.execute_batch("CREATE TABLE foo (t TEXT)").unwrap();
         db.execute_batch("INSERT INTO foo VALUES ('lisa')").unwrap();
         assert!(called);