Skip to content

Commit

Permalink
refactor: resolve c pointers const
Browse files Browse the repository at this point in the history
Signed-off-by: tison <wander4096@gmail.com>
  • Loading branch information
tisonkun committed Oct 8, 2024
1 parent 13cb7a6 commit 1b1c09e
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 60 deletions.
6 changes: 3 additions & 3 deletions bindings/c/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ 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");

Expand Down
6 changes: 3 additions & 3 deletions bindings/c/examples/basic.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ 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");

Expand Down
12 changes: 0 additions & 12 deletions bindings/c/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,6 @@ 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))
}
}
Expand Down
20 changes: 9 additions & 11 deletions bindings/c/src/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ static RUNTIME: Lazy<tokio::runtime::Runtime> = 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 {
Expand Down Expand Up @@ -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
Expand All @@ -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),
},
}
Expand Down
2 changes: 1 addition & 1 deletion bindings/c/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
49 changes: 29 additions & 20 deletions bindings/c/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,40 +31,49 @@ 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
pub capacity: usize,
}

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(),
}
}

/// \brief Frees the heap memory used by the 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);
pub unsafe extern "C" fn opendal_bytes_free(bs: opendal_bytes) {
if !bs.data.is_null() {
drop(Vec::from_raw_parts(bs.data, bs.len, bs.capacity));
}
}
}

impl Drop for opendal_bytes {
fn drop(&mut self) {
if !self.data.is_null() {
unsafe {
// Safety: the data is not null, and the capacity is correct
drop(Vec::from_raw_parts(self.data, self.len, self.capacity));
}
self.data = std::ptr::null_mut();
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions bindings/c/tests/bdd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions bindings/go/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,12 @@ type bytesFree func(b *opendalBytes)
var withBytesFree = withFFI(ffiOpts{
sym: symBytesFree,
rType: &ffi.TypeVoid,
aTypes: []*ffi.Type{&ffi.TypePointer},
aTypes: []*ffi.Type{&typeBytes},
}, func(_ context.Context, ffiCall func(rValue unsafe.Pointer, aValues ...unsafe.Pointer)) bytesFree {
return func(b *opendalBytes) {
return func(b opendalBytes) {
ffiCall(
nil,
unsafe.Pointer(&b),
b,
)
}
})
1 change: 0 additions & 1 deletion bindings/go/stat.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion bindings/swift/OpenDAL/Sources/OpenDAL/Data+OpenDAL.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ extension Data {
/// This can be used to read data from Rust with zero-copying.
/// The underlying buffer will be freed when the data gets
/// deallocated.
init(openDALBytes: UnsafeMutablePointer<opendal_bytes>) {
init(openDALBytes: opendal_bytes) {
let address = UnsafeRawPointer(openDALBytes.pointee.data)!
let length = Int(openDALBytes.pointee.len)
self.init(
Expand Down
4 changes: 2 additions & 2 deletions bindings/zig/test/bdd.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -60,8 +60,8 @@ test "Opendal BDD test" {
// 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),
.capacity = std.mem.len(testkit.content),
};
const result = opendal.c.opendal_operator_write(testkit.p, testkit.path, data);
try testing.expectEqual(result, null);
Expand Down

0 comments on commit 1b1c09e

Please sign in to comment.