From 653d37f3ccef1fb474cc59b04bd96a48ae1ccf2b Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Wed, 29 Jun 2022 17:51:51 +0200 Subject: [PATCH] Use NonNull in DBRawIteratorWithThreadMode (#650) --- src/db_iterator.rs | 56 +++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/src/db_iterator.rs b/src/db_iterator.rs index e3eabe2..f40c3bc 100644 --- a/src/db_iterator.rs +++ b/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, /// 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 { - Self { - inner: ffi::rocksdb_create_iterator(db.inner(), readopts.inner), - _readopts: readopts, - db: PhantomData, - } - } + let inner = unsafe { ffi::rocksdb_create_iterator(db.inner(), readopts.inner) }; + Self::from_inner(inner, readopts) } pub(crate) fn new_cf( @@ -96,12 +91,21 @@ impl<'a, D: DBAccess> DBRawIteratorWithThreadMode<'a, D> { cf_handle: *mut ffi::rocksdb_column_family_handle_t, readopts: ReadOptions, ) -> Self { - unsafe { - Self { - inner: ffi::rocksdb_create_iterator_cf(db.inner(), readopts.inner, cf_handle), - _readopts: readopts, - db: PhantomData, - } + 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, + _readopts: readopts, + db: PhantomData, } } @@ -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()); } } }