Skip to content

Commit

Permalink
refactor: more consistent C binding pattern (#5162)
Browse files Browse the repository at this point in the history
Signed-off-by: tison <wander4096@gmail.com>
  • Loading branch information
tisonkun authored Oct 6, 2024
1 parent d3e2f77 commit d74bf7c
Show file tree
Hide file tree
Showing 10 changed files with 192 additions and 256 deletions.
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

0 comments on commit d74bf7c

Please sign in to comment.