Skip to content

Commit bfae66b

Browse files
muzarskimuzarski
authored
Merge pull request #207 from muzarski/type-safety
type safety: ffi pointer manipulation API
2 parents 81e668e + dd80a17 commit bfae66b

24 files changed

+946
-675
lines changed

‎scylla-rust-wrapper/clippy.toml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
disallowed-methods = [
2+
"std::boxed::Box::from_raw",
3+
"std::boxed::Box::from_raw_in",
4+
"std::boxed::Box::into_raw",
5+
"std::boxed::Box::into_raw_with_allocator",
6+
7+
"std::sync::Arc::as_ptr",
8+
"std::sync::Arc::decrement_strong_count",
9+
"std::sync::Arc::from_raw",
10+
"std::sync::Arc::increment_strong_count",
11+
"std::sync::Arc::into_raw",
12+
13+
"std::rc::Rc::as_ptr",
14+
"std::rc::Rc::decrement_strong_count",
15+
"std::rc::Rc::from_raw",
16+
"std::rc::Rc::increment_strong_count",
17+
"std::rc::Rc::into_raw",
18+
19+
"const_ptr::as_ref",
20+
"mut_ptr::as_mut"
21+
]

‎scylla-rust-wrapper/src/argconv.rs

Lines changed: 87 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,6 @@ use std::ffi::CStr;
44
use std::os::raw::c_char;
55
use std::sync::Arc;
66

7-
pub unsafe fn ptr_to_ref<T>(ptr: *const T) -> &'static T {
8-
ptr.as_ref().unwrap()
9-
}
10-
11-
pub unsafe fn ptr_to_ref_mut<T>(ptr: *mut T) -> &'static mut T {
12-
ptr.as_mut().unwrap()
13-
}
14-
15-
pub unsafe fn free_boxed<T>(ptr: *mut T) {
16-
if !ptr.is_null() {
17-
// This takes the ownership of the boxed value and drops it
18-
let _ = Box::from_raw(ptr);
19-
}
20-
}
21-
22-
pub unsafe fn clone_arced<T>(ptr: *const T) -> Arc<T> {
23-
Arc::increment_strong_count(ptr);
24-
Arc::from_raw(ptr)
25-
}
26-
27-
pub unsafe fn free_arced<T>(ptr: *const T) {
28-
if !ptr.is_null() {
29-
// This decrements the arc's internal counter and potentially drops it
30-
Arc::from_raw(ptr);
31-
}
32-
}
33-
347
pub unsafe fn ptr_to_cstr(ptr: *const c_char) -> Option<&'static str> {
358
CStr::from_ptr(ptr).to_str().ok()
369
}
@@ -97,3 +70,90 @@ macro_rules! make_c_str {
9770

9871
#[cfg(test)]
9972
pub(crate) use make_c_str;
73+
74+
/// Defines a pointer manipulation API for non-shared heap-allocated data.
75+
///
76+
/// Implement this trait for types that are allocated by the driver via [`Box::new`],
77+
/// and then returned to the user as a pointer. The user is responsible for freeing
78+
/// the memory associated with the pointer using corresponding driver's API function.
79+
pub trait BoxFFI {
80+
fn into_ptr(self: Box<Self>) -> *mut Self {
81+
#[allow(clippy::disallowed_methods)]
82+
Box::into_raw(self)
83+
}
84+
unsafe fn from_ptr(ptr: *mut Self) -> Box<Self> {
85+
#[allow(clippy::disallowed_methods)]
86+
Box::from_raw(ptr)
87+
}
88+
unsafe fn as_maybe_ref<'a>(ptr: *const Self) -> Option<&'a Self> {
89+
#[allow(clippy::disallowed_methods)]
90+
ptr.as_ref()
91+
}
92+
unsafe fn as_ref<'a>(ptr: *const Self) -> &'a Self {
93+
#[allow(clippy::disallowed_methods)]
94+
ptr.as_ref().unwrap()
95+
}
96+
unsafe fn as_mut_ref<'a>(ptr: *mut Self) -> &'a mut Self {
97+
#[allow(clippy::disallowed_methods)]
98+
ptr.as_mut().unwrap()
99+
}
100+
unsafe fn free(ptr: *mut Self) {
101+
std::mem::drop(BoxFFI::from_ptr(ptr));
102+
}
103+
}
104+
105+
/// Defines a pointer manipulation API for shared heap-allocated data.
106+
///
107+
/// Implement this trait for types that require a shared ownership of data.
108+
/// The data should be allocated via [`Arc::new`], and then returned to the user as a pointer.
109+
/// The user is responsible for freeing the memory associated
110+
/// with the pointer using corresponding driver's API function.
111+
pub trait ArcFFI {
112+
fn as_ptr(self: &Arc<Self>) -> *const Self {
113+
#[allow(clippy::disallowed_methods)]
114+
Arc::as_ptr(self)
115+
}
116+
fn into_ptr(self: Arc<Self>) -> *const Self {
117+
#[allow(clippy::disallowed_methods)]
118+
Arc::into_raw(self)
119+
}
120+
unsafe fn from_ptr(ptr: *const Self) -> Arc<Self> {
121+
#[allow(clippy::disallowed_methods)]
122+
Arc::from_raw(ptr)
123+
}
124+
unsafe fn cloned_from_ptr(ptr: *const Self) -> Arc<Self> {
125+
#[allow(clippy::disallowed_methods)]
126+
Arc::increment_strong_count(ptr);
127+
#[allow(clippy::disallowed_methods)]
128+
Arc::from_raw(ptr)
129+
}
130+
unsafe fn as_maybe_ref<'a>(ptr: *const Self) -> Option<&'a Self> {
131+
#[allow(clippy::disallowed_methods)]
132+
ptr.as_ref()
133+
}
134+
unsafe fn as_ref<'a>(ptr: *const Self) -> &'a Self {
135+
#[allow(clippy::disallowed_methods)]
136+
ptr.as_ref().unwrap()
137+
}
138+
unsafe fn free(ptr: *const Self) {
139+
std::mem::drop(ArcFFI::from_ptr(ptr));
140+
}
141+
}
142+
143+
/// Defines a pointer manipulation API for data owned by some other object.
144+
///
145+
/// Implement this trait for the types that do not need to be freed (directly) by the user.
146+
/// The lifetime of the data is bound to some other object owning it.
147+
///
148+
/// For example: lifetime of CassRow is bound by the lifetime of CassResult.
149+
/// There is no API function that frees the CassRow. It should be automatically
150+
/// freed when user calls cass_result_free.
151+
pub trait RefFFI {
152+
fn as_ptr(&self) -> *const Self {
153+
self as *const Self
154+
}
155+
unsafe fn as_ref<'a>(ptr: *const Self) -> &'a Self {
156+
#[allow(clippy::disallowed_methods)]
157+
ptr.as_ref().unwrap()
158+
}
159+
}

‎scylla-rust-wrapper/src/batch.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::argconv::{free_boxed, ptr_to_ref, ptr_to_ref_mut};
1+
use crate::argconv::{ArcFFI, BoxFFI};
22
use crate::cass_error::CassError;
33
use crate::cass_types::CassConsistency;
44
use crate::cass_types::{make_batch_type, CassBatchType};
@@ -19,6 +19,8 @@ pub struct CassBatch {
1919
pub(crate) exec_profile: Option<PerStatementExecProfile>,
2020
}
2121

22+
impl BoxFFI for CassBatch {}
23+
2224
#[derive(Clone)]
2325
pub struct CassBatchState {
2426
pub batch: Batch,
@@ -28,7 +30,7 @@ pub struct CassBatchState {
2830
#[no_mangle]
2931
pub unsafe extern "C" fn cass_batch_new(type_: CassBatchType) -> *mut CassBatch {
3032
if let Some(batch_type) = make_batch_type(type_) {
31-
Box::into_raw(Box::new(CassBatch {
33+
BoxFFI::into_ptr(Box::new(CassBatch {
3234
state: Arc::new(CassBatchState {
3335
batch: Batch::new(batch_type),
3436
bound_values: Vec::new(),
@@ -43,15 +45,15 @@ pub unsafe extern "C" fn cass_batch_new(type_: CassBatchType) -> *mut CassBatch
4345

4446
#[no_mangle]
4547
pub unsafe extern "C" fn cass_batch_free(batch: *mut CassBatch) {
46-
free_boxed(batch)
48+
BoxFFI::free(batch);
4749
}
4850

4951
#[no_mangle]
5052
pub unsafe extern "C" fn cass_batch_set_consistency(
5153
batch: *mut CassBatch,
5254
consistency: CassConsistency,
5355
) -> CassError {
54-
let batch = ptr_to_ref_mut(batch);
56+
let batch = BoxFFI::as_mut_ref(batch);
5557
let consistency = match consistency.try_into().ok() {
5658
Some(c) => c,
5759
None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
@@ -68,7 +70,7 @@ pub unsafe extern "C" fn cass_batch_set_serial_consistency(
6870
batch: *mut CassBatch,
6971
serial_consistency: CassConsistency,
7072
) -> CassError {
71-
let batch = ptr_to_ref_mut(batch);
73+
let batch = BoxFFI::as_mut_ref(batch);
7274
let serial_consistency = match serial_consistency.try_into().ok() {
7375
Some(c) => c,
7476
None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
@@ -85,10 +87,10 @@ pub unsafe extern "C" fn cass_batch_set_retry_policy(
8587
batch: *mut CassBatch,
8688
retry_policy: *const CassRetryPolicy,
8789
) -> CassError {
88-
let batch = ptr_to_ref_mut(batch);
90+
let batch = BoxFFI::as_mut_ref(batch);
8991

9092
let maybe_arced_retry_policy: Option<Arc<dyn scylla::retry_policy::RetryPolicy>> =
91-
retry_policy.as_ref().map(|policy| match policy {
93+
ArcFFI::as_maybe_ref(retry_policy).map(|policy| match policy {
9294
CassRetryPolicy::DefaultRetryPolicy(default) => {
9395
default.clone() as Arc<dyn scylla::retry_policy::RetryPolicy>
9496
}
@@ -108,7 +110,7 @@ pub unsafe extern "C" fn cass_batch_set_timestamp(
108110
batch: *mut CassBatch,
109111
timestamp: cass_int64_t,
110112
) -> CassError {
111-
let batch = ptr_to_ref_mut(batch);
113+
let batch = BoxFFI::as_mut_ref(batch);
112114

113115
Arc::make_mut(&mut batch.state)
114116
.batch
@@ -122,7 +124,7 @@ pub unsafe extern "C" fn cass_batch_set_request_timeout(
122124
batch: *mut CassBatch,
123125
timeout_ms: cass_uint64_t,
124126
) -> CassError {
125-
let batch = ptr_to_ref_mut(batch);
127+
let batch = BoxFFI::as_mut_ref(batch);
126128
batch.batch_request_timeout_ms = Some(timeout_ms);
127129

128130
CassError::CASS_OK
@@ -133,7 +135,7 @@ pub unsafe extern "C" fn cass_batch_set_is_idempotent(
133135
batch: *mut CassBatch,
134136
is_idempotent: cass_bool_t,
135137
) -> CassError {
136-
let batch = ptr_to_ref_mut(batch);
138+
let batch = BoxFFI::as_mut_ref(batch);
137139
Arc::make_mut(&mut batch.state)
138140
.batch
139141
.set_is_idempotent(is_idempotent != 0);
@@ -146,7 +148,7 @@ pub unsafe extern "C" fn cass_batch_set_tracing(
146148
batch: *mut CassBatch,
147149
enabled: cass_bool_t,
148150
) -> CassError {
149-
let batch = ptr_to_ref_mut(batch);
151+
let batch = BoxFFI::as_mut_ref(batch);
150152
Arc::make_mut(&mut batch.state)
151153
.batch
152154
.set_tracing(enabled != 0);
@@ -159,9 +161,9 @@ pub unsafe extern "C" fn cass_batch_add_statement(
159161
batch: *mut CassBatch,
160162
statement: *const CassStatement,
161163
) -> CassError {
162-
let batch = ptr_to_ref_mut(batch);
164+
let batch = BoxFFI::as_mut_ref(batch);
163165
let state = Arc::make_mut(&mut batch.state);
164-
let statement = ptr_to_ref(statement);
166+
let statement = BoxFFI::as_ref(statement);
165167

166168
match &statement.statement {
167169
Statement::Simple(q) => state.batch.append_statement(q.query.clone()),

‎scylla-rust-wrapper/src/binding.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ macro_rules! make_index_binder {
6161
#[allow(unused_imports)]
6262
use crate::value::CassCqlValue::*;
6363
match ($e)($($arg), *) {
64-
Ok(v) => $consume_v(ptr_to_ref_mut(this), index as usize, v),
64+
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), index as usize, v),
6565
Err(e) => e,
6666
}
6767
}
@@ -82,7 +82,7 @@ macro_rules! make_name_binder {
8282
use crate::value::CassCqlValue::*;
8383
let name = ptr_to_cstr(name).unwrap();
8484
match ($e)($($arg), *) {
85-
Ok(v) => $consume_v(ptr_to_ref_mut(this), name, v),
85+
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), name, v),
8686
Err(e) => e,
8787
}
8888
}
@@ -104,7 +104,7 @@ macro_rules! make_name_n_binder {
104104
use crate::value::CassCqlValue::*;
105105
let name = ptr_to_cstr_n(name, name_length).unwrap();
106106
match ($e)($($arg), *) {
107-
Ok(v) => $consume_v(ptr_to_ref_mut(this), name, v),
107+
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), name, v),
108108
Err(e) => e,
109109
}
110110
}
@@ -123,7 +123,7 @@ macro_rules! make_appender {
123123
#[allow(unused_imports)]
124124
use crate::value::CassCqlValue::*;
125125
match ($e)($($arg), *) {
126-
Ok(v) => $consume_v(ptr_to_ref_mut(this), v),
126+
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), v),
127127
Err(e) => e,
128128
}
129129
}
@@ -303,7 +303,7 @@ macro_rules! invoke_binder_maker_macro_with_type {
303303
$consume_v,
304304
$fn,
305305
|p: *const crate::collection::CassCollection| {
306-
match std::convert::TryInto::try_into(ptr_to_ref(p)) {
306+
match std::convert::TryInto::try_into(BoxFFI::as_ref(p)) {
307307
Ok(v) => Ok(Some(v)),
308308
Err(_) => Err(CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE),
309309
}
@@ -317,7 +317,7 @@ macro_rules! invoke_binder_maker_macro_with_type {
317317
$consume_v,
318318
$fn,
319319
|p: *const crate::tuple::CassTuple| {
320-
Ok(Some(ptr_to_ref(p).into()))
320+
Ok(Some(BoxFFI::as_ref(p).into()))
321321
},
322322
[p @ *const crate::tuple::CassTuple]
323323
);
@@ -327,7 +327,7 @@ macro_rules! invoke_binder_maker_macro_with_type {
327327
$this,
328328
$consume_v,
329329
$fn,
330-
|p: *const crate::user_type::CassUserType| Ok(Some(ptr_to_ref(p).into())),
330+
|p: *const crate::user_type::CassUserType| Ok(Some(BoxFFI::as_ref(p).into())),
331331
[p @ *const crate::user_type::CassUserType]
332332
);
333333
};

0 commit comments

Comments
 (0)