Use NonNull in DBRawIteratorWithThreadMode (#650)

src/db_iterator.rs

@@ -70,7 +70,7 @@ pub type DBRawIterator<'a> = DBRawIteratorWithThreadMode<'a, DB>;
 /// let _ = DB::destroy(&Options::default(), path);
 /// ```
 pub struct DBRawIteratorWithThreadMode<'a, D: DBAccess> {
-    inner: *mut ffi::rocksdb_iterator_t,
+    inner: std::ptr::NonNull<ffi::rocksdb_iterator_t>,
 
     /// When iterate_upper_bound is set, the inner C iterator keeps a pointer to the upper bound
     /// inside `_readopts`. Storing this makes sure the upper bound is always alive when the
@@ -82,13 +82,8 @@ pub struct DBRawIteratorWithThreadMode<'a, D: DBAccess> {
 
 impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
     pub(crate) fn new(db: &D, readopts: ReadOptions) -> Self {
-        unsafe {
+        let inner = unsafe { ffi::rocksdb_create_iterator(db.inner(), readopts.inner) };
-            Self {
+        Self::from_inner(inner, readopts)
-                inner: ffi::rocksdb_create_iterator(db.inner(), readopts.inner),
-                _readopts: readopts,
-                db: PhantomData,
-            }
-        }
     }
 
     pub(crate) fn new_cf(
@@ -96,14 +91,23 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
         cf_handle: *mut ffi::rocksdb_column_family_handle_t,
         readopts: ReadOptions,
     ) -> Self {
-        unsafe {
+        let inner =
+            unsafe { ffi::rocksdb_create_iterator_cf(db.inner(), readopts.inner, cf_handle) };
+        Self::from_inner(inner, readopts)
+    }
+
+    fn from_inner(inner: *mut ffi::rocksdb_iterator_t, readopts: ReadOptions) -> Self {
+        // This unwrap will never fail since rocksdb_create_iterator and
+        // rocksdb_create_iterator_cf functions always return non-null.  They
+        // use new and deference the result so any nulls would end up in SIGSEGV
+        // there and we have bigger issue.
+        let inner = std::ptr::NonNull::new(inner).unwrap();
         Self {
-                inner: ffi::rocksdb_create_iterator_cf(db.inner(), readopts.inner, cf_handle),
+            inner,
             _readopts: readopts,
             db: PhantomData,
         }
     }
-    }
 
     /// Returns `true` if the iterator is valid. An iterator is invalidated when
     /// it reaches the end of its defined range, or when it encounters an error.
@@ -112,7 +116,7 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
     /// returned `false`, use the [`status`](DBRawIteratorWithThreadMode::status) method. `status` will never
     /// return an error when `valid` is `true`.
     pub fn valid(&self) -> bool {
-        unsafe { ffi::rocksdb_iter_valid(self.inner) != 0 }
+        unsafe { ffi::rocksdb_iter_valid(self.inner.as_ptr()) != 0 }
     }
 
     /// Returns an error `Result` if the iterator has encountered an error
@@ -122,7 +126,7 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
     /// Performing a seek will discard the current status.
     pub fn status(&self) -> Result<(), Error> {
         unsafe {
-            ffi_try!(ffi::rocksdb_iter_get_error(self.inner));
+            ffi_try!(ffi::rocksdb_iter_get_error(self.inner.as_ptr()));
         }
         Ok(())
     }
@@ -160,7 +164,7 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
     /// ```
     pub fn seek_to_first(&mut self) {
         unsafe {
-            ffi::rocksdb_iter_seek_to_first(self.inner);
+            ffi::rocksdb_iter_seek_to_first(self.inner.as_ptr());
         }
     }
 
@@ -197,7 +201,7 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
     /// ```
     pub fn seek_to_last(&mut self) {
         unsafe {
-            ffi::rocksdb_iter_seek_to_last(self.inner);
+            ffi::rocksdb_iter_seek_to_last(self.inner.as_ptr());
         }
     }
 
@@ -232,7 +236,7 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
 
         unsafe {
             ffi::rocksdb_iter_seek(
-                self.inner,
+                self.inner.as_ptr(),
                 key.as_ptr() as *const c_char,
                 key.len() as size_t,
             );
@@ -271,7 +275,7 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
 
         unsafe {
             ffi::rocksdb_iter_seek_for_prev(
-                self.inner,
+                self.inner.as_ptr(),
                 key.as_ptr() as *const c_char,
                 key.len() as size_t,
             );
@@ -281,14 +285,14 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
     /// Seeks to the next key.
     pub fn next(&mut self) {
         unsafe {
-            ffi::rocksdb_iter_next(self.inner);
+            ffi::rocksdb_iter_next(self.inner.as_ptr());
         }
     }
 
     /// Seeks to the previous key.
     pub fn prev(&mut self) {
         unsafe {
-            ffi::rocksdb_iter_prev(self.inner);
+            ffi::rocksdb_iter_prev(self.inner.as_ptr());
         }
     }
 
@@ -300,7 +304,8 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
             unsafe {
                 let mut key_len: size_t = 0;
                 let key_len_ptr: *mut size_t = &mut key_len;
-                let key_ptr = ffi::rocksdb_iter_key(self.inner, key_len_ptr) as *const c_uchar;
+                let key_ptr =
+                    ffi::rocksdb_iter_key(self.inner.as_ptr(), key_len_ptr) as *const c_uchar;
 
                 Some(slice::from_raw_parts(key_ptr, key_len as usize))
             }
@@ -317,7 +322,8 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
             unsafe {
                 let mut val_len: size_t = 0;
                 let val_len_ptr: *mut size_t = &mut val_len;
-                let val_ptr = ffi::rocksdb_iter_value(self.inner, val_len_ptr) as *const c_uchar;
+                let val_ptr =
+                    ffi::rocksdb_iter_value(self.inner.as_ptr(), val_len_ptr) as *const c_uchar;
 
                 Some(slice::from_raw_parts(val_ptr, val_len as usize))
             }
@@ -330,7 +336,7 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> {
 impl<'a, D: DBAccess> Drop for DBRawIteratorWithThreadMode<'a, D> {
     fn drop(&mut self) {
         unsafe {
-            ffi::rocksdb_iter_destroy(self.inner);
+            ffi::rocksdb_iter_destroy(self.inner.as_ptr());
         }
     }
 }