From 364f3abd49dceb851c18dac023b73d3dcab991b4 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Mon, 7 Nov 2016 23:23:12 +0000 Subject: [PATCH] Added `ffi_try!` macro and adapted native calls to use it. `ffi_try!` simplifies calls to native functions with an error pointer as their last parameter. --- src/ffi_util.rs | 38 +++++++++++++++++ src/lib.rs | 3 ++ src/rocksdb.rs | 109 ++++++++++-------------------------------------- 3 files changed, 63 insertions(+), 87 deletions(-) create mode 100644 src/ffi_util.rs diff --git a/src/ffi_util.rs b/src/ffi_util.rs new file mode 100644 index 0000000..6844a61 --- /dev/null +++ b/src/ffi_util.rs @@ -0,0 +1,38 @@ +// Copyright 2016 Alex Regueiro +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +use libc::{self, c_char, c_void}; +use std::ffi::{CStr}; +use std::str; + +pub fn error_message(ptr: *const c_char) -> String { + let cstr = unsafe { CStr::from_ptr(ptr as *const _) }; + let s = str::from_utf8(cstr.to_bytes()).unwrap().to_owned(); + unsafe { + libc::free(ptr as *mut c_void); + } + s +} + +macro_rules! ffi_try { + ( $($function:ident)::*( $( $arg:expr ),* ) ) => ({ + let mut err: *mut ::libc::c_char = ::std::ptr::null_mut(); + let result = $($function)::*($($arg),*, &mut err); + if !err.is_null() { + return Err(Error::new($crate::ffi_util::error_message(err))); + } + result + }) +} diff --git a/src/lib.rs b/src/lib.rs index 7faeb2f..893489a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,9 @@ extern crate libc; extern crate rocksdb_sys as ffi; +#[macro_use] +mod ffi_util; + pub mod comparator; pub mod merge_operator; mod rocksdb; diff --git a/src/rocksdb.rs b/src/rocksdb.rs index 7c105db..40fcc1f 100644 --- a/src/rocksdb.rs +++ b/src/rocksdb.rs @@ -14,29 +14,20 @@ // use std::collections::BTreeMap; -use std::ffi::{CStr, CString}; +use std::ffi::{CString}; +use std::fmt; use std::fs; use std::ops::Deref; use std::path::{Path, PathBuf}; use std::ptr; use std::slice; -use std::str::from_utf8; -use std::fmt; +use std::str; use libc::{self, c_char, c_uchar, c_int, c_void, size_t}; use {Options, WriteOptions}; use ffi; -fn error_message(ptr: *const i8) -> String { - let c_str = unsafe { CStr::from_ptr(ptr as *const _) }; - let s = from_utf8(c_str.to_bytes()).unwrap().to_owned(); - unsafe { - libc::free(ptr as *mut c_void); - } - s -} - pub fn new_bloom_filter(bits: c_int) -> *mut ffi::rocksdb_filterpolicy_t { unsafe { ffi::rocksdb_filterpolicy_create_bloom(bits) } } @@ -317,13 +308,12 @@ impl DB { return Err(Error::new(format!("Failed to create rocksdb directory: {:?}", e))); } - let mut err: *mut c_char = ptr::null_mut(); let db: *mut ffi::rocksdb_t; let mut cf_map = BTreeMap::new(); if cfs.len() == 0 { unsafe { - db = ffi::rocksdb_open(opts.inner, cpath_ptr as *const _, &mut err); + db = ffi_try!(ffi::rocksdb_open(opts.inner, cpath_ptr as *const _)); } } else { let mut cfs_v = cfs.to_vec(); @@ -344,7 +334,7 @@ impl DB { let cfopts: Vec<_> = cfs_v.iter().map(|_| unsafe { ffi::rocksdb_options_create() as *const _ }).collect(); unsafe { - db = ffi::rocksdb_open_column_families(opts.inner, cpath_ptr as *const _, cfs_v.len() as c_int, cfnames.as_ptr() as *const _, cfopts.as_ptr(), cfhandles.as_mut_ptr(), &mut err); + db = ffi_try!(ffi::rocksdb_open_column_families(opts.inner, cpath_ptr as *const _, cfs_v.len() as c_int, cfnames.as_ptr() as *const _, cfopts.as_ptr(), cfhandles.as_mut_ptr())); } for handle in &cfhandles { @@ -358,9 +348,6 @@ impl DB { } } - if !err.is_null() { - return Err(Error::new(error_message(err))); - } if db.is_null() { return Err(Error::new("Could not initialize database.".to_owned())); } @@ -374,24 +361,16 @@ impl DB { pub fn destroy>(opts: &Options, path: P) -> Result<(), Error> { let cpath = CString::new(path.as_ref().to_string_lossy().as_bytes()).unwrap(); - let mut err: *mut c_char = ptr::null_mut(); unsafe { - ffi::rocksdb_destroy_db(opts.inner, cpath.as_ptr(), &mut err); - } - if !err.is_null() { - return Err(Error::new(error_message(err))); + ffi_try!(ffi::rocksdb_destroy_db(opts.inner, cpath.as_ptr())); } Ok(()) } pub fn repair>(opts: Options, path: P) -> Result<(), Error> { let cpath = CString::new(path.as_ref().to_string_lossy().as_bytes()).unwrap(); - let mut err: *mut c_char = ptr::null_mut(); unsafe { - ffi::rocksdb_repair_db(opts.inner, cpath.as_ptr(), &mut err); - } - if !err.is_null() { - return Err(Error::new(error_message(err))); + ffi_try!(ffi::rocksdb_repair_db(opts.inner, cpath.as_ptr())); } Ok(()) } @@ -401,12 +380,8 @@ impl DB { } pub fn write_opt(&self, batch: WriteBatch, writeopts: &WriteOptions) -> Result<(), Error> { - let mut err: *mut c_char = ptr::null_mut(); unsafe { - ffi::rocksdb_write(self.inner, writeopts.inner, batch.inner, &mut err); - } - if !err.is_null() { - return Err(Error::new(error_message(err))); + ffi_try!(ffi::rocksdb_write(self.inner, writeopts.inner, batch.inner)); } Ok(()) } @@ -428,11 +403,7 @@ impl DB { unsafe { let mut val_len: size_t = 0; - let mut err: *mut c_char = ptr::null_mut(); - let val = ffi::rocksdb_get(self.inner, readopts.inner, key.as_ptr() as *const c_char, key.len() as size_t, &mut val_len, &mut err) as *mut u8; - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + let val = ffi_try!(ffi::rocksdb_get(self.inner, readopts.inner, key.as_ptr() as *const c_char, key.len() as size_t, &mut val_len)) as *mut u8; if val.is_null() { Ok(None) } else { @@ -453,11 +424,7 @@ impl DB { unsafe { let mut val_len: size_t = 0; - let mut err: *mut c_char = ptr::null_mut(); - let val = ffi::rocksdb_get_cf(self.inner, readopts.inner, cf, key.as_ptr() as *const c_char, key.len() as size_t, &mut val_len, &mut err) as *mut u8; - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + let val = ffi_try!(ffi::rocksdb_get_cf(self.inner, readopts.inner, cf, key.as_ptr() as *const c_char, key.len() as size_t, &mut val_len)) as *mut u8; if val.is_null() { Ok(None) } else { @@ -477,15 +444,11 @@ impl DB { return Err(Error::new("Failed to convert path to CString when opening rocksdb".to_owned())) } }; - let mut err: *mut c_char = ptr::null_mut(); let cf_handler = unsafe { - let cf_handler = ffi::rocksdb_create_column_family(self.inner, opts.inner, cname.as_ptr(), &mut err); + let cf_handler = ffi_try!(ffi::rocksdb_create_column_family(self.inner, opts.inner, cname.as_ptr())); self.cfs.insert(name.to_string(), cf_handler); cf_handler }; - if !err.is_null() { - return Err(Error::new(error_message(err))); - } Ok(cf_handler) } @@ -494,12 +457,8 @@ impl DB { if cf.is_none() { return Err(Error::new(format!("Invalid column family: {}", name).to_owned())); } - let mut err: *mut c_char = ptr::null_mut(); unsafe { - ffi::rocksdb_drop_column_family(self.inner, *cf.unwrap(), &mut err); - } - if !err.is_null() { - return Err(Error::new(error_message(err))); + ffi_try!(ffi::rocksdb_drop_column_family(self.inner, *cf.unwrap())); } Ok(()) } @@ -525,66 +484,42 @@ impl DB { pub fn put_opt(&self, key: &[u8], value: &[u8], writeopts: &WriteOptions) -> Result<(), Error> { unsafe { - let mut err: *mut c_char = ptr::null_mut(); - ffi::rocksdb_put(self.inner, writeopts.inner, key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, value.len() as size_t, &mut err); - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + ffi_try!(ffi::rocksdb_put(self.inner, writeopts.inner, key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, value.len() as size_t)); Ok(()) } } pub fn put_cf_opt(&self, cf: *mut ffi::rocksdb_column_family_handle_t, key: &[u8], value: &[u8], writeopts: &WriteOptions) -> Result<(), Error> { unsafe { - let mut err: *mut c_char = ptr::null_mut(); - ffi::rocksdb_put_cf(self.inner, writeopts.inner, cf, key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, value.len() as size_t, &mut err); - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + ffi_try!(ffi::rocksdb_put_cf(self.inner, writeopts.inner, cf, key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, value.len() as size_t)); Ok(()) } } pub fn merge_opt(&self, key: &[u8], value: &[u8], writeopts: &WriteOptions) -> Result<(), Error> { unsafe { - let mut err: *mut c_char = ptr::null_mut(); - ffi::rocksdb_merge(self.inner, writeopts.inner, key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, value.len() as size_t, &mut err); - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + ffi_try!(ffi::rocksdb_merge(self.inner, writeopts.inner, key.as_ptr() as *const c_char, key.len() as size_t, value.as_ptr() as *const c_char, value.len() as size_t)); Ok(()) } } fn merge_cf_opt(&self, cf: *mut ffi::rocksdb_column_family_handle_t, key: &[u8], value: &[u8], writeopts: &WriteOptions) -> Result<(), Error> { unsafe { - let mut err: *mut c_char = ptr::null_mut(); - ffi::rocksdb_merge_cf(self.inner, writeopts.inner, cf, key.as_ptr() as *const i8, key.len() as size_t, value.as_ptr() as *const i8, value.len() as size_t, &mut err); - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + ffi_try!(ffi::rocksdb_merge_cf(self.inner, writeopts.inner, cf, key.as_ptr() as *const i8, key.len() as size_t, value.as_ptr() as *const i8, value.len() as size_t)); Ok(()) } } fn delete_opt(&self, key: &[u8], writeopts: &WriteOptions) -> Result<(), Error> { unsafe { - let mut err: *mut c_char = ptr::null_mut(); - ffi::rocksdb_delete(self.inner, writeopts.inner, key.as_ptr() as *const c_char, key.len() as size_t, &mut err); - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + ffi_try!(ffi::rocksdb_delete(self.inner, writeopts.inner, key.as_ptr() as *const c_char, key.len() as size_t)); Ok(()) } } fn delete_cf_opt(&self, cf: *mut ffi::rocksdb_column_family_handle_t, key: &[u8], writeopts: &WriteOptions) -> Result<(), Error> { unsafe { - let mut err: *mut c_char = ptr::null_mut(); - ffi::rocksdb_delete_cf(self.inner, writeopts.inner, cf, key.as_ptr() as *const c_char, key.len() as size_t, &mut err); - if !err.is_null() { - return Err(Error::new(error_message(err))); - } + ffi_try!(ffi::rocksdb_delete_cf(self.inner, writeopts.inner, cf, key.as_ptr() as *const c_char, key.len() as size_t)); Ok(()) } } @@ -774,7 +709,7 @@ impl DBVector { } pub fn to_utf8(&self) -> Option<&str> { - from_utf8(self.deref()).ok() + str::from_utf8(self.deref()).ok() } } @@ -798,9 +733,9 @@ fn external() { #[test] fn errors_do_stuff() { let path = "_rust_rocksdb_error"; - let db = DB::open_default(path).unwrap(); + let _db = DB::open_default(path).unwrap(); let opts = Options::default(); - // The DB will still be open when we try to destroy and the lock should fail + // The DB will still be open when we try to destroy it and the lock should fail. match DB::destroy(&opts, path) { Err(s) => { assert!(s == Error::new("IO error: lock _rust_rocksdb_error/LOCK: No locks available".to_owned())) @@ -857,7 +792,7 @@ fn iterator_test() { assert!(p.is_ok()); let iter = db.iterator(IteratorMode::Start); for (k, v) in iter { - println!("Hello {}: {}", from_utf8(&*k).unwrap(), from_utf8(&*v).unwrap()); + println!("Hello {}: {}", str::from_utf8(&*k).unwrap(), str::from_utf8(&*v).unwrap()); } } let opts = Options::default();