Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not take ownership of key on get/delete #24

Closed
kpcyrd opened this issue Sep 20, 2017 · 2 comments
Closed

Do not take ownership of key on get/delete #24

kpcyrd opened this issue Sep 20, 2017 · 2 comments

Comments

@kpcyrd
Copy link

kpcyrd commented Sep 20, 2017

I've noticed that on get and delete, the method takes ownership of they key. This is somewhat unexpected, since the value can be borrowed on put, but the key can't. This may require calling the library with key.clone(), which I'd like to avoid. :)

I've looked into the code, while I think it should be safe to change that to a borrow, I'd like to avoid touching the unsafe parts of the crate myself.

leveldb/src/database/kv.rs

Lines 96 to 126 in 118359e

/// get a value from the database.
///
/// The passed key will be compared using the comparator.
fn get<'a>(&self, options: ReadOptions<'a, K>, key: K) -> Result<Option<Vec<u8>>, Error> {
unsafe {
key.as_slice(|k| {
let mut error = ptr::null_mut();
let mut length: size_t = 0;
let c_readoptions = c_readoptions(&options);
let result = leveldb_get(self.database.ptr,
c_readoptions,
k.as_ptr() as *mut c_char,
k.len() as size_t,
&mut length,
&mut error);
leveldb_readoptions_destroy(c_readoptions);
if error == ptr::null_mut() {
if result == ptr::null_mut() {
Ok(None)
} else {
let vec = from_raw_parts(result as *mut u8, length as usize).to_vec();
leveldb_free(result as *mut c_void);
Ok(Some(vec))
}
} else {
Err(Error::new_from_i8(error))
}
})
}
}

Thanks!

@skade
Copy link
Owner

skade commented Sep 20, 2017

Hi @kpcyrd,

I'm not sure why this decision was made, I'll check that later.

Thanks for the report.

@skade
Copy link
Owner

skade commented Sep 20, 2017

See release 0.8.4.

Relevant commit: 1dbc8e3

@skade skade closed this as completed Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants