fix(bindings/C): fix the memory found in valgrind.

bindings/c/include/opendal.h

@@ -364,7 +364,10 @@ const struct opendal_operator_ptr *opendal_operator_new(const char *scheme,
  * \brief Blockingly write raw bytes to `path`.
  *
  * Write the `bytes` into the `path` blockingly by `op_ptr`, returns the opendal_code OPENDAL_OK
- * if succeeds, others otherwise
+ * if succeeds, others otherwise.
+ *
+ * @NOTE It is important to notice that the `bytes` that is passes in will be consumed by this
+ *       function.
  *
  * @param ptr The opendal_operator_ptr created previously
  * @param path The designated path you want to write your bytes in
@@ -653,12 +656,12 @@ void opendal_operator_free(const struct opendal_operator_ptr *op);
 /**
  * \brief Frees the heap memory used by the opendal_bytes
  */
-void opendal_bytes_free(const struct opendal_bytes *self);
+void opendal_bytes_free(struct opendal_bytes *ptr);
 
 /**
  * \brief Free the heap-allocated metadata used by opendal_metadata
  */
-void opendal_metadata_free(struct opendal_metadata *self);
+void opendal_metadata_free(struct opendal_metadata *ptr);
 
 /**
  * \brief Return the content_length of the metadata
@@ -765,23 +768,29 @@ struct opendal_list_entry *opendal_lister_next(const struct opendal_blocking_lis
 void opendal_lister_free(const struct opendal_blocking_lister *p);
 
 /**
- * Path of entry. Path is relative to operator's root.
- * Only valid in current operator.
+ * \brief Path of entry.
+ *
+ * Path is relative to operator's root. Only valid in current operator.
+ *
+ * @NOTE To free the string, you can directly call free()
  */
-const char *opendal_list_entry_path(const struct opendal_list_entry *self);
+char *opendal_list_entry_path(const struct opendal_list_entry *self);
 
 /**
- * Name of entry. Name is the last segment of path.
+ * \brief Name of entry.
  *
+ * Name is the last segment of path.
  * If this entry is a dir, `Name` MUST endswith `/`
  * Otherwise, `Name` MUST NOT endswith `/`.
+ *
+ * @NOTE To free the string, you can directly call free()
  */
-const char *opendal_list_entry_name(const struct opendal_list_entry *self);
+char *opendal_list_entry_name(const struct opendal_list_entry *self);
 
 /**
  * \brief Frees the heap memory used by the opendal_list_entry
  */
-void opendal_list_entry_free(const struct opendal_list_entry *p);
+void opendal_list_entry_free(struct opendal_list_entry *ptr);
 
 #ifdef __cplusplus
 } // extern "C"

bindings/c/src/lib.rs

@@ -126,7 +126,10 @@ pub unsafe extern "C" fn opendal_operator_new(
 /// \brief Blockingly write raw bytes to `path`.
 ///
 /// Write the `bytes` into the `path` blockingly by `op_ptr`, returns the opendal_code OPENDAL_OK
-/// if succeeds, others otherwise
+/// if succeeds, others otherwise.
+///
+/// @NOTE It is important to notice that the `bytes` that is passes in will be consumed by this
+///       function.
 ///
 /// @param ptr The opendal_operator_ptr created previously
 /// @param path The designated path you want to write your bytes in
@@ -235,7 +238,7 @@ pub unsafe extern "C" fn opendal_operator_blocking_read(
     let data = op.read(path);
     match data {
         Ok(d) => {
-            let v = Box::new(opendal_bytes::from_vec(d));
+            let v = Box::new(opendal_bytes::new(d));
             opendal_result_read {
                 data: Box::into_raw(v),
                 code: opendal_code::OPENDAL_OK,

bindings/c/src/types.rs

@@ -16,7 +16,7 @@
 // under the License.
 
 use std::collections::HashMap;
-use std::mem;
+use std::ffi::CString;
 use std::os::raw::c_char;
 
 use ::opendal as od;
@@ -98,26 +98,26 @@ pub struct opendal_bytes {
     pub len: usize,
 }
 
-impl opendal_bytes {
-    /// \brief Frees the heap memory used by the opendal_bytes
-    #[no_mangle]
-    pub extern "C" fn opendal_bytes_free(&self) {
-        unsafe {
-            // this deallocates the vector by reconstructing the vector and letting
-            // it be dropped when its out of scope
-            Vec::from_raw_parts(self.data as *mut u8, self.len, self.len);
-        }
-    }
-}
-
 impl opendal_bytes {
     /// Construct a [`opendal_bytes`] from the Rust [`Vec`] of bytes
-    pub(crate) fn from_vec(vec: Vec<u8>) -> Self {
-        let data = vec.as_ptr() as *const u8;
+    pub(crate) fn new(vec: Vec<u8>) -> Self {
+        let data = vec.as_ptr();
         let len = vec.len();
-        std::mem::forget(vec); // To avoid deallocation of the vec.
+        std::mem::forget(vec);
         Self { data, len }
     }
+
+    /// \brief Frees the heap memory used by the opendal_bytes
+    #[no_mangle]
+    pub extern "C" fn opendal_bytes_free(ptr: *mut opendal_bytes) {
+        if !ptr.is_null() {
+            let data_mut = unsafe { (*ptr).data as *mut u8 };
+            // free the vector
+            let _ = unsafe { Vec::from_raw_parts(data_mut, (*ptr).len, (*ptr).len) };
+            // free the pointer
+            let _ = unsafe { Box::from_raw(ptr) };
+        }
+    }
 }
 
 #[allow(clippy::from_over_into)]
@@ -155,10 +155,10 @@ impl opendal_metadata {
 
     /// \brief Free the heap-allocated metadata used by opendal_metadata
     #[no_mangle]
-    pub extern "C" fn opendal_metadata_free(&mut self) {
+    pub extern "C" fn opendal_metadata_free(ptr: *mut opendal_metadata) {
         unsafe {
-            mem::drop(Box::from_raw(self.inner));
-            mem::drop(Box::from_raw(self as *mut Self));
+            let _ = Box::from_raw((*ptr).inner);
+            let _ = Box::from_raw(ptr);
         }
     }
 
@@ -373,27 +373,38 @@ impl opendal_list_entry {
         }
     }
 
-    /// Path of entry. Path is relative to operator's root.
-    /// Only valid in current operator.
+    /// \brief Path of entry.
+    ///
+    /// Path is relative to operator's root. Only valid in current operator.
+    ///
+    /// @NOTE To free the string, you can directly call free()
     #[no_mangle]
-    pub unsafe extern "C" fn opendal_list_entry_path(&self) -> *const c_char {
-        (*self.inner).path().as_ptr() as *const c_char
+    pub unsafe extern "C" fn opendal_list_entry_path(&self) -> *mut c_char {
+        let s = (*self.inner).path();
+        let c_str = CString::new(s).unwrap();
+        c_str.into_raw()
     }
 
-    /// Name of entry. Name is the last segment of path.
+    /// \brief Name of entry.
     ///
+    /// Name is the last segment of path.
     /// If this entry is a dir, `Name` MUST endswith `/`
     /// Otherwise, `Name` MUST NOT endswith `/`.
+    ///
+    /// @NOTE To free the string, you can directly call free()
     #[no_mangle]
-    pub unsafe extern "C" fn opendal_list_entry_name(&self) -> *const c_char {
-        (*self.inner).name().as_ptr() as *const c_char
+    pub unsafe extern "C" fn opendal_list_entry_name(&self) -> *mut c_char {
+        let s = (*self.inner).name();
+        let c_str = CString::new(s).unwrap();
+        c_str.into_raw()
     }
 
     /// \brief Frees the heap memory used by the opendal_list_entry
     #[no_mangle]
-    pub unsafe extern "C" fn opendal_list_entry_free(p: *const opendal_list_entry) {
-        unsafe {
-            let _ = Box::from_raw(p as *mut opendal_list_entry);
+    pub unsafe extern "C" fn opendal_list_entry_free(ptr: *mut opendal_list_entry) {
+        if !ptr.is_null() {
+            let _ = unsafe { Box::from_raw((*ptr).inner) };
+            let _ = unsafe { Box::from_raw(ptr) };
         }
     }
 }

bindings/c/tests/list.cpp

@@ -47,8 +47,8 @@ class OpendalListTest : public ::testing::Test {
 // Basic usecase of list
 TEST_F(OpendalListTest, ListDirTest)
 {
-    std::string dname = "some_random_dir_name_152312dbfas";
-    std::string fname = "some_random_file_name_2138912rbf";
+    std::string dname = "some_random_dir_name_152312";
+    std::string fname = "some_random_file_name_21389";
 
     // 4 MiB of random bytes
     uintptr_t nbytes = 4 * 1024 * 1024;
@@ -76,23 +76,24 @@ TEST_F(OpendalListTest, ListDirTest)
 
     opendal_list_entry* entry = opendal_lister_next(lister);
     while (entry) {
-        const char* de_path = opendal_list_entry_path(entry);
+        char* de_path = opendal_list_entry_path(entry);
 
         // stat must succeed
         opendal_result_stat s = opendal_operator_stat(this->p, de_path);
         EXPECT_EQ(s.code, OPENDAL_OK);
 
-        opendal_metadata* meta = s.meta;
-
         if (!strcmp(de_path, path.c_str())) {
             found = true;
 
             // the path we found has to be a file, and the length must be coherent
-            EXPECT_TRUE(opendal_metadata_is_file(meta));
-            EXPECT_EQ(opendal_metadata_content_length(meta), nbytes);
+            EXPECT_TRUE(opendal_metadata_is_file(s.meta));
+            EXPECT_EQ(opendal_metadata_content_length(s.meta), nbytes);
         }
 
+        free(de_path);
+        opendal_metadata_free(s.meta);
         opendal_list_entry_free(entry);
+
         entry = opendal_lister_next(lister);
     }