diff --git a/bindings/c/README.md b/bindings/c/README.md index bd225ceb14b9..398f1a1f198b 100644 --- a/bindings/c/README.md +++ b/bindings/c/README.md @@ -32,18 +32,18 @@ int main() /* We can read it out, make sure the data is the same */ opendal_result_read r = opendal_operator_read(op, "/testpath"); - opendal_bytes* read_bytes = r.data; + opendal_bytes read_bytes = r.data; assert(r.error == NULL); - assert(read_bytes->len == 24); + assert(read_bytes.len == 24); /* Lets print it out */ for (int i = 0; i < 24; ++i) { - printf("%c", read_bytes->data[i]); + printf("%c", read_bytes.data[i]); } printf("\n"); /* the opendal_bytes read is heap allocated, please free it */ - opendal_bytes_free(read_bytes); + opendal_bytes_free(&read_bytes); /* the operator_ptr is also heap allocated */ opendal_operator_free(&op); diff --git a/bindings/c/examples/basic.c b/bindings/c/examples/basic.c index 5e4e7925ba77..af55faeea0df 100644 --- a/bindings/c/examples/basic.c +++ b/bindings/c/examples/basic.c @@ -42,18 +42,18 @@ int main() /* We can read it out, make sure the data is the same */ opendal_result_read r = opendal_operator_read(op, "/testpath"); - opendal_bytes* read_bytes = r.data; + opendal_bytes read_bytes = r.data; assert(r.error == NULL); - assert(read_bytes->len == 24); + assert(read_bytes.len == 24); /* Lets print it out */ for (int i = 0; i < 24; ++i) { - printf("%c", read_bytes->data[i]); + printf("%c", read_bytes.data[i]); } printf("\n"); /* the opendal_bytes read is heap allocated, please free it */ - opendal_bytes_free(read_bytes); + opendal_bytes_free(&read_bytes); /* the operator_ptr is also heap allocated */ opendal_operator_free(op); diff --git a/bindings/c/include/opendal.h b/bindings/c/include/opendal.h index 4fba90916344..8eeacf7c5121 100644 --- a/bindings/c/include/opendal.h +++ b/bindings/c/include/opendal.h @@ -94,7 +94,7 @@ typedef struct opendal_bytes { /** * Pointing to the byte array on heap */ - const uint8_t *data; + uint8_t *data; /** * The length of the byte array */ @@ -113,7 +113,7 @@ typedef struct opendal_bytes { * represents no error has taken placed**. If any error has taken place, the caller should check * the error code and print the error message. * - * The error code is represented in opendal_code, which is a enum on different type of errors. + * The error code is represented in opendal_code, which is an enum on different type of errors. * The error messages is represented in opendal_bytes, which is a non-null terminated byte array. * * \note 1. The error message is on heap, so the error needs to be freed by the caller, by calling @@ -217,7 +217,7 @@ typedef struct opendal_operator { * The pointer to the opendal::BlockingOperator in the Rust code. * Only touch this on judging whether it is NULL. */ - const void *inner; + void *inner; } opendal_operator; /** @@ -273,7 +273,7 @@ typedef struct opendal_result_read { /** * The byte array with length returned by read operations */ - struct opendal_bytes *data; + struct opendal_bytes data; /** * The error, if ok, it is null */ @@ -814,7 +814,7 @@ struct opendal_result_operator_new opendal_operator_new(const char *scheme, */ struct opendal_error *opendal_operator_write(const struct opendal_operator *op, const char *path, - struct opendal_bytes bytes); + const struct opendal_bytes *bytes); /** * \brief Blocking read the data from `path`. @@ -842,8 +842,9 @@ struct opendal_error *opendal_operator_write(const struct opendal_operator *op, * opendal_result_read r = opendal_operator_read(op, "testpath"); * assert(r.error == NULL); * - * opendal_bytes *bytes = r.data; - * assert(bytes->len == 13); + * opendal_bytes bytes = r.data; + * assert(bytes.len == 13); + * opendal_bytes_free(&bytes); * ``` * * # Safety @@ -1394,7 +1395,7 @@ void opendal_reader_free(struct opendal_reader *ptr); * \brief Write data to the writer. */ struct opendal_result_writer_write opendal_writer_write(struct opendal_writer *self, - struct opendal_bytes bytes); + const struct opendal_bytes *bytes); /** * \brief Frees the heap memory used by the opendal_writer. diff --git a/bindings/c/src/error.rs b/bindings/c/src/error.rs index d1c6155ee72e..97388ddfe240 100644 --- a/bindings/c/src/error.rs +++ b/bindings/c/src/error.rs @@ -82,7 +82,7 @@ impl From for opendal_code { /// represents no error has taken placed**. If any error has taken place, the caller should check /// the error code and print the error message. /// -/// The error code is represented in opendal_code, which is a enum on different type of errors. +/// The error code is represented in opendal_code, which is an enum on different type of errors. /// The error messages is represented in opendal_bytes, which is a non-null terminated byte array. /// /// \note 1. The error message is on heap, so the error needs to be freed by the caller, by calling @@ -114,19 +114,7 @@ impl opendal_error { #[no_mangle] pub unsafe extern "C" fn opendal_error_free(ptr: *mut opendal_error) { if !ptr.is_null() { - let message_ptr = &(*ptr).message; - let message_ptr = message_ptr as *const opendal_bytes as *mut opendal_bytes; - if !message_ptr.is_null() { - let data_mut = (*message_ptr).data as *mut u8; - drop(Vec::from_raw_parts( - data_mut, - (*message_ptr).len, - (*message_ptr).len, - )); - } - - // free the pointer - drop(Box::from_raw(ptr)) + drop(Box::from_raw(ptr)); } } } diff --git a/bindings/c/src/operator.rs b/bindings/c/src/operator.rs index 04df68ec455f..ba03f9c2ae92 100644 --- a/bindings/c/src/operator.rs +++ b/bindings/c/src/operator.rs @@ -47,7 +47,7 @@ static RUNTIME: Lazy = Lazy::new(|| { pub struct opendal_operator { /// The pointer to the opendal::BlockingOperator in the Rust code. /// Only touch this on judging whether it is NULL. - inner: *const c_void, + inner: *mut c_void, } impl opendal_operator { @@ -226,7 +226,7 @@ pub unsafe extern "C" fn opendal_operator_new( pub unsafe extern "C" fn opendal_operator_write( op: &opendal_operator, path: *const c_char, - bytes: opendal_bytes, + bytes: &opendal_bytes, ) -> *mut opendal_error { assert!(!path.is_null()); let path = std::ffi::CStr::from_ptr(path) @@ -263,8 +263,9 @@ pub unsafe extern "C" fn opendal_operator_write( /// opendal_result_read r = opendal_operator_read(op, "testpath"); /// assert(r.error == NULL); /// -/// opendal_bytes *bytes = r.data; -/// assert(bytes->len == 13); +/// opendal_bytes bytes = r.data; +/// assert(bytes.len == 13); +/// opendal_bytes_free(&bytes); /// ``` /// /// # Safety @@ -286,15 +287,12 @@ pub unsafe extern "C" fn opendal_operator_read( .to_str() .expect("malformed path"); match op.deref().read(path) { - Ok(d) => { - let v = Box::new(opendal_bytes::new(d)); - opendal_result_read { - data: Box::into_raw(v), - error: std::ptr::null_mut(), - } - } + Ok(b) => opendal_result_read { + data: opendal_bytes::new(b), + error: std::ptr::null_mut(), + }, Err(e) => opendal_result_read { - data: std::ptr::null_mut(), + data: opendal_bytes::empty(), error: opendal_error::new(e), }, } diff --git a/bindings/c/src/result.rs b/bindings/c/src/result.rs index ba7e1c64a028..e9aede5d47fb 100644 --- a/bindings/c/src/result.rs +++ b/bindings/c/src/result.rs @@ -49,7 +49,7 @@ pub struct opendal_result_operator_new { #[repr(C)] pub struct opendal_result_read { /// The byte array with length returned by read operations - pub data: *mut opendal_bytes, + pub data: opendal_bytes, /// The error, if ok, it is null pub error: *mut opendal_error, } diff --git a/bindings/c/src/types.rs b/bindings/c/src/types.rs index 795c7a73b5c1..cba8f2540063 100644 --- a/bindings/c/src/types.rs +++ b/bindings/c/src/types.rs @@ -31,7 +31,7 @@ use opendal::Buffer; #[repr(C)] pub struct opendal_bytes { /// Pointing to the byte array on heap - pub data: *const u8, + pub data: *mut u8, /// The length of the byte array pub len: usize, /// The capacity of the byte array @@ -39,17 +39,21 @@ pub struct opendal_bytes { } impl opendal_bytes { + pub(crate) fn empty() -> Self { + Self { + data: std::ptr::null_mut(), + len: 0, + capacity: 0, + } + } + /// Construct a [`opendal_bytes`] from the Rust [`Vec`] of bytes - pub(crate) fn new(buf: Buffer) -> Self { - let vec = buf.to_vec(); - let mut buf = std::mem::ManuallyDrop::new(vec); - let data = buf.as_mut_ptr(); - let len = buf.len(); - let capacity = buf.capacity(); + pub(crate) fn new(b: Buffer) -> Self { + let mut b = std::mem::ManuallyDrop::new(b.to_vec()); Self { - data, - len, - capacity, + data: b.as_mut_ptr(), + len: b.len(), + capacity: b.capacity(), } } @@ -57,20 +61,28 @@ impl opendal_bytes { #[no_mangle] pub unsafe extern "C" fn opendal_bytes_free(ptr: *mut opendal_bytes) { if !ptr.is_null() { - // transmuting `*const u8` to `*mut u8` is undefined behavior in any cases - // however, fields type of `opendal_bytes` is already related to the zig binding - // it should be fixed later - let _ = Vec::from_raw_parts((*ptr).data as *mut u8, (*ptr).len, (*ptr).capacity); - // it is too weird that call `Box::new` outside `opendal_bytes::new` but dealloc it here - // also, boxing `opendal_bytes` might not be necessary - // `data` points to heap, so `opendal_bytes` could be passed as a stack value - let _ = Box::from_raw(ptr); + let bs = &mut *ptr; + if !bs.data.is_null() { + drop(Vec::from_raw_parts(bs.data, bs.len, bs.capacity)); + bs.data = std::ptr::null_mut(); + bs.len = 0; + bs.capacity = 0; + } + } + } +} + +impl Drop for opendal_bytes { + fn drop(&mut self) { + unsafe { + // Safety: the pointer is always valid + Self::opendal_bytes_free(self); } } } -impl From for Buffer { - fn from(v: opendal_bytes) -> Self { +impl From<&opendal_bytes> for Buffer { + fn from(v: &opendal_bytes) -> Self { let slice = unsafe { std::slice::from_raw_parts(v.data, v.len) }; Buffer::from(bytes::Bytes::copy_from_slice(slice)) } diff --git a/bindings/c/src/writer.rs b/bindings/c/src/writer.rs index 67ac7d53f45b..a72dbbbb6934 100644 --- a/bindings/c/src/writer.rs +++ b/bindings/c/src/writer.rs @@ -49,7 +49,7 @@ impl opendal_writer { #[no_mangle] pub unsafe extern "C" fn opendal_writer_write( &mut self, - bytes: opendal_bytes, + bytes: &opendal_bytes, ) -> opendal_result_writer_write { let size = bytes.len; match self.deref_mut().write(bytes) { diff --git a/bindings/c/tests/bdd.cpp b/bindings/c/tests/bdd.cpp index 39856e81e5f0..65e97f52bf44 100644 --- a/bindings/c/tests/bdd.cpp +++ b/bindings/c/tests/bdd.cpp @@ -61,7 +61,7 @@ TEST_F(OpendalBddTest, FeatureTest) .data = (uint8_t*)this->content.c_str(), .len = this->content.length(), }; - opendal_error* error = opendal_operator_write(this->p, this->path.c_str(), data); + opendal_error* error = opendal_operator_write(this->p, this->path.c_str(), &data); EXPECT_EQ(error, nullptr); // The blocking file "test" should exist @@ -85,9 +85,9 @@ TEST_F(OpendalBddTest, FeatureTest) // The blocking file "test" must have content "Hello, World!" struct opendal_result_read r = opendal_operator_read(this->p, this->path.c_str()); EXPECT_EQ(r.error, nullptr); - EXPECT_EQ(r.data->len, this->content.length()); - for (int i = 0; i < r.data->len; i++) { - EXPECT_EQ(this->content[i], (char)(r.data->data[i])); + EXPECT_EQ(r.data.len, this->content.length()); + for (int i = 0; i < r.data.len; i++) { + EXPECT_EQ(this->content[i], (char)(r.data.data[i])); } // The blocking file should be deleted @@ -99,7 +99,7 @@ TEST_F(OpendalBddTest, FeatureTest) opendal_result_operator_writer writer = opendal_operator_writer(this->p, this->path.c_str()); EXPECT_EQ(writer.error, nullptr); - opendal_result_writer_write w = opendal_writer_write(writer.writer, data); + opendal_result_writer_write w = opendal_writer_write(writer.writer, &data); EXPECT_EQ(w.error, nullptr); EXPECT_EQ(w.size, this->content.length()); opendal_writer_free(writer.writer); @@ -120,7 +120,7 @@ TEST_F(OpendalBddTest, FeatureTest) error = opendal_operator_delete(this->p, this->path.c_str()); EXPECT_EQ(error, nullptr); - opendal_bytes_free(r.data); + opendal_bytes_free(&r.data); // The directory "tmpdir/" should exist and should be a directory error = opendal_operator_create_dir(this->p, "tmpdir/"); diff --git a/bindings/c/tests/error_msg.cpp b/bindings/c/tests/error_msg.cpp index f2087ea3b0bf..516d27865aaf 100644 --- a/bindings/c/tests/error_msg.cpp +++ b/bindings/c/tests/error_msg.cpp @@ -59,7 +59,7 @@ TEST_F(OpendalErrorTest, ErrorReadTest) ASSERT_GT(error_msg->len, 0); // the opendal_bytes read is heap allocated, please free it - opendal_bytes_free(r.data); + opendal_bytes_free(&r.data); // free the error opendal_error_free(r.error); diff --git a/bindings/c/tests/list.cpp b/bindings/c/tests/list.cpp index d4e31cf79e5b..3b64fef460b6 100644 --- a/bindings/c/tests/list.cpp +++ b/bindings/c/tests/list.cpp @@ -65,7 +65,7 @@ TEST_F(OpendalListTest, ListDirTest) }; // write must succeed - EXPECT_EQ(opendal_operator_write(this->p, path.c_str(), data), + EXPECT_EQ(opendal_operator_write(this->p, path.c_str(), &data), nullptr); // list must succeed since the write succeeded diff --git a/bindings/go/error.go b/bindings/go/error.go index 33ed9b8c28f4..37ea26f07073 100644 --- a/bindings/go/error.go +++ b/bindings/go/error.go @@ -77,7 +77,7 @@ func parseError(ctx context.Context, err *opendalError) error { defer free(err) return &Error{ code: ErrorCode(err.code), - message: string(parseBytes(&err.message)), + message: string(parseBytes(err.message)), } } diff --git a/bindings/go/reader.go b/bindings/go/reader.go index c6fbb68238a8..ce26ce663ecd 100644 --- a/bindings/go/reader.go +++ b/bindings/go/reader.go @@ -68,8 +68,7 @@ func (op *Operator) Read(path string) ([]byte, error) { data := parseBytes(bytes) if len(data) > 0 { free := getFFI[bytesFree](op.ctx, symBytesFree) - free(bytes) - + free(&bytes) } return data, nil } @@ -203,17 +202,17 @@ func (r *Reader) Close() error { const symOperatorRead = "opendal_operator_read" -type operatorRead func(op *opendalOperator, path string) (*opendalBytes, error) +type operatorRead func(op *opendalOperator, path string) (opendalBytes, error) var withOperatorRead = withFFI(ffiOpts{ sym: symOperatorRead, rType: &typeResultRead, aTypes: []*ffi.Type{&ffi.TypePointer, &ffi.TypePointer}, }, func(ctx context.Context, ffiCall func(rValue unsafe.Pointer, aValues ...unsafe.Pointer)) operatorRead { - return func(op *opendalOperator, path string) (*opendalBytes, error) { + return func(op *opendalOperator, path string) (opendalBytes, error) { bytePath, err := unix.BytePtrFromString(path) if err != nil { - return nil, err + return opendalBytes{}, err } var result resultRead ffiCall( diff --git a/bindings/go/stat.go b/bindings/go/stat.go index 6b83ad605c80..7eb7345939a0 100644 --- a/bindings/go/stat.go +++ b/bindings/go/stat.go @@ -96,7 +96,6 @@ func (op *Operator) Stat(path string) (*Metadata, error) { // } else { // fmt.Println("The file does not exist") // } -// func (op *Operator) IsExist(path string) (bool, error) { isExist := getFFI[operatorIsExist](op.ctx, symOperatorIsExist) return isExist(op.inner, path) diff --git a/bindings/go/types.go b/bindings/go/types.go index 613825af5104..5751f0cdb05d 100644 --- a/bindings/go/types.go +++ b/bindings/go/types.go @@ -38,7 +38,7 @@ var ( typeResultRead = ffi.Type{ Type: ffi.Struct, Elements: &[]*ffi.Type{ - &ffi.TypePointer, + &typeBytes, &ffi.TypePointer, nil, }[0], @@ -217,7 +217,7 @@ type resultOperatorNew struct { type opendalOperator struct{} type resultRead struct { - data *opendalBytes + data opendalBytes error *opendalError } @@ -284,7 +284,7 @@ type opendalResultListerNext struct { type opendalEntry struct{} -func toOpendalBytes(data []byte) opendalBytes { +func toOpendalBytes(data []byte) *opendalBytes { var ptr *byte l := len(data) if l > 0 { @@ -293,15 +293,15 @@ func toOpendalBytes(data []byte) opendalBytes { var b byte ptr = &b } - return opendalBytes{ + return &opendalBytes{ data: ptr, len: uintptr(l), - capacity: uintptr(l), + capacity: uintptr(cap(data)), } } -func parseBytes(b *opendalBytes) (data []byte) { - if b == nil || b.len == 0 { +func parseBytes(b opendalBytes) (data []byte) { + if b.len == 0 { return nil } data = make([]byte, b.len) diff --git a/bindings/go/write.go b/bindings/go/write.go index d287524416c5..f0c23b816f5e 100644 --- a/bindings/go/write.go +++ b/bindings/go/write.go @@ -196,7 +196,7 @@ type operatorWrite func(op *opendalOperator, path string, data []byte) error var withOperatorWrite = withFFI(ffiOpts{ sym: symOperatorWrite, rType: &ffi.TypePointer, - aTypes: []*ffi.Type{&ffi.TypePointer, &ffi.TypePointer, &typeBytes}, + aTypes: []*ffi.Type{&ffi.TypePointer, &ffi.TypePointer, &ffi.TypePointer}, }, func(ctx context.Context, ffiCall func(rValue unsafe.Pointer, aValues ...unsafe.Pointer)) operatorWrite { return func(op *opendalOperator, path string, data []byte) error { bytePath, err := unix.BytePtrFromString(path) @@ -290,7 +290,7 @@ type writerWrite func(r *opendalWriter, buf []byte) (size int, err error) var withWriterWrite = withFFI(ffiOpts{ sym: symWriterWrite, rType: &typeResultWriterWrite, - aTypes: []*ffi.Type{&ffi.TypePointer, &typeBytes}, + aTypes: []*ffi.Type{&ffi.TypePointer, &ffi.TypePointer}, }, func(ctx context.Context, ffiCall func(rValue unsafe.Pointer, aValues ...unsafe.Pointer)) writerWrite { return func(r *opendalWriter, data []byte) (size int, err error) { bytes := toOpendalBytes(data) diff --git a/bindings/swift/OpenDAL/Sources/OpenDAL/Operator.swift b/bindings/swift/OpenDAL/Sources/OpenDAL/Operator.swift index 2ce27551e39e..6e2bb3a7bd1c 100644 --- a/bindings/swift/OpenDAL/Sources/OpenDAL/Operator.swift +++ b/bindings/swift/OpenDAL/Sources/OpenDAL/Operator.swift @@ -57,11 +57,13 @@ public class Operator { self.nativeOp = UnsafePointer(ret.op)! } - public func blockingWrite(_ data: Data, to path: String) throws { - let ret = data.withUnsafeBytes { dataPointer in + public func blockingWrite(_ data: inout Data, to path: String) throws { + let ret = data.withUnsafeMutableBytes { dataPointer in let address = dataPointer.baseAddress!.assumingMemoryBound(to: UInt8.self) - let bytes = opendal_bytes(data: address, len: UInt(dataPointer.count)) - return opendal_operator_write(nativeOp, path, bytes) + let bytes = opendal_bytes(data: address, len: UInt(dataPointer.count), capacity: UInt(dataPointer.count)) + return withUnsafePointer(to: bytes) { bytesPointer in + opendal_operator_write(nativeOp, path, bytesPointer) + } } if let err = ret { @@ -79,7 +81,7 @@ public class Operator { } public func blockingRead(_ path: String) throws -> Data { - let ret = opendal_operator_read(nativeOp, path) + var ret = opendal_operator_read(nativeOp, path) if let err = ret.error { defer { opendal_error_free(err) @@ -93,6 +95,6 @@ public class Operator { ) } - return Data(openDALBytes: ret.data) + return withUnsafeMutablePointer(to: &ret.data) { Data(openDALBytes: $0) } } } diff --git a/bindings/swift/OpenDAL/Tests/OpenDALTests/OpenDALTests.swift b/bindings/swift/OpenDAL/Tests/OpenDALTests/OpenDALTests.swift index ef17c7887486..304e6d5a27d8 100644 --- a/bindings/swift/OpenDAL/Tests/OpenDALTests/OpenDALTests.swift +++ b/bindings/swift/OpenDAL/Tests/OpenDALTests/OpenDALTests.swift @@ -28,8 +28,8 @@ final class OpenDALTests: XCTestCase { ] ) - let testContents = Data([1, 2, 3, 4]) - try op.blockingWrite(testContents, to: "test") + var testContents = Data([1, 2, 3, 4]) + try op.blockingWrite(&testContents, to: "test") let readContents = try op.blockingRead("test") diff --git a/bindings/zig/test/bdd.zig b/bindings/zig/test/bdd.zig index 98d49525ce0c..8d2f6ce79f6c 100644 --- a/bindings/zig/test/bdd.zig +++ b/bindings/zig/test/bdd.zig @@ -24,7 +24,7 @@ test "Opendal BDD test" { const c_str = [*:0]const u8; // define a type for 'const char*' in C const OpendalBDDTest = struct { - p: [*c]const opendal.c.opendal_operator, + p: [*c]opendal.c.opendal_operator, scheme: c_str, path: c_str, content: c_str, @@ -57,13 +57,15 @@ test "Opendal BDD test" { var testkit = OpendalBDDTest.init(); defer testkit.deinit(); + const allocator = std.heap.page_allocator; + const dupe_content = try allocator.dupeZ(u8, std.mem.span(testkit.content)); // When Blocking write path "test" with content "Hello, World!" const data: opendal.c.opendal_bytes = .{ - .data = testkit.content, - // c_str does not have len field (.* is ptr) - .len = std.mem.len(testkit.content), + .data = dupe_content.ptr, + .len = dupe_content.len, + .capacity = dupe_content.len, }; - const result = opendal.c.opendal_operator_write(testkit.p, testkit.path, data); + const result = opendal.c.opendal_operator_write(testkit.p, testkit.path, &data); try testing.expectEqual(result, null); // The blocking file "test" should exist @@ -82,14 +84,14 @@ test "Opendal BDD test" { defer opendal.c.opendal_metadata_free(meta); // The blocking file "test" must have content "Hello, World!" - const r: opendal.c.opendal_result_read = opendal.c.opendal_operator_read(testkit.p, testkit.path); - defer opendal.c.opendal_bytes_free(r.data); + var r: opendal.c.opendal_result_read = opendal.c.opendal_operator_read(testkit.p, testkit.path); + defer opendal.c.opendal_bytes_free(&r.data); try testing.expect(r.@"error" == null); - try testing.expectEqual(std.mem.len(testkit.content), r.data.*.len); + try testing.expectEqual(std.mem.len(testkit.content), r.data.len); var count: usize = 0; - while (count < r.data.*.len) : (count += 1) { - try testing.expectEqual(testkit.content[count], r.data.*.data[count]); + while (count < r.data.len) : (count += 1) { + try testing.expectEqual(testkit.content[count], r.data.data[count]); } }