Skip to content

refactor(bindings/C): refactor c bindings to call all APIs using pointer #2489

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
Xuanwo merged 4 commits into from
Jun 18, 2023
Merged

refactor(bindings/C): refactor c bindings to call all APIs using pointer #2489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c344efe
refactor the code to use pointers only
xyjixyjixyji Jun 18, 2023
256c284
reflect that in doc
xyjixyjixyji Jun 18, 2023
c635d63
fix sigsegv
xyjixyjixyji Jun 18, 2023
2a72f2c
no debug flag
xyjixyjixyji Jun 18, 2023
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
53 changes: 27 additions & 26 deletions bindings/c/include/opendal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ typedef struct opendal_result_stat {
/**
* The metadata output of the stat
*/
struct opendal_metadata meta;
const struct opendal_metadata *meta;
/**
* The error code, should be OPENDAL_OK if succeeds
*/
Expand Down Expand Up @@ -287,15 +287,15 @@ extern "C" {
* Following is an example.
* ```C
* // Allocate a new options
* opendal_operator_options options = opendal_operator_options_new();
* opendal_operator_options *options = opendal_operator_options_new();
* // Set the options you need
* opendal_operator_options_set(&options, "root", "/myroot");
* opendal_operator_options_set(options, "root", "/myroot");
*
* // Construct the operator based on the options and scheme
* opendal_operator_ptr ptr = opendal_operator_new("memory", &options);
* const opendal_operator_ptr *ptr = opendal_operator_new("memory", options);
*
* // you could free the options right away since the options is not used afterwards
* opendal_operator_options_free(&options);
* opendal_operator_options_free(options);
*
* // ... your operations
* ```
Expand All @@ -307,8 +307,8 @@ extern "C" {
* the string.
* * The `scheme` points to NULL, this function simply returns you a null opendal_operator_ptr.
*/
struct opendal_operator_ptr opendal_operator_new(const char *scheme,
const struct opendal_operator_options *options);
const struct opendal_operator_ptr *opendal_operator_new(const char *scheme,
const struct opendal_operator_options *options);

/**
* \brief Blockingly write raw bytes to `path`.
Expand Down Expand Up @@ -353,7 +353,7 @@ struct opendal_operator_ptr opendal_operator_new(const char *scheme,
*
* * If the `path` points to NULL, this function panics, i.e. exits with information
*/
enum opendal_code opendal_operator_blocking_write(struct opendal_operator_ptr ptr,
enum opendal_code opendal_operator_blocking_write(const struct opendal_operator_ptr *ptr,
const char *path,
struct opendal_bytes bytes);

Expand Down Expand Up @@ -398,7 +398,7 @@ enum opendal_code opendal_operator_blocking_write(struct opendal_operator_ptr pt
*
* * If the `path` points to NULL, this function panics, i.e. exits with information
*/
struct opendal_result_read opendal_operator_blocking_read(struct opendal_operator_ptr ptr,
struct opendal_result_read opendal_operator_blocking_read(const struct opendal_operator_ptr *ptr,
const char *path);

/**
Expand Down Expand Up @@ -441,7 +441,7 @@ struct opendal_result_read opendal_operator_blocking_read(struct opendal_operato
*
* * If the `path` points to NULL, this function panics, i.e. exits with information
*/
enum opendal_code opendal_operator_blocking_delete(struct opendal_operator_ptr ptr,
enum opendal_code opendal_operator_blocking_delete(const struct opendal_operator_ptr *ptr,
const char *path);

/**
Expand Down Expand Up @@ -485,7 +485,7 @@ enum opendal_code opendal_operator_blocking_delete(struct opendal_operator_ptr p
*
* * If the `path` points to NULL, this function panics, i.e. exits with information
*/
struct opendal_result_is_exist opendal_operator_is_exist(struct opendal_operator_ptr ptr,
struct opendal_result_is_exist opendal_operator_is_exist(const struct opendal_operator_ptr *ptr,
const char *path);

/**
Expand All @@ -512,7 +512,7 @@ struct opendal_result_is_exist opendal_operator_is_exist(struct opendal_operator
* opendal_result_stat s = opendal_operator_stat(ptr, "/testpath");
* assert(s.code == OPENDAL_OK);
*
* opendal_metadata meta = s.meta;
* const opendal_metadata *meta = s.meta;
*
* // ... you could now use your metadata, notice that please only access metadata
* // using the APIs provided by OpenDAL
Expand All @@ -528,7 +528,8 @@ struct opendal_result_is_exist opendal_operator_is_exist(struct opendal_operator
*
* * If the `path` points to NULL, this function panics, i.e. exits with information
*/
struct opendal_result_stat opendal_operator_stat(struct opendal_operator_ptr ptr, const char *path);
struct opendal_result_stat opendal_operator_stat(const struct opendal_operator_ptr *ptr,
const char *path);

/**
* \brief Free the heap-allocated operator pointed by opendal_operator_ptr.
Expand All @@ -540,14 +541,14 @@ struct opendal_result_stat opendal_operator_stat(struct opendal_operator_ptr ptr
* # Example
*
* ```C
* opendal_operator_ptr ptr = opendal_operator_new("fs", NULL);
* opendal_operator_ptr *ptr = opendal_operator_new("fs", NULL);
* // ... use this ptr, maybe some reads and writes
*
* // free this operator
* opendal_operator_free(&ptr);
* opendal_operator_free(ptr);
* ```
*/
void opendal_operator_free(const struct opendal_operator_ptr *self);
void opendal_operator_free(const struct opendal_operator_ptr *op);

/**
* \brief Frees the heap memory used by the opendal_bytes
Expand All @@ -568,8 +569,8 @@ void opendal_metadata_free(const struct opendal_metadata *self);
* opendal_result_stat s = opendal_operator_stat(ptr, "/testpath");
* assert(s.code == OPENDAL_OK);
*
* opendal_metadata meta = s.meta;
* assert(opendal_metadata_content_length(&meta) == 13);
* opendal_metadata *meta = s.meta;
* assert(opendal_metadata_content_length(meta) == 13);
* ```
*/
uint64_t opendal_metadata_content_length(const struct opendal_metadata *self);
Expand All @@ -583,8 +584,8 @@ uint64_t opendal_metadata_content_length(const struct opendal_metadata *self);
* opendal_result_stat s = opendal_operator_stat(ptr, "/testpath");
* assert(s.code == OPENDAL_OK);
*
* opendal_metadata meta = s.meta;
* assert(opendal_metadata_is_file(&meta));
* opendal_metadata *meta = s.meta;
* assert(opendal_metadata_is_file(meta));
* ```
*/
bool opendal_metadata_is_file(const struct opendal_metadata *self);
Expand All @@ -598,10 +599,10 @@ bool opendal_metadata_is_file(const struct opendal_metadata *self);
* opendal_result_stat s = opendal_operator_stat(ptr, "/testpath");
* assert(s.code == OPENDAL_OK);
*
* opendal_metadata meta = s.meta;
* opendal_metadata *meta = s.meta;
*
* // this is not a directory
* assert(!opendal_metadata_is_dir(&meta));
* assert(!opendal_metadata_is_dir(meta));
* ```
*
* \todo This is not a very clear example. A clearer example will be added
Expand All @@ -617,7 +618,7 @@ bool opendal_metadata_is_dir(const struct opendal_metadata *self);
*
* @see opendal_operator_option_set
*/
struct opendal_operator_options opendal_operator_options_new(void);
struct opendal_operator_options *opendal_operator_options_new(void);

/**
* \brief Set a Key-Value pair inside opendal_operator_options
Expand All @@ -630,8 +631,8 @@ struct opendal_operator_options opendal_operator_options_new(void);
* # Example
*
* ```C
* opendal_operator_options options = opendal_operator_options_new();
* opendal_operator_options_set(&options, "root", "/myroot");
* opendal_operator_options *options = opendal_operator_options_new();
* opendal_operator_options_set(options, "root", "/myroot");
*
* // .. use your opendal_operator_options
*
Expand All @@ -645,7 +646,7 @@ void opendal_operator_options_set(struct opendal_operator_options *self,
/**
* \brief Free the allocated memory used by [`opendal_operator_options`]
*/
void opendal_operator_options_free(const struct opendal_operator_options *self);
void opendal_operator_options_free(const struct opendal_operator_options *options);

#ifdef __cplusplus
} // extern "C"
Expand Down
44 changes: 22 additions & 22 deletions bindings/c/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ use crate::types::{
/// Following is an example.
/// ```C
/// // Allocate a new options
/// opendal_operator_options options = opendal_operator_options_new();
/// opendal_operator_options *options = opendal_operator_options_new();
/// // Set the options you need
/// opendal_operator_options_set(&options, "root", "/myroot");
/// opendal_operator_options_set(options, "root", "/myroot");
///
/// // Construct the operator based on the options and scheme
/// opendal_operator_ptr ptr = opendal_operator_new("memory", &options);
/// const opendal_operator_ptr *ptr = opendal_operator_new("memory", options);
///
/// // you could free the options right away since the options is not used afterwards
/// opendal_operator_options_free(&options);
/// opendal_operator_options_free(options);
///
/// // ... your operations
/// ```
Expand All @@ -77,16 +77,16 @@ use crate::types::{
pub unsafe extern "C" fn opendal_operator_new(
scheme: *const c_char,
options: *const opendal_operator_options,
) -> opendal_operator_ptr {
) -> *const opendal_operator_ptr {
if scheme.is_null() {
return opendal_operator_ptr::null();
return std::ptr::null();
}

let scheme_str = unsafe { std::ffi::CStr::from_ptr(scheme).to_str().unwrap() };
let scheme = match od::Scheme::from_str(scheme_str) {
Ok(s) => s,
Err(_) => {
return opendal_operator_ptr::null();
return std::ptr::null();
}
};

Expand All @@ -100,14 +100,14 @@ pub unsafe extern "C" fn opendal_operator_new(
let op = match od::Operator::via_map(scheme, map) {
Ok(o) => o.blocking(),
Err(_) => {
return opendal_operator_ptr::null();
return std::ptr::null();
}
};

// this prevents the operator memory from being dropped by the Box
let op = Box::leak(Box::new(op));
let op = opendal_operator_ptr::from(Box::leak(Box::new(op)));

opendal_operator_ptr::from(op)
Box::leak(Box::new(op))
}

/// \brief Blockingly write raw bytes to `path`.
Expand Down Expand Up @@ -153,15 +153,15 @@ pub unsafe extern "C" fn opendal_operator_new(
/// * If the `path` points to NULL, this function panics, i.e. exits with information
#[no_mangle]
pub unsafe extern "C" fn opendal_operator_blocking_write(
ptr: opendal_operator_ptr,
ptr: *const opendal_operator_ptr,
path: *const c_char,
bytes: opendal_bytes,
) -> opendal_code {
if path.is_null() {
panic!("The path given is pointing at NULL");
}

let op = ptr.as_ref();
let op = (*ptr).as_ref();
let path = unsafe { std::ffi::CStr::from_ptr(path).to_str().unwrap() };
match op.write(path, bytes) {
Ok(_) => opendal_code::OPENDAL_OK,
Expand Down Expand Up @@ -210,14 +210,14 @@ pub unsafe extern "C" fn opendal_operator_blocking_write(
/// * If the `path` points to NULL, this function panics, i.e. exits with information
#[no_mangle]
pub unsafe extern "C" fn opendal_operator_blocking_read(
ptr: opendal_operator_ptr,
ptr: *const opendal_operator_ptr,
path: *const c_char,
) -> opendal_result_read {
if path.is_null() {
panic!("The path given is pointing at NULL");
}

let op = ptr.as_ref();
let op = (*ptr).as_ref();
let path = unsafe { std::ffi::CStr::from_ptr(path).to_str().unwrap() };
let data = op.read(path);
match data {
Expand Down Expand Up @@ -275,14 +275,14 @@ pub unsafe extern "C" fn opendal_operator_blocking_read(
/// * If the `path` points to NULL, this function panics, i.e. exits with information
#[no_mangle]
pub unsafe extern "C" fn opendal_operator_blocking_delete(
ptr: opendal_operator_ptr,
ptr: *const opendal_operator_ptr,
path: *const c_char,
) -> opendal_code {
if path.is_null() {
panic!("The path given is pointing at NULL");
}

let op = ptr.as_ref();
let op = (*ptr).as_ref();
let path = unsafe { std::ffi::CStr::from_ptr(path).to_str().unwrap() };
match op.delete(path) {
Ok(_) => opendal_code::OPENDAL_OK,
Expand Down Expand Up @@ -331,14 +331,14 @@ pub unsafe extern "C" fn opendal_operator_blocking_delete(
/// * If the `path` points to NULL, this function panics, i.e. exits with information
#[no_mangle]
pub unsafe extern "C" fn opendal_operator_is_exist(
ptr: opendal_operator_ptr,
ptr: *const opendal_operator_ptr,
path: *const c_char,
) -> opendal_result_is_exist {
if path.is_null() {
panic!("The path given is pointing at NULL");
}

let op = ptr.as_ref();
let op = (*ptr).as_ref();
let path = unsafe { std::ffi::CStr::from_ptr(path).to_str().unwrap() };
match op.is_exist(path) {
Ok(e) => opendal_result_is_exist {
Expand Down Expand Up @@ -375,7 +375,7 @@ pub unsafe extern "C" fn opendal_operator_is_exist(
/// opendal_result_stat s = opendal_operator_stat(ptr, "/testpath");
/// assert(s.code == OPENDAL_OK);
///
/// opendal_metadata meta = s.meta;
/// const opendal_metadata *meta = s.meta;
///
/// // ... you could now use your metadata, notice that please only access metadata
/// // using the APIs provided by OpenDAL
Expand All @@ -392,22 +392,22 @@ pub unsafe extern "C" fn opendal_operator_is_exist(
/// * If the `path` points to NULL, this function panics, i.e. exits with information
#[no_mangle]
pub unsafe extern "C" fn opendal_operator_stat(
ptr: opendal_operator_ptr,
ptr: *const opendal_operator_ptr,
path: *const c_char,
) -> opendal_result_stat {
if path.is_null() {
panic!("The path given is pointing at NULL");
}

let op = ptr.as_ref();
let op = (*ptr).as_ref();
let path = unsafe { std::ffi::CStr::from_ptr(path).to_str().unwrap() };
match op.stat(path) {
Ok(m) => opendal_result_stat {
meta: opendal_metadata::from_metadata(m),
code: opendal_code::OPENDAL_OK,
},
Err(err) => opendal_result_stat {
meta: opendal_metadata::null(),
meta: std::ptr::null(),
code: opendal_code::from_opendal_error(err),
},
}
Expand Down
2 changes: 1 addition & 1 deletion bindings/c/src/result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub struct opendal_result_is_exist {
#[repr(C)]
pub struct opendal_result_stat {
/// The metadata output of the stat
pub meta: opendal_metadata,
pub meta: *const opendal_metadata,
/// The error code, should be OPENDAL_OK if succeeds
pub code: opendal_code,
}
Loading