Merge pull request #3 from vinipsmaker/fix-utf8-alloc2

fix/helper: Fix dealloc_c_utf8_alloced_from_rust

Author: ustulation

Diff for: src/ffi/directory_details.rs

@@ -29,6 +29,7 @@ use nfs::metadata::directory_metadata::DirectoryMetadata as NfsDirectoryMetadata
 use std::ptr;
 use std::sync::{Arc, Mutex};
 use super::helper;
+use ffi::low_level_api::misc::misc_u8_ptr_free;
 
 /// Details about a directory and its content.
 #[derive(Debug)]
@@ -94,8 +95,10 @@ impl Drop for DirectoryDetails {
 pub struct DirectoryMetadata {
     pub name: *mut u8,
     pub name_len: usize,
+    pub name_cap: usize,
     pub user_metadata: *mut u8,
     pub user_metadata_len: usize,
+    pub user_metadata_cap: usize,
     pub is_private: bool,
     pub is_versioned: bool,
     pub creation_time_sec: i64,
@@ -112,16 +115,19 @@ impl DirectoryMetadata {
         let created_time = dir_metadata.get_created_time().to_timespec();
         let modified_time = dir_metadata.get_modified_time().to_timespec();
 
-        let (name, name_len) = helper::string_to_c_utf8(dir_metadata.get_name()
+        let (name, name_len, name_cap) = helper::string_to_c_utf8(dir_metadata.get_name()
             .to_string());
         let user_metadata = dir_metadata.get_user_metadata().to_base64(config::get_base64_config());
-        let (user_metadata, user_metadata_len) = helper::string_to_c_utf8(user_metadata);
+        let (user_metadata, user_metadata_len, user_metadata_cap) =
+            helper::string_to_c_utf8(user_metadata);
 
         Ok(DirectoryMetadata {
             name: name,
             name_len: name_len,
+            name_cap: name_cap,
             user_metadata: user_metadata,
             user_metadata_len: user_metadata_len,
+            user_metadata_cap: user_metadata_cap,
             is_private: *dir_key.get_access_level() == ::nfs::AccessLevel::Private,
             is_versioned: dir_key.is_versioned(),
             creation_time_sec: created_time.sec,
@@ -135,9 +141,10 @@ impl DirectoryMetadata {
     // a proper impl Drop.
     fn deallocate(&mut self) {
         unsafe {
-            let _ = helper::dealloc_c_utf8_alloced_from_rust(self.name, self.name_len);
-            let _ = helper::dealloc_c_utf8_alloced_from_rust(self.user_metadata,
-                                                             self.user_metadata_len);
+            let _ = misc_u8_ptr_free(self.name, self.name_len, self.name_cap);
+            let _ = misc_u8_ptr_free(self.user_metadata,
+                                     self.user_metadata_len,
+                                     self.user_metadata_cap);
         }
     }
 }

Diff for: src/ffi/file_details.rs

@@ -21,6 +21,7 @@
 use core::client::Client;
 use ffi::config;
 use ffi::errors::FfiError;
+use ffi::low_level_api::misc::misc_u8_ptr_free;
 use nfs::file::File;
 use nfs::helper::file_helper::FileHelper;
 use nfs::metadata::file_metadata::FileMetadata as NfsFileMetadata;
@@ -37,6 +38,8 @@ pub struct FileDetails {
     pub content: *mut u8,
     /// Size of `content`
     pub content_len: usize,
+    /// Capacity of `content`. Only used by the allocator's `dealloc` algorithm.
+    pub content_cap: usize,
     /// Metadata of the file.
     pub metadata: *mut FileMetadata,
 }
@@ -59,7 +62,7 @@ impl FileDetails {
 
         let content = try!(reader.read(start_position, size));
         let content = content.to_base64(config::get_base64_config());
-        let (content, content_len) = helper::string_to_c_utf8(content);
+        let (content, content_len, content_cap) = helper::string_to_c_utf8(content);
 
         let file_metadata_ptr = if include_metadata {
             Box::into_raw(Box::new(try!(FileMetadata::new(file.get_metadata()))))
@@ -70,6 +73,7 @@ impl FileDetails {
         Ok(FileDetails {
             content: content,
             content_len: content_len,
+            content_cap: content_cap,
             metadata: file_metadata_ptr,
         })
     }
@@ -78,7 +82,7 @@ impl FileDetails {
     // a proper impl Drop.
     fn deallocate(self) {
         unsafe {
-            helper::dealloc_c_utf8_alloced_from_rust(self.content, self.content_len);
+            misc_u8_ptr_free(self.content, self.content_len, self.content_cap);
         }
 
         if !self.metadata.is_null() {
@@ -94,8 +98,10 @@ impl FileDetails {
 pub struct FileMetadata {
     pub name: *mut u8,
     pub name_len: usize,
+    pub name_cap: usize,
     pub user_metadata: *mut u8,
     pub user_metadata_len: usize,
+    pub user_metadata_cap: usize,
     pub size: i64,
     pub creation_time_sec: i64,
     pub creation_time_nsec: i64,
@@ -111,18 +117,22 @@ impl FileMetadata {
         let created_time = file_metadata.get_created_time().to_timespec();
         let modified_time = file_metadata.get_modified_time().to_timespec();
 
-        let (name, name_len) = helper::string_to_c_utf8(file_metadata.get_name().to_string());
+        let (name, name_len, name_cap) = helper::string_to_c_utf8(file_metadata.get_name()
+            .to_string());
 
         let user_metadata = file_metadata.get_user_metadata()
             .to_base64(config::get_base64_config());
-        let (user_metadata, user_metadata_len) = helper::string_to_c_utf8(user_metadata);
+        let (user_metadata, user_metadata_len, user_metadata_cap) =
+            helper::string_to_c_utf8(user_metadata);
 
         Ok(FileMetadata {
             name: name,
             name_len: name_len,
+            name_cap: name_cap,
             size: file_metadata.get_size() as i64,
             user_metadata: user_metadata,
             user_metadata_len: user_metadata_len,
+            user_metadata_cap: user_metadata_cap,
             creation_time_sec: created_time.sec,
             creation_time_nsec: created_time.nsec as i64,
             modification_time_sec: modified_time.sec,
@@ -135,8 +145,10 @@ impl FileMetadata {
     // a proper impl Drop.
     pub fn deallocate(&mut self) {
         unsafe {
-            helper::dealloc_c_utf8_alloced_from_rust(self.name, self.name_len);
-            helper::dealloc_c_utf8_alloced_from_rust(self.user_metadata, self.user_metadata_len);
+            misc_u8_ptr_free(self.name, self.name_len, self.name_cap);
+            misc_u8_ptr_free(self.user_metadata,
+                             self.user_metadata_len,
+                             self.user_metadata_cap);
         }
     }
 }

Diff for: src/ffi/helper.rs

@@ -53,17 +53,17 @@ pub unsafe fn c_utf8_to_opt_string(ptr: *const u8, len: usize) -> Result<Option<
     }
 }
 
-pub fn string_to_c_utf8(s: String) -> (*mut u8, usize) {
+/// Returns a heap-allocated raw string, usable by C/FFI-boundary.
+/// The tuple means (pointer, length_in_bytes, capacity).
+/// Use `dealloc_c_utf8_alloced_from_rust` to free the memory.
+pub fn string_to_c_utf8(s: String) -> (*mut u8, usize, usize) {
     let mut v = s.into_bytes();
     v.shrink_to_fit();
     let p = v.as_mut_ptr();
     let len = v.len();
+    let cap = v.capacity();
     mem::forget(v);
-    (p, len)
-}
-
-pub unsafe fn dealloc_c_utf8_alloced_from_rust(p: *mut u8, len: usize) {
-    let _ = Vec::from_raw_parts(p, len, len);
+    (p, len, cap)
 }
 
 // TODO: add c_char_ptr_to_str and c_char_ptr_to_opt_str (return &str instead of String)

Diff for: src/ffi/low_level_api/misc.rs

@@ -52,8 +52,8 @@ pub extern "C" fn misc_sign_key_free(handle: SignKeyHandle) -> i32 {
 #[no_mangle]
 pub unsafe extern "C" fn misc_serailise_data_id(data_id_h: DataIdHandle,
                                                 o_data: *mut *mut u8,
-                                                o_size: *mut u64,
-                                                o_capacity: *mut u64)
+                                                o_size: *mut usize,
+                                                o_capacity: *mut usize)
                                                 -> i32 {
     helper::catch_unwind_i32(|| {
         let mut ser_data_id = ffi_try!(serialise(ffi_try!(unwrap!(object_cache().lock())
@@ -63,8 +63,8 @@ pub unsafe extern "C" fn misc_serailise_data_id(data_id_h: DataIdHandle,
             .map_err(FfiError::from));
 
         *o_data = ser_data_id.as_mut_ptr();
-        ptr::write(o_size, ser_data_id.len() as u64);
-        ptr::write(o_capacity, ser_data_id.capacity() as u64);
+        ptr::write(o_size, ser_data_id.len());
+        ptr::write(o_capacity, ser_data_id.capacity());
         mem::forget(ser_data_id);
 
         0
@@ -74,11 +74,11 @@ pub unsafe extern "C" fn misc_serailise_data_id(data_id_h: DataIdHandle,
 /// Deserialise DataIdentifier
 #[no_mangle]
 pub unsafe extern "C" fn misc_deserailise_data_id(data: *const u8,
-                                                  size: u64,
+                                                  size: usize,
                                                   o_handle: *mut DataIdHandle)
                                                   -> i32 {
     helper::catch_unwind_i32(|| {
-        let ser_data_id = slice::from_raw_parts(data, size as usize);
+        let ser_data_id = slice::from_raw_parts(data, size);
         let data_id = ffi_try!(deserialise(ser_data_id).map_err(FfiError::from));
 
         let mut object_cache = unwrap!(object_cache().lock());
@@ -94,8 +94,10 @@ pub unsafe extern "C" fn misc_deserailise_data_id(data: *const u8,
 
 /// Deallocate pointer obtained via FFI and allocated by safe_core
 #[no_mangle]
-pub unsafe extern "C" fn misc_u8_ptr_free(ptr: *mut u8, size: u64, capacity: u64) {
-    let _ = Vec::from_raw_parts(ptr, size as usize, capacity as usize);
+pub unsafe extern "C" fn misc_u8_ptr_free(ptr: *mut u8, size: usize, capacity: usize) {
+    // TODO: refactor implementation to remove the need for `cap`. Related issue:
+    // <https://github.com/rust-lang/rust/issues/36284>.
+    let _ = Vec::from_raw_parts(ptr, size, capacity);
 }
 
 #[cfg(test)]