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

@@ -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
+    })
+}

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;

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::str;
-use std::fmt;
 
 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<P: AsRef<Path>>(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);
+            ffi_try!(ffi::rocksdb_destroy_db(opts.inner, cpath.as_ptr()));
-        }
-        if !err.is_null() {
-            return Err(Error::new(error_message(err)));
         }
         Ok(())
     }
 
     pub fn repair<P: AsRef<Path>>(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);
+            ffi_try!(ffi::rocksdb_repair_db(opts.inner, cpath.as_ptr()));
-        }
-        if !err.is_null() {
-            return Err(Error::new(error_message(err)));
         }
         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);
+            ffi_try!(ffi::rocksdb_write(self.inner, writeopts.inner, batch.inner));
-        }
-        if !err.is_null() {
-            return Err(Error::new(error_message(err)));
         }
         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_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;
-            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)));
-            }
             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_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;
-            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)));
-            }
             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);
+            ffi_try!(ffi::rocksdb_drop_column_family(self.inner, *cf.unwrap()));
-        }
-        if !err.is_null() {
-            return Err(Error::new(error_message(err)));
         }
         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_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));
-            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)));
-            }
             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_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));
-            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)));
-            }
             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_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));
-            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)));
-            }
             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_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));
-            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)));
-            }
             Ok(())
         }
     }
 
     fn delete_opt(&self, key: &[u8], writeopts: &WriteOptions) -> Result<(), Error> {
         unsafe {
-            let mut err: *mut c_char = ptr::null_mut();
+            ffi_try!(ffi::rocksdb_delete(self.inner, writeopts.inner, key.as_ptr() as *const c_char, key.len() as size_t));
-            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)));
-            }
             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_try!(ffi::rocksdb_delete_cf(self.inner, writeopts.inner, cf, key.as_ptr() as *const c_char, key.len() as size_t));
-            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)));
-            }
             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();