type safety: ffi pointer manipulation API

scylla-rust-wrapper/clippy.toml

@@ -0,0 +1,21 @@
+disallowed-methods = [
+    "std::boxed::Box::from_raw",
+    "std::boxed::Box::from_raw_in",
+    "std::boxed::Box::into_raw",
+    "std::boxed::Box::into_raw_with_allocator",
+
+    "std::sync::Arc::as_ptr",
+    "std::sync::Arc::decrement_strong_count",
+    "std::sync::Arc::from_raw",
+    "std::sync::Arc::increment_strong_count",
+    "std::sync::Arc::into_raw",
+
+    "std::rc::Rc::as_ptr",
+    "std::rc::Rc::decrement_strong_count",
+    "std::rc::Rc::from_raw",
+    "std::rc::Rc::increment_strong_count",
+    "std::rc::Rc::into_raw",
+
+    "const_ptr::as_ref",
+    "mut_ptr::as_mut"
+]

scylla-rust-wrapper/src/argconv.rs

@@ -4,33 +4,6 @@ use std::ffi::CStr;
 use std::os::raw::c_char;
 use std::sync::Arc;
 
-pub unsafe fn ptr_to_ref<T>(ptr: *const T) -> &'static T {
-    ptr.as_ref().unwrap()
-}
-
-pub unsafe fn ptr_to_ref_mut<T>(ptr: *mut T) -> &'static mut T {
-    ptr.as_mut().unwrap()
-}
-
-pub unsafe fn free_boxed<T>(ptr: *mut T) {
-    if !ptr.is_null() {
-        // This takes the ownership of the boxed value and drops it
-        let _ = Box::from_raw(ptr);
-    }
-}
-
-pub unsafe fn clone_arced<T>(ptr: *const T) -> Arc<T> {
-    Arc::increment_strong_count(ptr);
-    Arc::from_raw(ptr)
-}
-
-pub unsafe fn free_arced<T>(ptr: *const T) {
-    if !ptr.is_null() {
-        // This decrements the arc's internal counter and potentially drops it
-        Arc::from_raw(ptr);
-    }
-}
-
 pub unsafe fn ptr_to_cstr(ptr: *const c_char) -> Option<&'static str> {
     CStr::from_ptr(ptr).to_str().ok()
 }
@@ -97,3 +70,90 @@ macro_rules! make_c_str {
 
 #[cfg(test)]
 pub(crate) use make_c_str;
+
+/// Defines a pointer manipulation API for non-shared heap-allocated data.
+///
+/// Implement this trait for types that are allocated by the driver via [`Box::new`],
+/// and then returned to the user as a pointer. The user is responsible for freeing
+/// the memory associated with the pointer using corresponding driver's API function.
+pub trait BoxFFI {
+    fn into_ptr(self: Box<Self>) -> *mut Self {
+        #[allow(clippy::disallowed_methods)]
+        Box::into_raw(self)
+    }
+    unsafe fn from_ptr(ptr: *mut Self) -> Box<Self> {
+        #[allow(clippy::disallowed_methods)]
+        Box::from_raw(ptr)
+    }
+    unsafe fn as_maybe_ref<'a>(ptr: *const Self) -> Option<&'a Self> {
+        #[allow(clippy::disallowed_methods)]
+        ptr.as_ref()
+    }
+    unsafe fn as_ref<'a>(ptr: *const Self) -> &'a Self {
+        #[allow(clippy::disallowed_methods)]
+        ptr.as_ref().unwrap()
+    }
+    unsafe fn as_mut_ref<'a>(ptr: *mut Self) -> &'a mut Self {
+        #[allow(clippy::disallowed_methods)]
+        ptr.as_mut().unwrap()
+    }
+    unsafe fn free(ptr: *mut Self) {
+        std::mem::drop(BoxFFI::from_ptr(ptr));
+    }
+}
+
+/// Defines a pointer manipulation API for shared heap-allocated data.
+///
+/// Implement this trait for types that require a shared ownership of data.
+/// The data should be allocated via [`Arc::new`], and then returned to the user as a pointer.
+/// The user is responsible for freeing the memory associated
+/// with the pointer using corresponding driver's API function.
+pub trait ArcFFI {
+    fn as_ptr(self: &Arc<Self>) -> *const Self {
+        #[allow(clippy::disallowed_methods)]
+        Arc::as_ptr(self)
+    }
+    fn into_ptr(self: Arc<Self>) -> *const Self {
+        #[allow(clippy::disallowed_methods)]
+        Arc::into_raw(self)
+    }
+    unsafe fn from_ptr(ptr: *const Self) -> Arc<Self> {
+        #[allow(clippy::disallowed_methods)]
+        Arc::from_raw(ptr)
+    }
+    unsafe fn cloned_from_ptr(ptr: *const Self) -> Arc<Self> {
+        #[allow(clippy::disallowed_methods)]
+        Arc::increment_strong_count(ptr);
+        #[allow(clippy::disallowed_methods)]
+        Arc::from_raw(ptr)
+    }
+    unsafe fn as_maybe_ref<'a>(ptr: *const Self) -> Option<&'a Self> {
+        #[allow(clippy::disallowed_methods)]
+        ptr.as_ref()
+    }
+    unsafe fn as_ref<'a>(ptr: *const Self) -> &'a Self {
+        #[allow(clippy::disallowed_methods)]
+        ptr.as_ref().unwrap()
+    }
+    unsafe fn free(ptr: *const Self) {
+        std::mem::drop(ArcFFI::from_ptr(ptr));
+    }
+}
+
+/// Defines a pointer manipulation API for data owned by some other object.
+///
+/// Implement this trait for the types that do not need to be freed (directly) by the user.
+/// The lifetime of the data is bound to some other object owning it.
+///
+/// For example: lifetime of CassRow is bound by the lifetime of CassResult.
+/// There is no API function that frees the CassRow. It should be automatically
+/// freed when user calls cass_result_free.
+pub trait RefFFI {
+    fn as_ptr(&self) -> *const Self {
+        self as *const Self
+    }
+    unsafe fn as_ref<'a>(ptr: *const Self) -> &'a Self {
+        #[allow(clippy::disallowed_methods)]
+        ptr.as_ref().unwrap()
+    }
+}

scylla-rust-wrapper/src/batch.rs

@@ -1,4 +1,4 @@
-use crate::argconv::{free_boxed, ptr_to_ref, ptr_to_ref_mut};
+use crate::argconv::{ArcFFI, BoxFFI};
 use crate::cass_error::CassError;
 use crate::cass_types::CassConsistency;
 use crate::cass_types::{make_batch_type, CassBatchType};
@@ -19,6 +19,8 @@ pub struct CassBatch {
     pub(crate) exec_profile: Option<PerStatementExecProfile>,
 }
 
+impl BoxFFI for CassBatch {}
+
 #[derive(Clone)]
 pub struct CassBatchState {
     pub batch: Batch,
@@ -28,7 +30,7 @@ pub struct CassBatchState {
 #[no_mangle]
 pub unsafe extern "C" fn cass_batch_new(type_: CassBatchType) -> *mut CassBatch {
     if let Some(batch_type) = make_batch_type(type_) {
-        Box::into_raw(Box::new(CassBatch {
+        BoxFFI::into_ptr(Box::new(CassBatch {
             state: Arc::new(CassBatchState {
                 batch: Batch::new(batch_type),
                 bound_values: Vec::new(),
@@ -43,15 +45,15 @@ pub unsafe extern "C" fn cass_batch_new(type_: CassBatchType) -> *mut CassBatch
 
 #[no_mangle]
 pub unsafe extern "C" fn cass_batch_free(batch: *mut CassBatch) {
-    free_boxed(batch)
+    BoxFFI::free(batch);
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn cass_batch_set_consistency(
     batch: *mut CassBatch,
     consistency: CassConsistency,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
     let consistency = match consistency.try_into().ok() {
         Some(c) => c,
         None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
@@ -68,7 +70,7 @@ pub unsafe extern "C" fn cass_batch_set_serial_consistency(
     batch: *mut CassBatch,
     serial_consistency: CassConsistency,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
     let serial_consistency = match serial_consistency.try_into().ok() {
         Some(c) => c,
         None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
@@ -85,10 +87,10 @@ pub unsafe extern "C" fn cass_batch_set_retry_policy(
     batch: *mut CassBatch,
     retry_policy: *const CassRetryPolicy,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
 
     let maybe_arced_retry_policy: Option<Arc<dyn scylla::retry_policy::RetryPolicy>> =
-        retry_policy.as_ref().map(|policy| match policy {
+        ArcFFI::as_maybe_ref(retry_policy).map(|policy| match policy {
             CassRetryPolicy::DefaultRetryPolicy(default) => {
                 default.clone() as Arc<dyn scylla::retry_policy::RetryPolicy>
             }
@@ -108,7 +110,7 @@ pub unsafe extern "C" fn cass_batch_set_timestamp(
     batch: *mut CassBatch,
     timestamp: cass_int64_t,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
 
     Arc::make_mut(&mut batch.state)
         .batch
@@ -122,7 +124,7 @@ pub unsafe extern "C" fn cass_batch_set_request_timeout(
     batch: *mut CassBatch,
     timeout_ms: cass_uint64_t,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
     batch.batch_request_timeout_ms = Some(timeout_ms);
 
     CassError::CASS_OK
@@ -133,7 +135,7 @@ pub unsafe extern "C" fn cass_batch_set_is_idempotent(
     batch: *mut CassBatch,
     is_idempotent: cass_bool_t,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
     Arc::make_mut(&mut batch.state)
         .batch
         .set_is_idempotent(is_idempotent != 0);
@@ -146,7 +148,7 @@ pub unsafe extern "C" fn cass_batch_set_tracing(
     batch: *mut CassBatch,
     enabled: cass_bool_t,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
     Arc::make_mut(&mut batch.state)
         .batch
         .set_tracing(enabled != 0);
@@ -159,9 +161,9 @@ pub unsafe extern "C" fn cass_batch_add_statement(
     batch: *mut CassBatch,
     statement: *const CassStatement,
 ) -> CassError {
-    let batch = ptr_to_ref_mut(batch);
+    let batch = BoxFFI::as_mut_ref(batch);
     let state = Arc::make_mut(&mut batch.state);
-    let statement = ptr_to_ref(statement);
+    let statement = BoxFFI::as_ref(statement);
 
     match &statement.statement {
         Statement::Simple(q) => state.batch.append_statement(q.query.clone()),

scylla-rust-wrapper/src/binding.rs

@@ -61,7 +61,7 @@ macro_rules! make_index_binder {
             #[allow(unused_imports)]
             use crate::value::CassCqlValue::*;
             match ($e)($($arg), *) {
-                Ok(v) => $consume_v(ptr_to_ref_mut(this), index as usize, v),
+                Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), index as usize, v),
                 Err(e) => e,
             }
         }
@@ -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(ptr_to_ref_mut(this), name, v),
+                Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), name, v),
                 Err(e) => e,
             }
         }
@@ -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(ptr_to_ref_mut(this), name, v),
+                Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), name, v),
                 Err(e) => e,
             }
         }
@@ -123,7 +123,7 @@ macro_rules! make_appender {
             #[allow(unused_imports)]
             use crate::value::CassCqlValue::*;
             match ($e)($($arg), *) {
-                Ok(v) => $consume_v(ptr_to_ref_mut(this), v),
+                Ok(v) => $consume_v(BoxFFI::as_mut_ref(this), v),
                 Err(e) => e,
             }
         }
@@ -303,7 +303,7 @@ macro_rules! invoke_binder_maker_macro_with_type {
             $consume_v,
             $fn,
             |p: *const crate::collection::CassCollection| {
-                match std::convert::TryInto::try_into(ptr_to_ref(p)) {
+                match std::convert::TryInto::try_into(BoxFFI::as_ref(p)) {
                     Ok(v) => Ok(Some(v)),
                     Err(_) => Err(CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE),
                 }
@@ -317,7 +317,7 @@ macro_rules! invoke_binder_maker_macro_with_type {
             $consume_v,
             $fn,
             |p: *const crate::tuple::CassTuple| {
-                Ok(Some(ptr_to_ref(p).into()))
+                Ok(Some(BoxFFI::as_ref(p).into()))
             },
             [p @ *const crate::tuple::CassTuple]
         );
@@ -327,7 +327,7 @@ macro_rules! invoke_binder_maker_macro_with_type {
             $this,
             $consume_v,
             $fn,
-            |p: *const crate::user_type::CassUserType| Ok(Some(ptr_to_ref(p).into())),
+            |p: *const crate::user_type::CassUserType| Ok(Some(BoxFFI::as_ref(p).into())),
             [p @ *const crate::user_type::CassUserType]
         );
     };