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.

Sign up for GitHub

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

Improve handling of non-unicode C strings #34

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Improve handling of non-unicode C strings #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6dff7c
Use portable c_char instead of i8
a-rodin Oct 25, 2019
20c9263
Use `pub(crate)` instead of `pub` for `new_from_char` method
a-rodin Oct 31, 2019
e931162
Don't test Rust 1.15 support in CI
a-rodin Oct 31, 2019
9fb3308
Improve handling of non-unicode C strings
a-rodin Oct 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
language: rust
rust:
- 1.38.0
- 1.20.0
- 1.15.0
- nightly
matrix:
allow_failures:
Expand Down
8 changes: 4 additions & 4 deletions src/database/batch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<K: Key> Batch<K> for Database<K> {
if error == ptr::null_mut() {
Ok(())
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
}
}
Expand Down Expand Up @@ -124,9 +124,9 @@ pub trait WritebatchIterator {
}

extern "C" fn put_callback<K: Key, T: WritebatchIterator<K = K>>(state: *mut c_void,
key: *const i8,
key: *const c_char,
keylen: size_t,
val: *const i8,
val: *const c_char,
vallen: size_t) {
unsafe {
let iter: &mut T = &mut *(state as *mut T);
Expand All @@ -138,7 +138,7 @@ extern "C" fn put_callback<K: Key, T: WritebatchIterator<K = K>>(state: *mut c_v
}

extern "C" fn deleted_callback<K: Key, T: WritebatchIterator<K = K>>(state: *mut c_void,
key: *const i8,
key: *const c_char,
keylen: size_t) {
unsafe {
let iter: &mut T = &mut *(state as *mut T);
Expand Down
4 changes: 2 additions & 2 deletions src/database/comparator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ unsafe trait InternalComparator : Comparator where Self: Sized {
}

extern "C" fn compare(state: *mut c_void,
a: *const i8,
a: *const c_char,
a_len: size_t,
b: *const i8,
b: *const c_char,
b_len: size_t)
-> i32 {
unsafe {
Expand Down
13 changes: 9 additions & 4 deletions src/database/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! The module defining custom leveldb error type.

use libc::c_void;
use libc::{c_char, c_void};
use leveldb_sys::leveldb_free;
use std;

Expand All @@ -21,13 +21,18 @@ impl Error {
///
/// This method is `unsafe` because the pointer must be valid and point to heap.
/// The pointer will be passed to `free`!
pub unsafe fn new_from_i8(message: *const i8) -> Error {
pub(crate) unsafe fn new_from_char(message: *const c_char) -> Error {
use std::str::from_utf8;
use std::ffi::CStr;

let err_string = from_utf8(CStr::from_ptr(message).to_bytes()).unwrap().to_string();
let err_string = from_utf8(CStr::from_ptr(message).to_bytes())
.map(|s| s.to_string());
leveldb_free(message as *mut c_void);
Error::new(err_string)

match err_string {
Ok(err_string) => Error::new(err_string),
Err(_) => Error::new("The error returned by LevelDB is not valid UTF-8".into())
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/database/kv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl<K: Key> KV<K> for Database<K> {
if error == ptr::null_mut() {
Ok(())
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
})
}
Expand All @@ -96,7 +96,7 @@ impl<K: Key> KV<K> for Database<K> {
if error == ptr::null_mut() {
Ok(())
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
})
}
Expand All @@ -119,7 +119,7 @@ impl<K: Key> KV<K> for Database<K> {
if error == ptr::null_mut() {
Ok(Bytes::from_raw(result as *mut u8, length))
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
})
}
Expand Down
9 changes: 5 additions & 4 deletions src/database/management.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use error::Error;
use std::ffi::CString;
use std::ptr;
use std::path::Path;
use libc::c_char;

use leveldb_sys::{leveldb_destroy_db, leveldb_repair_db};

Expand All @@ -14,13 +15,13 @@ pub fn destroy(name: &Path, options: Options) -> Result<(), Error> {
let c_string = CString::new(name.to_str().unwrap()).unwrap();
let c_options = c_options(&options, None);
leveldb_destroy_db(c_options,
c_string.as_bytes_with_nul().as_ptr() as *const i8,
c_string.as_bytes_with_nul().as_ptr() as *const c_char,
&mut error);

if error == ptr::null_mut() {
Ok(())
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
}
}
Expand All @@ -32,13 +33,13 @@ pub fn repair(name: &Path, options: Options) -> Result<(), Error> {
let c_string = CString::new(name.to_str().unwrap()).unwrap();
let c_options = c_options(&options, None);
leveldb_repair_db(c_options,
c_string.as_bytes_with_nul().as_ptr() as *const i8,
c_string.as_bytes_with_nul().as_ptr() as *const c_char,
&mut error);

if error == ptr::null_mut() {
Ok(())
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
}
}
20 changes: 15 additions & 5 deletions src/database/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ use leveldb_sys::*;
use self::options::{Options, c_options};
use self::error::Error;
use std::ffi::CString;
use libc::c_char;
#[cfg(unix)]
use std::os::unix::ffi::OsStrExt;

use std::path::Path;

Expand Down Expand Up @@ -107,17 +110,24 @@ impl<K: Key> Database<K> {
pub fn open(name: &Path, options: Options) -> Result<Database<K>, Error> {
let mut error = ptr::null_mut();
unsafe {
let c_string = CString::new(name.to_str().unwrap()).unwrap();
#[cfg(unix)]
let c_string = CString::new(name.as_os_str().as_bytes()).unwrap();
#[cfg(not(unix))]
let c_string = CString::new(
name.to_str()
.ok_or_else(|| Error::new("Path is not valid Unicode".into()))?,
)
.unwrap();
let c_options = c_options(&options, None);
let db = leveldb_open(c_options as *const leveldb_options_t,
c_string.as_bytes_with_nul().as_ptr() as *const i8,
c_string.as_bytes_with_nul().as_ptr() as *const c_char,
&mut error);
leveldb_options_destroy(c_options);

if error == ptr::null_mut() {
Ok(Database::new(db, options, None))
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
}
}
Expand All @@ -140,14 +150,14 @@ impl<K: Key> Database<K> {
let c_string = CString::new(name.to_str().unwrap()).unwrap();
let c_options = c_options(&options, Some(comp_ptr));
let db = leveldb_open(c_options as *const leveldb_options_t,
c_string.as_bytes_with_nul().as_ptr() as *const i8,
c_string.as_bytes_with_nul().as_ptr() as *const c_char,
&mut error);
leveldb_options_destroy(c_options);

if error == ptr::null_mut() {
Ok(Database::new(db, options, Some(comp_ptr)))
} else {
Err(Error::new_from_i8(error))
Err(Error::new_from_char(error))
}
}
}
Expand Down