diff --git a/bindings/c/include/opendal.h b/bindings/c/include/opendal.h index c2aa31721b4f..d833563242a5 100644 --- a/bindings/c/include/opendal.h +++ b/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" diff --git a/bindings/c/src/lib.rs b/bindings/c/src/lib.rs index b749a5d6b064..6e51302656cb 100644 --- a/bindings/c/src/lib.rs +++ b/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, diff --git a/bindings/c/src/types.rs b/bindings/c/src/types.rs index e324b508e873..77d6a1c225c3 100644 --- a/bindings/c/src/types.rs +++ b/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) -> Self { - let data = vec.as_ptr() as *const u8; + pub(crate) fn new(vec: Vec) -> 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) }; } } } diff --git a/bindings/c/tests/list.cpp b/bindings/c/tests/list.cpp index de905f17df5d..b6b44dd44d02 100644 --- a/bindings/c/tests/list.cpp +++ b/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); }