address comment

librocksdb_sys/crocksdb/c.cc

@@ -2972,17 +2972,13 @@ void crocksdb_user_collected_properties_iter_next(
 
 const char* crocksdb_user_collected_properties_iter_key(
   crocksdb_user_collected_properties_iterator_t* it, size_t* klen) {
-  if (klen) {
-    *klen = it->cur_->first.size();
-  }
+  *klen = it->cur_->first.size();
   return it->cur_->first.data();
 }
 
 const char* crocksdb_user_collected_properties_iter_value(
   crocksdb_user_collected_properties_iterator_t* it, size_t* vlen) {
-  if (vlen) {
-    *vlen = it->cur_->second.size();
-  }
+  *vlen = it->cur_->second.size();
   return it->cur_->second.data();
 }
 
@@ -3111,9 +3107,7 @@ void crocksdb_table_properties_collection_iter_next(
 
 const char* crocksdb_table_properties_collection_iter_key(
   crocksdb_table_properties_collection_iterator_t* it, size_t* klen) {
-  if (klen) {
-    *klen = it->cur_->first.size();
-  }
+  *klen = it->cur_->first.size();
   return it->cur_->first.data();
 }
 

src/rocksdb.rs

@@ -26,7 +26,7 @@ use std::fmt::{self, Debug, Formatter};
 use std::ops::Deref;
 use std::path::Path;
 use std::str::from_utf8;
-use table_properties::TablePropertiesCollection;
+use table_properties::{TablePropertiesCollection, TablePropertiesCollectionHandle};
 
 const DEFAULT_COLUMN_FAMILY: &'static str = "default";
 
@@ -1047,9 +1047,9 @@ impl DB {
 
     pub fn get_properties_of_all_tables(&self) -> Result<TablePropertiesCollection, String> {
         unsafe {
-            let props = TablePropertiesCollection::new();
-            ffi_try!(crocksdb_get_properties_of_all_tables(self.inner, props.inner));
-            Ok(props)
+            let handle = TablePropertiesCollectionHandle::new();
+            ffi_try!(crocksdb_get_properties_of_all_tables(self.inner, handle.inner));
+            Ok(TablePropertiesCollection::new(handle))
         }
     }
 
@@ -1057,9 +1057,9 @@ impl DB {
                                            cf: &CFHandle)
                                            -> Result<TablePropertiesCollection, String> {
         unsafe {
-            let props = TablePropertiesCollection::new();
-            ffi_try!(crocksdb_get_properties_of_all_tables_cf(self.inner, cf.inner, props.inner));
-            Ok(props)
+            let handle = TablePropertiesCollectionHandle::new();
+            ffi_try!(crocksdb_get_properties_of_all_tables_cf(self.inner, cf.inner, handle.inner));
+            Ok(TablePropertiesCollection::new(handle))
         }
     }
 
@@ -1072,7 +1072,7 @@ impl DB {
         let limit_keys: Vec<*const u8> = ranges.iter().map(|x| x.end_key.as_ptr()).collect();
         let limit_keys_lens: Vec<_> = ranges.iter().map(|x| x.end_key.len()).collect();
         unsafe {
-            let props = TablePropertiesCollection::new();
+            let handle = TablePropertiesCollectionHandle::new();
             ffi_try!(crocksdb_get_properties_of_tables_in_range(self.inner,
                                                                 cf.inner,
                                                                 ranges.len() as i32,
@@ -1080,8 +1080,8 @@ impl DB {
                                                                 start_keys_lens.as_ptr(),
                                                                 limit_keys.as_ptr(),
                                                                 limit_keys_lens.as_ptr(),
-                                                                props.inner));
-            Ok(props)
+                                                                handle.inner));
+            Ok(TablePropertiesCollection::new(handle))
         }
     }
 }

src/rocksdb_options.rs

@@ -382,9 +382,10 @@ impl Options {
     }
 
     pub fn add_table_properties_collector_factory(&mut self,
+                                                  fname: &str,
                                                   factory: Box<TablePropertiesCollectorFactory>) {
         unsafe {
-            let f = new_table_properties_collector_factory(factory);
+            let f = new_table_properties_collector_factory(fname, factory);
             crocksdb_ffi::crocksdb_options_add_table_properties_collector_factory(self.inner, f);
         }
     }

src/table_properties.rs

@@ -11,20 +11,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crocksdb_ffi::{self, DBTableProperties, DBTableProperty, DBUserCollectedProperties,
-                   DBUserCollectedPropertiesIterator, DBTablePropertiesCollection,
-                   DBTablePropertiesCollectionIterator};
+use crocksdb_ffi::{self, DBTableProperties, DBTableProperty, DBUserCollectedPropertiesIterator,
+                   DBTablePropertiesCollection, DBTablePropertiesCollectionIterator};
 use libc::size_t;
 use std::collections::HashMap;
 use std::marker::PhantomData;
 use std::slice;
 use std::str;
 
-pub struct TablePropertiesCollection {
+pub struct TablePropertiesCollectionHandle {
     pub inner: *mut DBTablePropertiesCollection,
 }
 
-impl Drop for TablePropertiesCollection {
+impl Drop for TablePropertiesCollectionHandle {
     fn drop(&mut self) {
         unsafe {
             crocksdb_ffi::crocksdb_table_properties_collection_destroy(self.inner);
@@ -32,18 +31,28 @@ impl Drop for TablePropertiesCollection {
     }
 }
 
-impl TablePropertiesCollection {
-    pub fn new() -> TablePropertiesCollection {
+impl TablePropertiesCollectionHandle {
+    pub fn new() -> TablePropertiesCollectionHandle {
         unsafe {
-            TablePropertiesCollection {
+            TablePropertiesCollectionHandle {
                 inner: crocksdb_ffi::crocksdb_table_properties_collection_create(),
             }
         }
     }
+}
+
+pub struct TablePropertiesCollection {
+    handle: TablePropertiesCollectionHandle,
+}
+
+impl TablePropertiesCollection {
+    pub fn new(handle: TablePropertiesCollectionHandle) -> TablePropertiesCollection {
+        TablePropertiesCollection { handle: handle }
+    }
 
     pub fn collect(&self) -> HashMap<&str, TableProperties> {
         let mut res = HashMap::new();
-        let mut iter = TablePropertiesCollectionIter::new(self, self.inner);
+        let mut iter = TablePropertiesCollectionIter::new(self);
         while iter.valid() {
             res.insert(iter.key(), iter.value());
             iter.next();
@@ -66,10 +75,9 @@ impl<'a> Drop for TablePropertiesCollectionIter<'a> {
 }
 
 impl<'a> TablePropertiesCollectionIter<'a> {
-    fn new(_: &'a TablePropertiesCollection,
-           inner: *mut DBTablePropertiesCollection)
-           -> TablePropertiesCollectionIter<'a> {
+    fn new(props: &'a TablePropertiesCollection) -> TablePropertiesCollectionIter<'a> {
         unsafe {
+            let inner = props.handle.inner;
             TablePropertiesCollectionIter {
                 props: PhantomData,
                 inner: crocksdb_ffi::crocksdb_table_properties_collection_iter_create(inner),
@@ -206,13 +214,10 @@ impl TableProperties {
 
     pub fn user_collected_properties(&self) -> HashMap<&[u8], &[u8]> {
         let mut res = HashMap::new();
-        unsafe {
-            let inner = crocksdb_ffi::crocksdb_table_properties_get_user_properties(self.inner);
-            let mut iter = UserCollectedPropertiesIter::new(self, inner);
-            while iter.valid() {
-                res.insert(iter.key(), iter.value());
-                iter.next();
-            }
+        let mut iter = UserCollectedPropertiesIter::new(self);
+        while iter.valid() {
+            res.insert(iter.key(), iter.value());
+            iter.next();
         }
         res
     }
@@ -232,10 +237,9 @@ impl<'a> Drop for UserCollectedPropertiesIter<'a> {
 }
 
 impl<'a> UserCollectedPropertiesIter<'a> {
-    fn new(_: &'a TableProperties,
-           inner: *mut DBUserCollectedProperties)
-           -> UserCollectedPropertiesIter<'a> {
+    fn new(props: &'a TableProperties) -> UserCollectedPropertiesIter<'a> {
         unsafe {
+            let inner = crocksdb_ffi::crocksdb_table_properties_get_user_properties(props.inner);
             UserCollectedPropertiesIter {
                 props: PhantomData,
                 inner: crocksdb_ffi::crocksdb_user_collected_properties_iter_create(inner),

src/table_properties_collector.rs

@@ -25,9 +25,6 @@ use std::slice;
 /// don't need to be thread-safe, as we will create exactly one
 /// TablePropertiesCollector object per table and then call it sequentially
 pub trait TablePropertiesCollector {
-    /// The name of the properties collector.
-    fn name(&self) -> &str;
-
     /// Will be called when a new key/value pair is inserted into the table.
     fn add(&mut self,
            key: &[u8],
@@ -47,10 +44,10 @@ struct TablePropertiesCollectorHandle {
 }
 
 impl TablePropertiesCollectorHandle {
-    fn new(collector: Box<TablePropertiesCollector>) -> TablePropertiesCollectorHandle {
+    fn new(name: &str, rep: Box<TablePropertiesCollector>) -> TablePropertiesCollectorHandle {
         TablePropertiesCollectorHandle {
-            name: CString::new(collector.name()).unwrap(),
-            rep: collector,
+            name: CString::new(name).unwrap(),
+            rep: rep,
         }
     }
 }
@@ -98,9 +95,10 @@ pub extern "C" fn finish(handle: *mut c_void, props: *mut DBUserCollectedPropert
     }
 }
 
-pub unsafe fn new_table_properties_collector(collector: Box<TablePropertiesCollector>)
+pub unsafe fn new_table_properties_collector(cname: &str,
+                                             collector: Box<TablePropertiesCollector>)
                                              -> *mut DBTablePropertiesCollector {
-    let handle = TablePropertiesCollectorHandle::new(collector);
+    let handle = TablePropertiesCollectorHandle::new(cname, collector);
     crocksdb_ffi::crocksdb_table_properties_collector_create(
         Box::into_raw(Box::new(handle)) as *mut c_void,
         name,

src/table_properties_collector_factory.rs

@@ -19,8 +19,6 @@ use table_properties_collector::{TablePropertiesCollector, new_table_properties_
 /// Constructs `TablePropertiesCollector`.
 /// Internals create a new `TablePropertiesCollector` for each new table.
 pub trait TablePropertiesCollectorFactory {
-    /// The name of the properties collector factory.
-    fn name(&self) -> &str;
     /// Has to be thread-safe.
     fn create_table_properties_collector(&mut self, cf: u32) -> Box<TablePropertiesCollector>;
 }
@@ -31,9 +29,11 @@ struct TablePropertiesCollectorFactoryHandle {
 }
 
 impl TablePropertiesCollectorFactoryHandle {
-    fn new(rep: Box<TablePropertiesCollectorFactory>) -> TablePropertiesCollectorFactoryHandle {
+    fn new(name: &str,
+           rep: Box<TablePropertiesCollectorFactory>)
+           -> TablePropertiesCollectorFactoryHandle {
         TablePropertiesCollectorFactoryHandle {
-            name: CString::new(rep.name()).unwrap(),
+            name: CString::new(name).unwrap(),
             rep: rep,
         }
     }
@@ -58,14 +58,14 @@ extern "C" fn create_table_properties_collector(handle: *mut c_void,
     unsafe {
         let handle = &mut *(handle as *mut TablePropertiesCollectorFactoryHandle);
         let collector = handle.rep.create_table_properties_collector(cf);
-        new_table_properties_collector(collector)
+        new_table_properties_collector(handle.name.to_str().unwrap(), collector)
     }
 }
 
 pub unsafe fn new_table_properties_collector_factory
-    (factory: Box<TablePropertiesCollectorFactory>)
+    (fname: &str, factory: Box<TablePropertiesCollectorFactory>)
      -> *mut DBTablePropertiesCollectorFactory {
-    let handle = TablePropertiesCollectorFactoryHandle::new(factory);
+    let handle = TablePropertiesCollectorFactoryHandle::new(fname, factory);
     crocksdb_ffi::crocksdb_table_properties_collector_factory_create(
             Box::into_raw(Box::new(handle)) as *mut c_void,
             name,

tests/test_table_properties.rs

@@ -93,10 +93,6 @@ impl fmt::Display for ExampleCollector {
 }
 
 impl TablePropertiesCollector for ExampleCollector {
-    fn name(&self) -> &str {
-        "example-collector"
-    }
-
     fn add(&mut self, key: &[u8], _: &[u8], entry_type: DBEntryType, _: u64, _: u64) {
         if key.cmp(&self.last_key) != Ordering::Equal {
             self.num_keys += 1;
@@ -126,10 +122,6 @@ impl ExampleFactory {
 }
 
 impl TablePropertiesCollectorFactory for ExampleFactory {
-    fn name(&self) -> &str {
-        "example-factory"
-    }
-
     fn create_table_properties_collector(&mut self, _: u32) -> Box<TablePropertiesCollector> {
         Box::new(ExampleCollector::new())
     }
@@ -145,7 +137,7 @@ fn check_collection(collection: &TablePropertiesCollection,
     let props = collection.collect();
     for (k, v) in &props {
         assert!(k.ends_with(".sst"));
-        assert_eq!(v.property_collectors_names(), "[example-factory]");
+        assert_eq!(v.property_collectors_names(), "[example-collector]");
         res.add(&ExampleCollector::decode(v.user_collected_properties()));
     }
     assert_eq!(props.len(), num_files);
@@ -160,7 +152,7 @@ fn test_table_properties_collector_factory() {
     let f = ExampleFactory::new();
     let mut opts = Options::new();
     opts.create_if_missing(true);
-    opts.add_table_properties_collector_factory(Box::new(f));
+    opts.add_table_properties_collector_factory("example-collector", Box::new(f));
 
     let path = TempDir::new("_rust_rocksdb_collectortest").expect("");
     let db = DB::open(opts, path.path().to_str().unwrap()).unwrap();