diff --git a/src/db.rs b/src/db.rs index 7a8a071..bc80d98 100644 --- a/src/db.rs +++ b/src/db.rs @@ -488,53 +488,39 @@ impl<'a> DBRawIterator<'a> { } } - /// Returns a slice to the internal buffer storing the current key. - /// - /// This may be slightly more performant to use than the standard ``.key()`` method - /// as it does not copy the key. However, you must be careful to not use the buffer - /// if the iterator's seek position is ever moved by any of the seek commands or the - /// ``.next()`` and ``.previous()`` methods as the underlying buffer may be reused - /// for something else or freed entirely. - pub unsafe fn key_inner(&self) -> Option<&[u8]> { + /// Returns a slice of the current key. + pub fn key(&self) -> Option<&[u8]> { if self.valid() { - 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; + // Safety Note: This is safe as all methods that may invalidate the buffer returned + // take `&mut self`, so borrow checker will prevent use of buffer after seek. + 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; - Some(slice::from_raw_parts(key_ptr, key_len as usize)) + Some(slice::from_raw_parts(key_ptr, key_len as usize)) + } } else { None } } - /// Returns a copy of the current key. - pub fn key(&self) -> Option> { - unsafe { self.key_inner().map(|key| key.to_vec()) } - } - - /// Returns a slice to the internal buffer storing the current value. - /// - /// This may be slightly more performant to use than the standard ``.value()`` method - /// as it does not copy the value. However, you must be careful to not use the buffer - /// if the iterator's seek position is ever moved by any of the seek commands or the - /// ``.next()`` and ``.previous()`` methods as the underlying buffer may be reused - /// for something else or freed entirely. - pub unsafe fn value_inner(&self) -> Option<&[u8]> { + /// Returns a slice of the current value. + pub fn value(&self) -> Option<&[u8]> { if self.valid() { - 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; + // Safety Note: This is safe as all methods that may invalidate the buffer returned + // take `&mut self`, so borrow checker will prevent use of buffer after seek. + 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; - Some(slice::from_raw_parts(val_ptr, val_len as usize)) + Some(slice::from_raw_parts(val_ptr, val_len as usize)) + } } else { None } } - - /// Returns a copy of the current value. - pub fn value(&self) -> Option> { - unsafe { self.value_inner().map(|value| value.to_vec()) } - } } impl<'a> Drop for DBRawIterator<'a> { @@ -623,8 +609,8 @@ impl<'a> Iterator for DBIterator<'a> { if self.raw.valid() { // .key() and .value() only ever return None if valid == false, which we've just cheked Some(( - self.raw.key().unwrap().into_boxed_slice(), - self.raw.value().unwrap().into_boxed_slice(), + self.raw.key().unwrap().to_vec().into_boxed_slice(), + self.raw.value().unwrap().to_vec().into_boxed_slice(), )) } else { None @@ -2031,6 +2017,7 @@ unsafe impl Send for ReadOptions {} // Sync is similarly safe for many types because they do not expose interior mutability, and their // use within the rocksdb library is generally behind a const reference +unsafe impl<'a> Sync for DBRawIterator<'a> {} unsafe impl Sync for ReadOptions {} /// Vector of bytes stored in the database. diff --git a/tests/test_raw_iterator.rs b/tests/test_raw_iterator.rs index 8d4f019..7fd09e8 100644 --- a/tests/test_raw_iterator.rs +++ b/tests/test_raw_iterator.rs @@ -32,14 +32,14 @@ pub fn test_forwards_iteration() { iter.seek_to_first(); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k1".to_vec())); - assert_eq!(iter.value(), Some(b"v1".to_vec())); + assert_eq!(iter.key(), Some(b"k1".as_ref())); + assert_eq!(iter.value(), Some(b"v1".as_ref())); iter.next(); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k2".to_vec())); - assert_eq!(iter.value(), Some(b"v2".to_vec())); + assert_eq!(iter.key(), Some(b"k2".as_ref())); + assert_eq!(iter.value(), Some(b"v2".as_ref())); iter.next(); // k3 iter.next(); // k4 @@ -65,14 +65,14 @@ pub fn test_seek_last() { iter.seek_to_last(); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k4".to_vec())); - assert_eq!(iter.value(), Some(b"v4".to_vec())); + assert_eq!(iter.key(), Some(b"k4".as_ref())); + assert_eq!(iter.value(), Some(b"v4".as_ref())); iter.prev(); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k3".to_vec())); - assert_eq!(iter.value(), Some(b"v3".to_vec())); + assert_eq!(iter.key(), Some(b"k3".as_ref())); + assert_eq!(iter.value(), Some(b"v3".as_ref())); iter.prev(); // k2 iter.prev(); // k1 @@ -97,15 +97,15 @@ pub fn test_seek() { iter.seek(b"k2"); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k2".to_vec())); - assert_eq!(iter.value(), Some(b"v2".to_vec())); + assert_eq!(iter.key(), Some(b"k2".as_ref())); + assert_eq!(iter.value(), Some(b"v2".as_ref())); // Check it gets the next key when the key doesn't exist iter.seek(b"k3"); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k4".to_vec())); - assert_eq!(iter.value(), Some(b"v4".to_vec())); + assert_eq!(iter.key(), Some(b"k4".as_ref())); + assert_eq!(iter.value(), Some(b"v4".as_ref())); } } @@ -122,8 +122,8 @@ pub fn test_seek_to_nonexistant() { iter.seek(b"k2"); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k3".to_vec())); - assert_eq!(iter.value(), Some(b"v3".to_vec())); + assert_eq!(iter.key(), Some(b"k3".as_ref())); + assert_eq!(iter.value(), Some(b"v3".as_ref())); } } @@ -140,14 +140,14 @@ pub fn test_seek_for_prev() { iter.seek(b"k2"); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k2".to_vec())); - assert_eq!(iter.value(), Some(b"v2".to_vec())); + assert_eq!(iter.key(), Some(b"k2".as_ref())); + assert_eq!(iter.value(), Some(b"v2".as_ref())); // Check it gets the previous key when the key doesn't exist iter.seek_for_prev(b"k3"); assert_eq!(iter.valid(), true); - assert_eq!(iter.key(), Some(b"k2".to_vec())); - assert_eq!(iter.value(), Some(b"v2".to_vec())); + assert_eq!(iter.key(), Some(b"k2".as_ref())); + assert_eq!(iter.value(), Some(b"v2".as_ref())); } }