From 716222894d95a1a7c9773baabead99679fc73c21 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sat, 18 Nov 2023 00:23:09 +0100 Subject: [PATCH] chore: make Cursor iterators depend on the cursor's lifetime (#5479) --- crates/storage/libmdbx-rs/src/codec.rs | 30 +++------- crates/storage/libmdbx-rs/src/cursor.rs | 58 +++++++++----------- crates/storage/libmdbx-rs/src/environment.rs | 2 +- crates/storage/libmdbx-rs/src/transaction.rs | 2 +- 4 files changed, 37 insertions(+), 55 deletions(-) diff --git a/crates/storage/libmdbx-rs/src/codec.rs b/crates/storage/libmdbx-rs/src/codec.rs index 40509f533d85..fc9f48856ec8 100644 --- a/crates/storage/libmdbx-rs/src/codec.rs +++ b/crates/storage/libmdbx-rs/src/codec.rs @@ -3,11 +3,9 @@ use derive_more::*; use std::{borrow::Cow, slice}; /// Implement this to be able to decode data values -pub trait TableObject { +pub trait TableObject: Sized { /// Decodes the object from the given bytes. - fn decode(data_val: &[u8]) -> Result - where - Self: Sized; + fn decode(data_val: &[u8]) -> Result; /// Decodes the value directly from the given MDBX_val pointer. /// @@ -17,14 +15,13 @@ pub trait TableObject { #[doc(hidden)] unsafe fn decode_val( _: *const ffi::MDBX_txn, - data_val: &ffi::MDBX_val, + data_val: ffi::MDBX_val, ) -> Result where Self: Sized, { let s = slice::from_raw_parts(data_val.iov_base as *const u8, data_val.iov_len); - - TableObject::decode(s) + Self::decode(s) } } @@ -36,7 +33,7 @@ impl<'tx> TableObject for Cow<'tx, [u8]> { #[doc(hidden)] unsafe fn decode_val( _txn: *const ffi::MDBX_txn, - data_val: &ffi::MDBX_val, + data_val: ffi::MDBX_val, ) -> Result { let s = slice::from_raw_parts(data_val.iov_base as *const u8, data_val.iov_len); @@ -56,10 +53,7 @@ impl<'tx> TableObject for Cow<'tx, [u8]> { } impl TableObject for Vec { - fn decode(data_val: &[u8]) -> Result - where - Self: Sized, - { + fn decode(data_val: &[u8]) -> Result { Ok(data_val.to_vec()) } } @@ -71,7 +65,7 @@ impl TableObject for () { unsafe fn decode_val( _: *const ffi::MDBX_txn, - _: &ffi::MDBX_val, + _: ffi::MDBX_val, ) -> Result { Ok(()) } @@ -82,19 +76,13 @@ impl TableObject for () { pub struct ObjectLength(pub usize); impl TableObject for ObjectLength { - fn decode(data_val: &[u8]) -> Result - where - Self: Sized, - { + fn decode(data_val: &[u8]) -> Result { Ok(Self(data_val.len())) } } impl TableObject for [u8; LEN] { - fn decode(data_val: &[u8]) -> Result - where - Self: Sized, - { + fn decode(data_val: &[u8]) -> Result { if data_val.len() != LEN { return Err(Error::DecodeErrorLenDiff) } diff --git a/crates/storage/libmdbx-rs/src/cursor.rs b/crates/storage/libmdbx-rs/src/cursor.rs index 30765bc93e85..a5cb2a3830a8 100644 --- a/crates/storage/libmdbx-rs/src/cursor.rs +++ b/crates/storage/libmdbx-rs/src/cursor.rs @@ -1,14 +1,3 @@ -use std::{borrow::Cow, fmt, marker::PhantomData, mem, ptr}; - -use libc::c_void; - -use ffi::{ - MDBX_cursor_op, MDBX_FIRST, MDBX_FIRST_DUP, MDBX_GET_BOTH, MDBX_GET_BOTH_RANGE, - MDBX_GET_CURRENT, MDBX_GET_MULTIPLE, MDBX_LAST, MDBX_LAST_DUP, MDBX_NEXT, MDBX_NEXT_DUP, - MDBX_NEXT_MULTIPLE, MDBX_NEXT_NODUP, MDBX_PREV, MDBX_PREV_DUP, MDBX_PREV_MULTIPLE, - MDBX_PREV_NODUP, MDBX_SET, MDBX_SET_KEY, MDBX_SET_LOWERBOUND, MDBX_SET_RANGE, -}; - use crate::{ error::{mdbx_result, Error, Result}, flags::*, @@ -16,6 +5,14 @@ use crate::{ transaction::{TransactionKind, RW}, TableObject, Transaction, }; +use ffi::{ + MDBX_cursor_op, MDBX_FIRST, MDBX_FIRST_DUP, MDBX_GET_BOTH, MDBX_GET_BOTH_RANGE, + MDBX_GET_CURRENT, MDBX_GET_MULTIPLE, MDBX_LAST, MDBX_LAST_DUP, MDBX_NEXT, MDBX_NEXT_DUP, + MDBX_NEXT_MULTIPLE, MDBX_NEXT_NODUP, MDBX_PREV, MDBX_PREV_DUP, MDBX_PREV_MULTIPLE, + MDBX_PREV_NODUP, MDBX_SET, MDBX_SET_KEY, MDBX_SET_LOWERBOUND, MDBX_SET_RANGE, +}; +use libc::c_void; +use std::{borrow::Cow, fmt, marker::PhantomData, mem, ptr}; /// A cursor for navigating the items within a database. pub struct Cursor @@ -60,22 +57,20 @@ where self.cursor } - /// Returns an Iterator over the raw key value slices - /// - /// Note: The lifetime ensures that the transaction is kept alive while entries are used - pub fn into_iter_slices<'cur>(self) -> IntoIter<'cur, K, Cow<'cur, [u8]>, Cow<'cur, [u8]>> { + /// Returns an iterator over the raw key value slices. + #[allow(clippy::needless_lifetimes)] + pub fn iter_slices<'a>(&'a self) -> IntoIter<'a, K, Cow<'a, [u8]>, Cow<'a, [u8]>> { self.into_iter() } - /// Returns an Iterator over key value pairs of the cursor - /// - /// Note: The lifetime ensures that the transaction is kept alive while entries are used + + /// Returns an iterator over database items. #[allow(clippy::should_implement_trait)] - pub fn into_iter<'cur, Key, Value>(self) -> IntoIter<'cur, K, Key, Value> + pub fn into_iter(&self) -> IntoIter<'_, K, Key, Value> where Key: TableObject, Value: TableObject, { - IntoIter::new(self, MDBX_NEXT, MDBX_NEXT) + IntoIter::new(self.clone(), MDBX_NEXT, MDBX_NEXT) } /// Retrieves a key/data pair from the cursor. Depending on the cursor op, @@ -106,12 +101,12 @@ where let key_out = { // MDBX wrote in new key if key_ptr != key_val.iov_base { - Some(Key::decode_val::(txn, &key_val)?) + Some(Key::decode_val::(txn, key_val)?) } else { None } }; - let data_out = Value::decode_val::(txn, &data_val)?; + let data_out = Value::decode_val::(txn, data_val)?; Ok((key_out, data_out, v)) }) } @@ -335,16 +330,16 @@ where Ok(Some((found, k.unwrap(), v))) } - /// Iterate over database items. The iterator will begin with item next - /// after the cursor, and continue until the end of the database. For new - /// cursors, the iterator will begin with the first item in the database. + /// Returns an iterator over database items. + /// + /// The iterator will begin with item next after the cursor, and continue until the end of the + /// database. For new cursors, the iterator will begin with the first item in the database. /// /// For databases with duplicate data items ([DatabaseFlags::DUP_SORT]), the /// duplicate data items of each key will be returned before moving on to /// the next key. pub fn iter(&mut self) -> Iter<'_, K, Key, Value> where - Self: Sized, Key: TableObject, Value: TableObject, { @@ -358,7 +353,6 @@ where /// the next key. pub fn iter_start(&mut self) -> Iter<'_, K, Key, Value> where - Self: Sized, Key: TableObject, Value: TableObject, { @@ -536,7 +530,7 @@ where /// The next and subsequent operations to perform. next_op: ffi::MDBX_cursor_op, - _marker: PhantomData, + _marker: PhantomData<(&'cur (), Key, Value)>, }, } @@ -570,11 +564,11 @@ where cursor.txn.txn_execute(|txn| { match ffi::mdbx_cursor_get(cursor.cursor(), &mut key, &mut data, op) { ffi::MDBX_SUCCESS => { - let key = match Key::decode_val::(txn, &key) { + let key = match Key::decode_val::(txn, key) { Ok(v) => v, Err(e) => return Some(Err(e)), }; - let data = match Value::decode_val::(txn, &data) { + let data = match Value::decode_val::(txn, data) { Ok(v) => v, Err(e) => return Some(Err(e)), }; @@ -661,11 +655,11 @@ where cursor.txn.txn_execute(|txn| { match ffi::mdbx_cursor_get(cursor.cursor(), &mut key, &mut data, op) { ffi::MDBX_SUCCESS => { - let key = match Key::decode_val::(txn, &key) { + let key = match Key::decode_val::(txn, key) { Ok(v) => v, Err(e) => return Some(Err(e)), }; - let data = match Value::decode_val::(txn, &data) { + let data = match Value::decode_val::(txn, data) { Ok(v) => v, Err(e) => return Some(Err(e)), }; diff --git a/crates/storage/libmdbx-rs/src/environment.rs b/crates/storage/libmdbx-rs/src/environment.rs index 2342373a27c6..0b83a243c0f4 100644 --- a/crates/storage/libmdbx-rs/src/environment.rs +++ b/crates/storage/libmdbx-rs/src/environment.rs @@ -208,7 +208,7 @@ impl Environment { let db = Database::freelist_db(); let cursor = txn.cursor(&db)?; - for result in cursor.into_iter_slices() { + for result in cursor.iter_slices() { let (_key, value) = result?; if value.len() < size_of::() { return Err(Error::Corrupted) diff --git a/crates/storage/libmdbx-rs/src/transaction.rs b/crates/storage/libmdbx-rs/src/transaction.rs index 1b102ad4ffe6..2330c3a3f891 100644 --- a/crates/storage/libmdbx-rs/src/transaction.rs +++ b/crates/storage/libmdbx-rs/src/transaction.rs @@ -156,7 +156,7 @@ where self.txn_execute(|txn| unsafe { match ffi::mdbx_get(txn, dbi, &key_val, &mut data_val) { - ffi::MDBX_SUCCESS => Key::decode_val::(txn, &data_val).map(Some), + ffi::MDBX_SUCCESS => Key::decode_val::(txn, data_val).map(Some), ffi::MDBX_NOTFOUND => Ok(None), err_code => Err(Error::from_err_code(err_code)), }