address comment

librocksdb_sys/src/lib.rs

@@ -17,7 +17,7 @@ extern crate libc;
 #[cfg(test)]
 extern crate tempdir;
 
-use libc::{c_char, c_uchar, c_int, c_void, size_t, uint8_t, uint64_t, c_double};
+use libc::{c_char, c_uchar, c_int, c_void, size_t, uint8_t, uint32_t, uint64_t, c_double};
 use std::ffi::CStr;
 
 pub enum DBOptions {}
@@ -183,6 +183,31 @@ pub enum DBInfoLogLevel {
     DBNumInfoLog = 6,
 }
 
+#[repr(C)]
+pub struct DBTablePropertiesCollectorContext {
+    pub collector: *mut c_void,
+    pub name: extern "C" fn(*mut c_void) -> *const c_char,
+    pub destructor: extern "C" fn(*mut c_void),
+    pub add_userkey: extern "C" fn(*mut c_void,
+                                   *const uint8_t,
+                                   size_t,
+                                   *const uint8_t,
+                                   size_t,
+                                   c_int,
+                                   uint64_t,
+                                   uint64_t),
+    pub finish: extern "C" fn(*mut c_void, *mut c_void),
+    pub readable_properties: extern "C" fn(*mut c_void, *mut c_void),
+}
+
+#[repr(C)]
+pub struct DBTablePropertiesCollectorFactoryContext {
+    pub factory: *mut c_void,
+    pub name: extern "C" fn(*mut c_void) -> *const c_char,
+    pub destructor: extern "C" fn(*mut c_void),
+    pub create_table_properties_collector: extern "C" fn(*mut c_void, uint32_t) -> *mut c_void,
+}
+
 pub fn error_message(ptr: *mut c_char) -> String {
     let c_str = unsafe { CStr::from_ptr(ptr) };
     let s = format!("{}", c_str.to_string_lossy());
@@ -827,7 +852,7 @@ extern "C" {
                                                   value_len: size_t);
 
     pub fn crocksdb_table_properties_collector_factory_create
-        (context: *mut c_void)
+        (context: *mut DBTablePropertiesCollectorFactoryContext)
          -> *mut DBTablePropertiesCollectorFactory;
 
     pub fn crocksdb_table_properties_collector_factory_destroy(

src/rocksdb_options.rs

@@ -384,8 +384,7 @@ impl Options {
     pub fn add_table_properties_collector_factory(&mut self,
                                                   factory: Box<TablePropertiesCollectorFactory>) {
         unsafe {
-            let context = new_table_properties_collector_factory_context(factory);
-            let context = Box::into_raw(context) as *mut c_void;
+            let context = Box::into_raw(new_table_properties_collector_factory_context(factory));
             let f = crocksdb_ffi::crocksdb_table_properties_collector_factory_create(context);
             crocksdb_ffi::crocksdb_options_add_table_properties_collector_factory(self.inner, f);
         }

src/table_properties.rs

@@ -55,9 +55,11 @@ impl CShallowStringsMap {
             let keys_lens = slice::from_raw_parts(self.keys_lens, self.size);
             let values = slice::from_raw_parts(self.values, self.size);
             let values_lens = slice::from_raw_parts(self.values_lens, self.size);
-            for i in 0..self.size {
-                let k = slice::from_raw_parts(keys[i] as *const u8, keys_lens[i]);
-                let v = slice::from_raw_parts(values[i] as *const u8, values_lens[i]);
+            for ((k, klen), (v, vlen)) in keys.iter()
+                .zip(keys_lens.iter())
+                .zip(values.iter().zip(values_lens.iter())) {
+                let k = slice::from_raw_parts(*k as *const u8, *klen);
+                let v = slice::from_raw_parts(*v as *const u8, *vlen);
                 res.insert(k.to_owned(), v.to_owned());
             }
         }

src/table_properties_collector.rs

-use crocksdb_ffi::{self, DBEntryType};
+use crocksdb_ffi::{self, DBEntryType, DBTablePropertiesCollectorContext};
 use libc::{c_void, c_char, c_int, uint8_t, uint64_t, size_t};
 use std::collections::HashMap;
 use std::mem;
 use std::slice;
 
+/// `TablePropertiesCollector` provides the mechanism for users to collect
+/// their own properties that they are interested in. This class is essentially
+/// a collection of callback functions that will be invoked during table
+/// building. It is construced with TablePropertiesCollectorFactory. The methods
+/// 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_userkey(&mut self, key: &[u8], value: &[u8], entry_type: DBEntryType);
+
+    /// Will be called when a table has already been built and is ready for
+    /// writing the properties block.
     fn finish(&mut self) -> HashMap<Vec<u8>, Vec<u8>>;
-    fn readable_properties(&self) -> HashMap<String, String>;
-}
 
-#[repr(C)]
-pub struct TablePropertiesCollectorContext {
-    collector: *mut c_void,
-    name: extern "C" fn(*mut c_void) -> *const c_char,
-    destructor: extern "C" fn(*mut c_void),
-    add_userkey: extern "C" fn(*mut c_void,
-                               *const uint8_t,
-                               size_t,
-                               *const uint8_t,
-                               size_t,
-                               c_int,
-                               uint64_t,
-                               uint64_t),
-    finish: extern "C" fn(*mut c_void, *mut c_void),
-    readable_properties: extern "C" fn(*mut c_void, *mut c_void),
+    /// Return the human-readable properties, where the key is property name and
+    /// the value is the human-readable form of value.
+    fn readable_properties(&self) -> HashMap<String, String>;
 }
 
 extern "C" fn name(context: *mut c_void) -> *const c_char {
     unsafe {
-        let context = &mut *(context as *mut TablePropertiesCollectorContext);
+        let context = &mut *(context as *mut DBTablePropertiesCollectorContext);
         let collector = &mut *(context.collector as *mut Box<TablePropertiesCollector>);
         collector.name().as_ptr() as *const c_char
     }
@@ -38,7 +36,7 @@ extern "C" fn name(context: *mut c_void) -> *const c_char {
 
 extern "C" fn destructor(context: *mut c_void) {
     unsafe {
-        let context = Box::from_raw(context as *mut TablePropertiesCollectorContext);
+        let context = Box::from_raw(context as *mut DBTablePropertiesCollectorContext);
         Box::from_raw(context.collector as *mut Box<TablePropertiesCollector>);
     }
 }
@@ -52,7 +50,7 @@ pub extern "C" fn add_userkey(context: *mut c_void,
                               _: uint64_t,
                               _: uint64_t) {
     unsafe {
-        let context = &mut *(context as *mut TablePropertiesCollectorContext);
+        let context = &mut *(context as *mut DBTablePropertiesCollectorContext);
         let collector = &mut *(context.collector as *mut Box<TablePropertiesCollector>);
         let key = slice::from_raw_parts(key, key_len);
         let value = slice::from_raw_parts(value, value_len);
@@ -62,7 +60,7 @@ pub extern "C" fn add_userkey(context: *mut c_void,
 
 pub extern "C" fn finish(context: *mut c_void, props: *mut c_void) {
     unsafe {
-        let context = &mut *(context as *mut TablePropertiesCollectorContext);
+        let context = &mut *(context as *mut DBTablePropertiesCollectorContext);
         let collector = &mut *(context.collector as *mut Box<TablePropertiesCollector>);
         for (key, value) in collector.finish() {
             crocksdb_ffi::crocksdb_user_collected_properties_add(props,
@@ -76,7 +74,7 @@ pub extern "C" fn finish(context: *mut c_void, props: *mut c_void) {
 
 pub extern "C" fn readable_properties(context: *mut c_void, props: *mut c_void) {
     unsafe {
-        let context = &mut *(context as *mut TablePropertiesCollectorContext);
+        let context = &mut *(context as *mut DBTablePropertiesCollectorContext);
         let collector = &mut *(context.collector as *mut Box<TablePropertiesCollector>);
         for (key, value) in collector.readable_properties() {
             crocksdb_ffi::crocksdb_user_collected_properties_add(props,
@@ -89,8 +87,8 @@ pub extern "C" fn readable_properties(context: *mut c_void, props: *mut c_void)
 }
 
 pub unsafe fn new_table_properties_collector_context(collector: Box<TablePropertiesCollector>)
-                                                     -> Box<TablePropertiesCollectorContext> {
-    Box::new(TablePropertiesCollectorContext {
+                                                     -> Box<DBTablePropertiesCollectorContext> {
+    Box::new(DBTablePropertiesCollectorContext {
         collector: Box::into_raw(Box::new(collector)) as *mut c_void,
         name: name,
         destructor: destructor,

src/table_properties_collector_factory.rs

+use crocksdb_ffi::DBTablePropertiesCollectorFactoryContext;
 use libc::{c_void, c_char, uint32_t};
 use table_properties_collector::{TablePropertiesCollector, new_table_properties_collector_context};
 
+/// 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>;
 }
 
-#[repr(C)]
-pub struct TablePropertiesCollectorFactoryContext {
-    factory: *mut c_void,
-    name: extern "C" fn(*mut c_void) -> *const c_char,
-    destructor: extern "C" fn(*mut c_void),
-    create_table_properties_collector: extern "C" fn(*mut c_void, uint32_t) -> *mut c_void,
-}
-
 extern "C" fn name(context: *mut c_void) -> *const c_char {
     unsafe {
-        let context = &mut *(context as *mut TablePropertiesCollectorFactoryContext);
+        let context = &mut *(context as *mut DBTablePropertiesCollectorFactoryContext);
         let factory = &mut *(context.factory as *mut Box<TablePropertiesCollectorFactory>);
         factory.name().as_ptr() as *const c_char
     }
@@ -24,14 +21,14 @@ extern "C" fn name(context: *mut c_void) -> *const c_char {
 
 extern "C" fn destructor(context: *mut c_void) {
     unsafe {
-        let context = Box::from_raw(context as *mut TablePropertiesCollectorFactoryContext);
+        let context = Box::from_raw(context as *mut DBTablePropertiesCollectorFactoryContext);
         Box::from_raw(context.factory as *mut Box<TablePropertiesCollectorFactory>);
     }
 }
 
 extern "C" fn create_table_properties_collector(context: *mut c_void, cf: uint32_t) -> *mut c_void {
     unsafe {
-        let context = &mut *(context as *mut TablePropertiesCollectorFactoryContext);
+        let context = &mut *(context as *mut DBTablePropertiesCollectorFactoryContext);
         let factory = &mut *(context.factory as *mut Box<TablePropertiesCollectorFactory>);
         let collector = factory.create_table_properties_collector(cf);
         Box::into_raw(new_table_properties_collector_context(collector)) as *mut c_void
@@ -40,8 +37,8 @@ extern "C" fn create_table_properties_collector(context: *mut c_void, cf: uint32
 
 pub unsafe fn new_table_properties_collector_factory_context
     (factory: Box<TablePropertiesCollectorFactory>)
-     -> Box<TablePropertiesCollectorFactoryContext> {
-    Box::new(TablePropertiesCollectorFactoryContext {
+     -> Box<DBTablePropertiesCollectorFactoryContext> {
+    Box::new(DBTablePropertiesCollectorFactoryContext {
         factory: Box::into_raw(Box::new(factory)) as *mut c_void,
         name: name,
         destructor: destructor,