Tighten up `unsafe{}` blocks in backup logic.

1 files changed, 26 insertions, 31 deletions

diff --git a/src/db/backup.rs b/src/db/backup.rs
index e34df9f..573a4c4 100644
--- a/src/db/backup.rs
+++ b/src/db/backup.rs
@@ -51,34 +51,34 @@ impl Backup {
         step_result
     }
 
-    fn start(to: NonNull<sqlite3>, from: NonNull<sqlite3>) -> Result<*mut sqlite3_backup, Error> {
+    fn start(
+        to: NonNull<sqlite3>,
+        from: NonNull<sqlite3>,
+    ) -> Result<NonNull<sqlite3_backup>, Error> {
         let name = CString::new("main").expect("static constant is a valid C string");
-        unsafe {
-            // Invariants:
-            //
-            // * `to` and `from` must be valid `sqlite3` pointers (guaranteed by sqlx)
-            // * `zDestName` and `zSourceName` must be valid C strings (see above)
-            //
-            // Never evaluates to null (even though `sqlite3_backup_init` can).
-            let handle =
-                sqlite3_backup_init(to.as_ptr(), name.as_ptr(), from.as_ptr(), name.as_ptr());
-            if handle.is_null() {
-                Err(Error::Backup {
-                    code: Error::code_for(to),
-                    message: Error::message_for(to),
-                })?;
-            }
-            Ok(handle)
+        // Invariants:
+        //
+        // * `to` and `from` must be valid `sqlite3` pointers (guaranteed by sqlx)
+        // * `zDestName` and `zSourceName` must be valid C strings (see above)
+        //
+        // Never evaluates to null (even though `sqlite3_backup_init` can).
+        let handle = unsafe {
+            sqlite3_backup_init(to.as_ptr(), name.as_ptr(), from.as_ptr(), name.as_ptr())
+        };
+        if handle.is_null() {
+            Err(Error::Backup {
+                code: Error::code_for(to),
+                message: Error::message_for(to),
+            })?;
         }
+        // Having proven that `handle` is not null, we could use new_unchecked here.
+        // Choosing not to so that any mistakes are caught, rather than causing
+        // undefined behaviour later on.
+        Ok(NonNull::new(handle).expect("backup handle is non-null"))
     }
 
-    fn step(handle: *mut sqlite3_backup, pages: c_int) -> Result<(), Error> {
-        let step = unsafe {
-            // Invariants:
-            //
-            // * `handle` must be a valid backup handle (see above).
-            sqlite3_backup_step(handle, pages)
-        };
+    fn step(handle: NonNull<sqlite3_backup>, pages: c_int) -> Result<(), Error> {
+        let step = unsafe { sqlite3_backup_step(handle.as_ptr(), pages) };
         if SQLITE_BUSY == step {
             Err(Error::Backup {
                 code: step,
@@ -94,13 +94,8 @@ impl Backup {
         }
     }
 
-    fn finish(to: NonNull<sqlite3>, handle: *mut sqlite3_backup) -> Result<(), Error> {
-        let finished = unsafe {
-            // Invariants:
-            //
-            // * `handle` must be a valid backup handle (see above).
-            sqlite3_backup_finish(handle)
-        };
+    fn finish(to: NonNull<sqlite3>, handle: NonNull<sqlite3_backup>) -> Result<(), Error> {
+        let finished = unsafe { sqlite3_backup_finish(handle.as_ptr()) };
         if finished == SQLITE_OK {
             Ok(())
         } else {