Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: more consistent C binding pattern #5162

Merged
merged 4 commits into from
Oct 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 27 additions & 61 deletions bindings/c/include/opendal.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,60 +81,6 @@ typedef enum opendal_code {
OPENDAL_RANGE_NOT_SATISFIED,
} opendal_code;

/**
* BlockingOperator is the entry for all public blocking APIs.
*
* Read [`concepts`][docs::concepts] for know more about [`Operator`].
*
* # Examples
*
* ## Init backends
*
* Read more backend init examples in [`services`]
*
* ```rust,no_run
* # use anyhow::Result;
* use opendal::services::Fs;
* use opendal::BlockingOperator;
* use opendal::Operator;
*
* fn main() -> Result<()> {
* // Create fs backend builder.
* let builder = Fs::default().root("/tmp");
*
* // Build an `BlockingOperator` to start operating the storage.
* let _: BlockingOperator = Operator::new(builder)?.finish().blocking();
*
* Ok(())
* }
* ```
*
* ## Init backends with blocking layer
*
* Some services like s3, gcs doesn't have native blocking supports, we can use [`layers::BlockingLayer`]
* to wrap the async operator to make it blocking.
* # use anyhow::Result;
* use opendal::layers::BlockingLayer;
* use opendal::services::S3;
* use opendal::BlockingOperator;
* use opendal::Operator;
*
* async fn test() -> Result<()> {
* // Create fs backend builder.
* let mut builder = S3::default().bucket("test").region("us-east-1");
*
* // Build an `BlockingOperator` with blocking layer to start operating the storage.
* let _: BlockingOperator = Operator::new(builder)?
* .layer(BlockingLayer::create()?)
* .finish()
* .blocking();
*
* Ok(())
* }
* ```
*/
typedef struct BlockingOperator BlockingOperator;

/**
* \brief opendal_bytes carries raw-bytes with its length
*
Expand Down Expand Up @@ -188,6 +134,10 @@ typedef struct opendal_error {
* @see opendal_list_entry_name()
*/
typedef struct opendal_entry {
/**
* The pointer to the opendal::Entry in the Rust code.
* Only touch this on judging whether it is NULL.
*/
void *inner;
} opendal_entry;

Expand Down Expand Up @@ -219,6 +169,10 @@ typedef struct opendal_result_lister_next {
* @see opendal_operator_list()
*/
typedef struct opendal_lister {
/**
* The pointer to the opendal::BlockingLister in the Rust code.
* Only touch this on judging whether it is NULL.
*/
void *inner;
} opendal_lister;

Expand Down Expand Up @@ -259,7 +213,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 struct BlockingOperator *ptr;
const void *inner;
} opendal_operator;

/**
Expand Down Expand Up @@ -297,7 +251,7 @@ typedef struct opendal_result_operator_new {
*/
typedef struct opendal_operator_options {
/**
* The pointer to the Rust HashMap<String, String>
* The pointer to the HashMap<String, String> in the Rust code.
* Only touch this on judging whether it is NULL.
*/
void *inner;
Expand Down Expand Up @@ -329,6 +283,10 @@ typedef struct opendal_result_read {
* a opendal::BlockingReader, which is inside the Rust core code.
*/
typedef struct opendal_reader {
/**
* The pointer to the opendal::StdReader in the Rust code.
* Only touch this on judging whether it is NULL.
*/
void *inner;
} opendal_reader;

Expand All @@ -355,6 +313,10 @@ typedef struct opendal_result_operator_reader {
* an opendal::BlockingWriter, which is inside the Rust core code.
*/
typedef struct opendal_writer {
/**
* The pointer to the opendal::BlockingWriter in the Rust code.
* Only touch this on judging whether it is NULL.
*/
void *inner;
} opendal_writer;

Expand Down Expand Up @@ -436,6 +398,10 @@ typedef struct opendal_result_list {
* of operator.
*/
typedef struct opendal_operator_info {
/**
* The pointer to the opendal::OperatorInfo in the Rust code.
* Only touch this on judging whether it is NULL.
*/
void *inner;
} opendal_operator_info;

Expand Down Expand Up @@ -658,7 +624,7 @@ void opendal_error_free(struct opendal_error *ptr);
* For examples, please see the comment section of opendal_operator_list()
* @see opendal_operator_list()
*/
struct opendal_result_lister_next opendal_lister_next(const struct opendal_lister *self);
struct opendal_result_lister_next opendal_lister_next(struct opendal_lister *self);

/**
* \brief Free the heap-allocated metadata used by opendal_lister
Expand Down Expand Up @@ -752,7 +718,7 @@ int64_t opendal_metadata_last_modified_ms(const struct opendal_metadata *self);
* opendal_operator_free(op);
* ```
*/
void opendal_operator_free(const struct opendal_operator *op);
void opendal_operator_free(const struct opendal_operator *ptr);

/**
* \brief Construct an operator based on `scheme` and `options`
Expand Down Expand Up @@ -1343,7 +1309,7 @@ struct opendal_capability opendal_operator_info_get_native_capability(const stru
/**
* \brief Frees the heap memory used by the opendal_bytes
*/
void opendal_bytes_free(struct opendal_bytes *bs);
void opendal_bytes_free(struct opendal_bytes *ptr);

/**
* \brief Construct a heap-allocated opendal_operator_options
Expand Down Expand Up @@ -1411,7 +1377,7 @@ void opendal_entry_free(struct opendal_entry *ptr);
/**
* \brief Read data from the reader.
*/
struct opendal_result_reader_read opendal_reader_read(const struct opendal_reader *self,
struct opendal_result_reader_read opendal_reader_read(struct opendal_reader *self,
uint8_t *buf,
uintptr_t len);

Expand All @@ -1423,7 +1389,7 @@ void opendal_reader_free(struct opendal_reader *ptr);
/**
* \brief Write data to the writer.
*/
struct opendal_result_writer_write opendal_writer_write(const struct opendal_writer *self,
struct opendal_result_writer_write opendal_writer_write(struct opendal_writer *self,
struct opendal_bytes bytes);

/**
Expand Down
6 changes: 4 additions & 2 deletions bindings/c/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ use ::opendal as core;
/// @see opendal_list_entry_name()
#[repr(C)]
pub struct opendal_entry {
/// The pointer to the opendal::Entry in the Rust code.
/// Only touch this on judging whether it is NULL.
inner: *mut c_void,
}

Expand Down Expand Up @@ -77,8 +79,8 @@ impl opendal_entry {
#[no_mangle]
pub unsafe extern "C" fn opendal_entry_free(ptr: *mut opendal_entry) {
if !ptr.is_null() {
let _ = Box::from_raw((*ptr).inner as *mut core::Entry);
let _ = Box::from_raw(ptr);
drop(Box::from_raw((*ptr).inner as *mut core::Entry));
drop(Box::from_raw(ptr));
}
}
}
19 changes: 12 additions & 7 deletions bindings/c/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ impl From<core::ErrorKind> for opendal_code {
core::ErrorKind::RangeNotSatisfied => opendal_code::OPENDAL_RANGE_NOT_SATISFIED,
// if this is triggered, check the [`core`] crate and add a
// new error code accordingly
_ => panic!("The newly added ErrorKind in core crate is not handled in C bindings"),
_ => unimplemented!(
"The newly added ErrorKind in core crate is not handled in C bindings"
),
}
}
}
Expand Down Expand Up @@ -112,16 +114,19 @@ 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 as *const opendal_bytes as *mut opendal_bytes;
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 = unsafe { (*message_ptr).data as *mut u8 };
let _ = unsafe {
Vec::from_raw_parts(data_mut, (*message_ptr).len, (*message_ptr).len)
};
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
let _ = unsafe { Box::from_raw(ptr) };
drop(Box::from_raw(ptr))
}
}
}
10 changes: 6 additions & 4 deletions bindings/c/src/lister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ use super::*;
/// @see opendal_operator_list()
#[repr(C)]
pub struct opendal_lister {
/// The pointer to the opendal::BlockingLister in the Rust code.
/// Only touch this on judging whether it is NULL.
inner: *mut c_void,
}

impl opendal_lister {
fn deref_mut(&self) -> &mut core::BlockingLister {
fn deref_mut(&mut self) -> &mut core::BlockingLister {
// Safety: the inner should never be null once constructed
// The use-after-free is undefined behavior
unsafe { &mut *(self.inner as *mut core::BlockingLister) }
Expand All @@ -55,7 +57,7 @@ impl opendal_lister {
/// For examples, please see the comment section of opendal_operator_list()
/// @see opendal_operator_list()
#[no_mangle]
pub unsafe extern "C" fn opendal_lister_next(&self) -> opendal_result_lister_next {
pub unsafe extern "C" fn opendal_lister_next(&mut self) -> opendal_result_lister_next {
let e = self.deref_mut().next();
if e.is_none() {
return opendal_result_lister_next {
Expand Down Expand Up @@ -83,8 +85,8 @@ impl opendal_lister {
#[no_mangle]
pub unsafe extern "C" fn opendal_lister_free(ptr: *mut opendal_lister) {
if !ptr.is_null() {
let _ = Box::from_raw((*ptr).inner as *mut core::BlockingLister);
let _ = Box::from_raw(ptr);
drop(Box::from_raw((*ptr).inner as *mut core::BlockingLister));
drop(Box::from_raw(ptr));
}
}
}
4 changes: 2 additions & 2 deletions bindings/c/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ impl opendal_metadata {
#[no_mangle]
pub unsafe extern "C" fn opendal_metadata_free(ptr: *mut opendal_metadata) {
if !ptr.is_null() {
let _ = Box::from_raw((*ptr).inner as *mut core::Metadata);
let _ = Box::from_raw(ptr);
drop(Box::from_raw((*ptr).inner as *mut core::Metadata));
drop(Box::from_raw(ptr));
}
}

Expand Down
Loading
Loading