Skip to content
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.

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

safety: introduce pointer types and their restrictions #208

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion scylla-rust-wrapper/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ ntest = "0.9.3"
rusty-fork = "0.3.0"
[lib]
name = "scylla_cpp_driver"
crate-type = ["cdylib", "staticlib"]
crate-type = ["cdylib", "staticlib", "lib"]

[profile.dev]
panic = "abort"
Expand Down
563 changes: 520 additions & 43 deletions scylla-rust-wrapper/src/argconv.rs

Large diffs are not rendered by default.

54 changes: 29 additions & 25 deletions scylla-rust-wrapper/src/batch.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::argconv::{ArcFFI, BoxFFI};
use crate::argconv::{
ArcFFI, BoxFFI, CassBorrowedMutPtr, CassBorrowedPtr, CassOwnedMutPtr, FromBox, FFI,
};
use crate::cass_error::CassError;
use crate::cass_types::CassConsistency;
use crate::cass_types::{make_batch_type, CassBatchType};
Expand All @@ -19,7 +21,9 @@ pub struct CassBatch {
pub(crate) exec_profile: Option<PerStatementExecProfile>,
}

impl BoxFFI for CassBatch {}
impl FFI for CassBatch {
type Origin = FromBox;
}

#[derive(Clone)]
pub struct CassBatchState {
Expand All @@ -28,7 +32,7 @@ pub struct CassBatchState {
}

#[no_mangle]
pub unsafe extern "C" fn cass_batch_new(type_: CassBatchType) -> *mut CassBatch {
pub unsafe extern "C" fn cass_batch_new(type_: CassBatchType) -> CassOwnedMutPtr<CassBatch> {
if let Some(batch_type) = make_batch_type(type_) {
BoxFFI::into_ptr(Box::new(CassBatch {
state: Arc::new(CassBatchState {
Expand All @@ -39,21 +43,21 @@ pub unsafe extern "C" fn cass_batch_new(type_: CassBatchType) -> *mut CassBatch
exec_profile: None,
}))
} else {
std::ptr::null_mut()
BoxFFI::null_mut()
}
}

#[no_mangle]
pub unsafe extern "C" fn cass_batch_free(batch: *mut CassBatch) {
pub unsafe extern "C" fn cass_batch_free(batch: CassOwnedMutPtr<CassBatch>) {
BoxFFI::free(batch);
}

#[no_mangle]
pub unsafe extern "C" fn cass_batch_set_consistency(
batch: *mut CassBatch,
batch: CassBorrowedMutPtr<CassBatch>,
consistency: CassConsistency,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();
let consistency = match consistency.try_into().ok() {
Some(c) => c,
None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
Expand All @@ -67,10 +71,10 @@ pub unsafe extern "C" fn cass_batch_set_consistency(

#[no_mangle]
pub unsafe extern "C" fn cass_batch_set_serial_consistency(
batch: *mut CassBatch,
batch: CassBorrowedMutPtr<CassBatch>,
serial_consistency: CassConsistency,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();
let serial_consistency = match serial_consistency.try_into().ok() {
Some(c) => c,
None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
Expand All @@ -84,13 +88,13 @@ pub unsafe extern "C" fn cass_batch_set_serial_consistency(

#[no_mangle]
pub unsafe extern "C" fn cass_batch_set_retry_policy(
batch: *mut CassBatch,
retry_policy: *const CassRetryPolicy,
batch: CassBorrowedMutPtr<CassBatch>,
retry_policy: CassBorrowedPtr<CassRetryPolicy>,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();

let maybe_arced_retry_policy: Option<Arc<dyn scylla::retry_policy::RetryPolicy>> =
ArcFFI::as_maybe_ref(retry_policy).map(|policy| match policy {
ArcFFI::as_ref(retry_policy).map(|policy| match policy {
CassRetryPolicy::DefaultRetryPolicy(default) => {
default.clone() as Arc<dyn scylla::retry_policy::RetryPolicy>
}
Expand All @@ -107,10 +111,10 @@ pub unsafe extern "C" fn cass_batch_set_retry_policy(

#[no_mangle]
pub unsafe extern "C" fn cass_batch_set_timestamp(
batch: *mut CassBatch,
batch: CassBorrowedMutPtr<CassBatch>,
timestamp: cass_int64_t,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();

Arc::make_mut(&mut batch.state)
.batch
Expand All @@ -121,21 +125,21 @@ pub unsafe extern "C" fn cass_batch_set_timestamp(

#[no_mangle]
pub unsafe extern "C" fn cass_batch_set_request_timeout(
batch: *mut CassBatch,
batch: CassBorrowedMutPtr<CassBatch>,
timeout_ms: cass_uint64_t,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();
batch.batch_request_timeout_ms = Some(timeout_ms);

CassError::CASS_OK
}

#[no_mangle]
pub unsafe extern "C" fn cass_batch_set_is_idempotent(
batch: *mut CassBatch,
batch: CassBorrowedMutPtr<CassBatch>,
is_idempotent: cass_bool_t,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();
Arc::make_mut(&mut batch.state)
.batch
.set_is_idempotent(is_idempotent != 0);
Expand All @@ -145,10 +149,10 @@ pub unsafe extern "C" fn cass_batch_set_is_idempotent(

#[no_mangle]
pub unsafe extern "C" fn cass_batch_set_tracing(
batch: *mut CassBatch,
batch: CassBorrowedMutPtr<CassBatch>,
enabled: cass_bool_t,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();
Arc::make_mut(&mut batch.state)
.batch
.set_tracing(enabled != 0);
Expand All @@ -158,12 +162,12 @@ pub unsafe extern "C" fn cass_batch_set_tracing(

#[no_mangle]
pub unsafe extern "C" fn cass_batch_add_statement(
batch: *mut CassBatch,
statement: *const CassStatement,
batch: CassBorrowedMutPtr<CassBatch>,
statement: CassBorrowedPtr<CassStatement>,
) -> CassError {
let batch = BoxFFI::as_mut_ref(batch);
let batch = BoxFFI::as_mut_ref(batch).unwrap();
let state = Arc::make_mut(&mut batch.state);
let statement = BoxFFI::as_ref(statement);
let statement = BoxFFI::as_ref(statement).unwrap();

match &statement.statement {
BoundStatement::Simple(q) => {
Expand Down
38 changes: 20 additions & 18 deletions scylla-rust-wrapper/src/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ macro_rules! make_index_binder {
#[no_mangle]
#[allow(clippy::redundant_closure_call)]
pub unsafe extern "C" fn $fn_by_idx(
this: *mut $this,
this: CassBorrowedMutPtr<$this>,
index: size_t,
$($arg: $t), *
) -> CassError {
// For some reason detected as unused, which is not true
#[allow(unused_imports)]
use crate::value::CassCqlValue::*;
match ($e)($($arg), *) {
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), index as usize, v),
Ok(v) => $consume_v(BoxFFI::as_mut_ref( this).unwrap(), index as usize, v),
Err(e) => e,
}
}
Expand All @@ -73,7 +73,7 @@ macro_rules! make_name_binder {
#[no_mangle]
#[allow(clippy::redundant_closure_call)]
pub unsafe extern "C" fn $fn_by_name(
this: *mut $this,
this: CassBorrowedMutPtr<$this>,
name: *const c_char,
$($arg: $t), *
) -> CassError {
Expand All @@ -82,7 +82,7 @@ macro_rules! make_name_binder {
use crate::value::CassCqlValue::*;
let name = ptr_to_cstr(name).unwrap();
match ($e)($($arg), *) {
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), name, v),
Ok(v) => $consume_v(BoxFFI::as_mut_ref( this).unwrap(), name, v),
Err(e) => e,
}
}
Expand All @@ -94,7 +94,7 @@ macro_rules! make_name_n_binder {
#[no_mangle]
#[allow(clippy::redundant_closure_call)]
pub unsafe extern "C" fn $fn_by_name_n(
this: *mut $this,
this: CassBorrowedMutPtr<$this>,
name: *const c_char,
name_length: size_t,
$($arg: $t), *
Expand All @@ -104,7 +104,7 @@ macro_rules! make_name_n_binder {
use crate::value::CassCqlValue::*;
let name = ptr_to_cstr_n(name, name_length).unwrap();
match ($e)($($arg), *) {
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), name, v),
Ok(v) => $consume_v(BoxFFI::as_mut_ref( this).unwrap(), name, v),
Err(e) => e,
}
}
Expand All @@ -116,14 +116,14 @@ macro_rules! make_appender {
#[no_mangle]
#[allow(clippy::redundant_closure_call)]
pub unsafe extern "C" fn $fn_append(
this: *mut $this,
this: CassBorrowedMutPtr<$this>,
$($arg: $t), *
) -> CassError {
// For some reason detected as unused, which is not true
#[allow(unused_imports)]
use crate::value::CassCqlValue::*;
match ($e)($($arg), *) {
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), v),
Ok(v) => $consume_v(BoxFFI::as_mut_ref( this).unwrap(), v),
Err(e) => e,
}
}
Expand Down Expand Up @@ -302,33 +302,35 @@ macro_rules! invoke_binder_maker_macro_with_type {
$this,
$consume_v,
$fn,
|p: *const crate::collection::CassCollection| {
match std::convert::TryInto::try_into(BoxFFI::as_ref(p)) {
|p: CassBorrowedPtr<crate::collection::CassCollection>| {
match std::convert::TryInto::try_into(BoxFFI::as_ref(p).unwrap()) {
Ok(v) => Ok(Some(v)),
Err(_) => Err(CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE),
}
},
[p @ *const crate::collection::CassCollection]
[p @ CassBorrowedPtr<crate::collection::CassCollection>]
);
};
(tuple, $macro_name:ident, $this:ty, $consume_v:expr, $fn:ident) => {
$macro_name!(
$this,
$consume_v,
$fn,
|p: *const crate::tuple::CassTuple| {
Ok(Some(BoxFFI::as_ref(p).into()))
|p: CassBorrowedPtr<crate::tuple::CassTuple>| {
Ok(Some(BoxFFI::as_ref(p).unwrap().into()))
},
[p @ *const crate::tuple::CassTuple]
[p @ CassBorrowedPtr<crate::tuple::CassTuple>]
);
};
(user_type, $macro_name:ident, $this:ty, $consume_v:expr, $fn:ident) => {
$macro_name!(
$this,
$consume_v,
$fn,
|p: *const crate::user_type::CassUserType| Ok(Some(BoxFFI::as_ref(p).into())),
[p @ *const crate::user_type::CassUserType]
|p: CassBorrowedPtr<crate::user_type::CassUserType>| {
Ok(Some(BoxFFI::as_ref(p).unwrap().into()))
},
[p @ CassBorrowedPtr<crate::user_type::CassUserType>]
);
};
}
Expand Down Expand Up @@ -371,13 +373,13 @@ macro_rules! invoke_binder_maker_macro_with_type {
/// There are also 2 helper variants, to accomodate scenarios often encountered in cppdriver (sets of 3 functions,
/// binding the same type by index, name and name_n):
/// * `make_binders!(type, fn_idx, fn_name, fn_name_n)` - is equivalent to:
/// ```
/// ```text
/// make_binders!(@index type, fn_idx);
/// make_binders!(@name type, fn_name);
/// make_binders!(@name_n type, fn_name_n);
/// ```
/// * `make_binders!(t1, fn_idx, t2, fn_name, t3, fn_name_n)` - is equivalent to:
/// ```
/// ```text
/// make_binders!(@index t1, fn_idx);
/// make_binders!(@name t2, fn_name);
/// make_binders!(@name_n t3, fn_name_n);
Expand Down
Loading