From e3e30979470bf9a24697fb049f3a4ee7f023f721 Mon Sep 17 00:00:00 2001 From: Jazz Turner-Baggs <473256+jazzz@users.noreply.github.com> Date: Mon, 9 Feb 2026 06:37:47 -0800 Subject: [PATCH] Safer FFI Migration (#47) * Use safer_ffi for all functions * Clean up FFI docs and imports * Update nim bindings * Binding Memory Management * Update tests --- conversations/src/api.rs | 176 ++++++++++++++++++++++++---------- conversations/src/lib.rs | 69 +++---------- nim-bindings/src/bindings.nim | 67 +++++++++---- nim-bindings/src/libchat.nim | 62 +++++------- 4 files changed, 212 insertions(+), 162 deletions(-) diff --git a/conversations/src/api.rs b/conversations/src/api.rs index d8951bd..14fba3a 100644 --- a/conversations/src/api.rs +++ b/conversations/src/api.rs @@ -1,6 +1,25 @@ -use safer_ffi::prelude::*; +// This is the FFI Interface to enable libchat to be used from other languages such as Nim and C. +// This interface makes heavy use of safer_ffi in order to safely move bytes across the FFI. +// +// The following table explains the safer_ffi types in use, and under what circumstances. +// +// - c_slice::Ref<'_, u8> : Borrowed, read-only byte slice for input parameters +// - c_slice::Mut<'_, u8> : Borrowed, mutable byte slice for in/out parameters +// - repr_c::Vec : Owned vector, used for return values (transfers ownership to caller) +// - repr_c::String : Owned string, used for return values (transfers ownership to caller) -// Must only contain negative values, values cannot be changed once set. +use safer_ffi::{ + String, derive_ReprC, ffi_export, + prelude::{c_slice, repr_c}, +}; + +use crate::{ + context::{Context, Introduction}, + errors::ChatError, + types::ContentData, +}; + +// Must only contain negative values or 0, values cannot be changed once set. #[repr(i32)] pub enum ErrorCode { None = 0, @@ -12,7 +31,13 @@ pub enum ErrorCode { UnknownError = -6, } -use crate::context::{Context, Introduction}; +pub fn is_ok(error: i32) -> bool { + error == ErrorCode::None as i32 +} + +// ------------------------------------------ +// Exported Functions +// ------------------------------------------ /// Opaque wrapper for Context #[derive_ReprC] @@ -45,18 +70,17 @@ pub fn destroy_context(ctx: repr_c::Box) { /// Returns the number of bytes written to bundle_out /// Check error_code field: 0 means success, negative values indicate errors (see ErrorCode). #[ffi_export] -pub fn create_intro_bundle(ctx: &mut ContextHandle, mut bundle_out: c_slice::Mut<'_, u8>) -> i32 { - let Ok(bundle) = ctx.0.create_intro_bundle() else { - return ErrorCode::UnknownError as i32; - }; - - // Check buffer is large enough - if bundle_out.len() < bundle.len() { - return ErrorCode::BufferExceeded as i32; +pub fn create_intro_bundle(ctx: &mut ContextHandle) -> CreateIntroResult { + match ctx.0.create_intro_bundle() { + Ok(v) => CreateIntroResult { + error_code: ErrorCode::None as i32, + intro_bytes: v.into(), + }, + Err(_e) => CreateIntroResult { + error_code: ErrorCode::UnknownError as i32, + intro_bytes: repr_c::Vec::EMPTY, + }, } - - bundle_out[..bundle.len()].copy_from_slice(&bundle); - bundle.len() as i32 } /// Creates a new private conversation @@ -108,11 +132,11 @@ pub fn send_content( ctx: &mut ContextHandle, convo_id: repr_c::String, content: c_slice::Ref<'_, u8>, -) -> PayloadResult { +) -> SendContentResult { let payloads = match ctx.0.send_content(&convo_id, &content) { Ok(p) => p, Err(_) => { - return PayloadResult { + return SendContentResult { error_code: ErrorCode::UnknownError as i32, payloads: safer_ffi::Vec::EMPTY, }; @@ -127,51 +151,47 @@ pub fn send_content( }) .collect(); - PayloadResult { + SendContentResult { error_code: 0, payloads: ffi_payloads.into(), } } -/// Handles an incoming payload and writes content to caller-provided buffers +/// Handles an incoming payload /// /// # Returns -/// Returns the number of bytes written to data_out on success (>= 0). -/// Returns negative error code on failure (see ErrorCode). -/// conversation_id_out_len is set to the number of bytes written to conversation_id_out. +/// Returns HandlePayloadResult +/// This call does not always generate content. If data is zero bytes long then there +/// is no data, and the converation_id should be ignored. #[ffi_export] pub fn handle_payload( ctx: &mut ContextHandle, payload: c_slice::Ref<'_, u8>, - mut conversation_id_out: c_slice::Mut<'_, u8>, - conversation_id_out_len: Out<'_, u32>, - mut content_out: c_slice::Mut<'_, u8>, -) -> i32 { +) -> HandlePayloadResult { match ctx.0.handle_payload(&payload) { - Ok(Some(content)) => { - let convo_id_bytes = content.conversation_id.as_bytes(); - - if conversation_id_out.len() < convo_id_bytes.len() { - return ErrorCode::BufferExceeded as i32; - } - - if content_out.len() < content.data.len() { - return ErrorCode::BufferExceeded as i32; - } - - conversation_id_out[..convo_id_bytes.len()].copy_from_slice(convo_id_bytes); - conversation_id_out_len.write(convo_id_bytes.len() as u32); - content_out[..content.data.len()].copy_from_slice(&content.data); - - content.data.len() as i32 - } - _ => 0, + Ok(o) => o.into(), + Err(e) => e.into(), } } -// ============================================================================ -// safer_ffi implementation -// =============================================================================================================================== +// ------------------------------------------ +// Return Type Definitions +// ------------------------------------------ + +/// Result structure for create_intro_bundle +/// error_code is 0 on success, negative on error (see ErrorCode) +#[derive_ReprC] +#[repr(C)] +pub struct CreateIntroResult { + pub error_code: i32, + pub intro_bytes: repr_c::Vec, +} + +/// Free the result from create_intro_bundle +#[ffi_export] +pub fn destroy_intro_result(result: CreateIntroResult) { + drop(result); +} /// Payload structure for FFI #[derive(Debug)] @@ -182,22 +202,74 @@ pub struct Payload { pub data: repr_c::Vec, } -/// Result structure for create_intro_bundle_safe +/// Result structure for send_content /// error_code is 0 on success, negative on error (see ErrorCode) #[derive_ReprC] #[repr(C)] -pub struct PayloadResult { +pub struct SendContentResult { pub error_code: i32, pub payloads: repr_c::Vec, } -/// Free the result from create_intro_bundle_safe +/// Free the result from send_content #[ffi_export] -pub fn destroy_payload_result(result: PayloadResult) { +pub fn destroy_send_content_result(result: SendContentResult) { drop(result); } -/// Result structure for create_new_private_convo_safe +/// Result structure for handle_payload +/// error_code is 0 on success, negative on error (see ErrorCode) +#[derive(Debug)] +#[derive_ReprC] +#[repr(C)] +pub struct HandlePayloadResult { + pub error_code: i32, + pub convo_id: repr_c::String, + pub content: repr_c::Vec, +} + +/// Free the result from handle_payload +#[ffi_export] +pub fn destroy_handle_payload_result(result: HandlePayloadResult) { + drop(result); +} + +impl From for HandlePayloadResult { + fn from(value: ContentData) -> Self { + HandlePayloadResult { + error_code: ErrorCode::None as i32, + convo_id: value.conversation_id.into(), + content: value.data.into(), + } + } +} + +impl From> for HandlePayloadResult { + fn from(value: Option) -> Self { + if let Some(content) = value { + content.into() + } else { + HandlePayloadResult { + error_code: ErrorCode::None as i32, + convo_id: repr_c::String::EMPTY, + content: repr_c::Vec::EMPTY, + } + } + } +} + +impl From for HandlePayloadResult { + fn from(_value: ChatError) -> Self { + HandlePayloadResult { + // TODO: (P2) Translate ChatError into ErrorCode + error_code: ErrorCode::UnknownError as i32, + convo_id: String::EMPTY, + content: repr_c::Vec::EMPTY, + } + } +} + +/// Result structure for create_new_private_convo /// error_code is 0 on success, negative on error (see ErrorCode) #[derive_ReprC] #[repr(C)] @@ -207,7 +279,7 @@ pub struct NewConvoResult { pub payloads: repr_c::Vec, } -/// Free the result from create_new_private_convo_safe +/// Free the result from create_new_private_convo #[ffi_export] pub fn destroy_convo_result(result: NewConvoResult) { drop(result); diff --git a/conversations/src/lib.rs b/conversations/src/lib.rs index 91aab32..2da1f82 100644 --- a/conversations/src/lib.rs +++ b/conversations/src/lib.rs @@ -20,72 +20,31 @@ mod tests { #[test] fn test_ffi() {} - #[test] - fn test_invite_convo() { - let mut ctx = create_context(); - let mut bundle = vec![0u8; 200]; - - let bundle_len = create_intro_bundle(&mut ctx, (&mut bundle[..]).into()); - unsafe { - bundle.set_len(bundle_len as usize); - } - - assert!(bundle_len > 0, "bundle failed: {}", bundle_len); - let content = b"Hello"; - let result = create_new_private_convo(&mut ctx, bundle[..].into(), content[..].into()); - - assert!(result.error_code == 0, "Error: {}", result.error_code); - - destroy_context(ctx); - } - #[test] fn test_message_roundtrip() { let mut saro = create_context(); let mut raya = create_context(); - let mut raya_bundle = vec![0u8; 200]; - let bundle_len = create_intro_bundle(&mut raya, (&mut raya_bundle[..]).into()); - unsafe { - raya_bundle.set_len(bundle_len as usize); - } + // Raya Creates Bundle and Sends to Saro + let intro_result = create_intro_bundle(&mut raya); + assert!(is_ok(intro_result.error_code)); - assert!(bundle_len > 0, "bundle failed: {}", bundle_len); - let content = String::from_str("Hello").unwrap(); - let result = create_new_private_convo( - &mut saro, - raya_bundle.as_slice().into(), - content.as_bytes().into(), - ); + let raya_bundle = intro_result.intro_bytes.as_ref(); - assert!(result.error_code == 0, "Error: {}", result.error_code); + // Saro creates a new conversation with Raya + let content: &[u8] = "hello".as_bytes(); - // Handle payloads on raya's side - let mut conversation_id_out = vec![0u8; 256]; - let mut conversation_id_out_len: u32 = 0; - let mut content_out = vec![0u8; 256]; + let convo_result = create_new_private_convo(&mut saro, raya_bundle, content.into()); + assert!(is_ok(convo_result.error_code)); - for p in result.payloads.iter() { - let bytes_written = handle_payload( - &mut raya, - p.data[..].into(), - (&mut conversation_id_out[..]).into(), - (&mut conversation_id_out_len).into(), - (&mut content_out[..]).into(), - ); + // Raya recieves initial message + let payload = convo_result.payloads.first().unwrap(); - unsafe { - content_out.set_len(bytes_written as usize); - } + let handle_result = handle_payload(&mut raya, payload.data.as_ref()); + assert!(is_ok(handle_result.error_code)); - assert!( - bytes_written >= 0, - "handle_payload failed: {}", - bytes_written - ); - - //TODO: Verify output match - } + // Check that the Content sent was the content received + assert!(handle_result.content.as_ref().as_slice() == content); destroy_context(saro); destroy_context(raya); diff --git a/nim-bindings/src/bindings.nim b/nim-bindings/src/bindings.nim index c453e0a..9b3d878 100644 --- a/nim-bindings/src/bindings.nim +++ b/nim-bindings/src/bindings.nim @@ -68,10 +68,23 @@ type ## Result structure for create_intro_bundle ## error_code is 0 on success, negative on error (see ErrorCode) - PayloadResult* = object + CreateIntroResult* = object + error_code*: int32 + intro_bytes*: VecUint8 + + ## Result structure for send_content + ## error_code is 0 on success, negative on error (see ErrorCode) + SendContentResult* = object error_code*: int32 payloads*: VecPayload + ## Result structure for handle_payload + ## error_code is 0 on success, negative on error (see ErrorCode) + HandlePayloadResult* = object + error_code*: int32 + convo_id*: ReprCString + content*: VecUint8 + ## Result from create_new_private_convo ## error_code is 0 on success, negative on error (see ErrorCode) NewConvoResult* = object @@ -91,11 +104,11 @@ proc create_context*(): ContextHandle {.importc, dynlib: CONVERSATIONS_LIB.} proc destroy_context*(ctx: ContextHandle) {.importc, dynlib: CONVERSATIONS_LIB.} ## Creates an intro bundle for sharing with other users -## Returns: Number of bytes written to bundle_out, or negative error code +## Returns: CreateIntroResult struct - check error_code field (0 = success, negative = error) +## The result must be freed with destroy_intro_result() proc create_intro_bundle*( ctx: ContextHandle, - bundle_out: SliceUint8, -): int32 {.importc, dynlib: CONVERSATIONS_LIB.} +): CreateIntroResult {.importc, dynlib: CONVERSATIONS_LIB.} ## Creates a new private conversation ## Returns: NewConvoResult struct - check error_code field (0 = success, negative = error) @@ -107,30 +120,35 @@ proc create_new_private_convo*( ): NewConvoResult {.importc, dynlib: CONVERSATIONS_LIB.} ## Sends content to an existing conversation -## Returns: PayloadResult struct - check error_code field (0 = success, negative = error) -## The result must be freed with destroy_payload_result() +## Returns: SendContentResult struct - check error_code field (0 = success, negative = error) +## The result must be freed with destroy_send_content_result() proc send_content*( ctx: ContextHandle, - convo_id: SliceUint8, + convo_id: ReprCString, content: SliceUint8, -): PayloadResult {.importc, dynlib: CONVERSATIONS_LIB.} +): SendContentResult {.importc, dynlib: CONVERSATIONS_LIB.} -## Handles an incoming payload and writes content to caller-provided buffers -## Returns: Number of bytes written to content_out on success (>= 0), negative error code on failure -## conversation_id_out_len is set to the number of bytes written to conversation_id_out +## Handles an incoming payload +## Returns: HandlePayloadResult struct - check error_code field (0 = success, negative = error) +## This call does not always generate content. If content is zero bytes long then there +## is no data, and the convo_id should be ignored. +## The result must be freed with destroy_handle_payload_result() proc handle_payload*( ctx: ContextHandle, payload: SliceUint8, - conversation_id_out: SliceUint8, - conversation_id_out_len: ptr uint32, - content_out: SliceUint8, -): int32 {.importc, dynlib: CONVERSATIONS_LIB.} +): HandlePayloadResult {.importc, dynlib: CONVERSATIONS_LIB.} + +## Free the result from create_intro_bundle +proc destroy_intro_result*(result: CreateIntroResult) {.importc, dynlib: CONVERSATIONS_LIB.} ## Free the result from create_new_private_convo proc destroy_convo_result*(result: NewConvoResult) {.importc, dynlib: CONVERSATIONS_LIB.} -## Free the PayloadResult -proc destroy_payload_result*(result: PayloadResult) {.importc, dynlib: CONVERSATIONS_LIB.} +## Free the result from send_content +proc destroy_send_content_result*(result: SendContentResult) {.importc, dynlib: CONVERSATIONS_LIB.} + +## Free the result from handle_payload +proc destroy_handle_payload_result*(result: HandlePayloadResult) {.importc, dynlib: CONVERSATIONS_LIB.} # ============================================================================ # Helper functions @@ -157,6 +175,16 @@ proc `$`*(s: ReprCString): string = result = newString(s.len) copyMem(addr result[0], s.ptr, s.len) +## Create a ReprCString from a Nim string for passing to FFI functions. +## WARNING: The returned ReprCString borrows from the input string. +## The input string must remain valid for the duration of the FFI call. +## cap is set to 0 to prevent Rust from attempting to deallocate Nim memory. +proc toReprCString*(s: string): ReprCString = + if s.len == 0: + ReprCString(`ptr`: nil, len: 0, cap: 0) + else: + ReprCString(`ptr`: cast[ptr char](unsafeAddr s[0]), len: csize_t(s.len), cap: 0) + ## Convert a VecUint8 to a seq[byte] proc toSeq*(v: VecUint8): seq[byte] = if v.ptr == nil or v.len == 0: @@ -173,6 +201,11 @@ proc `[]`*(v: VecPayload, i: int): Payload = proc len*(v: VecPayload): int = int(v.len) +## Iterator for VecPayload +iterator items*(v: VecPayload): Payload = + for i in 0 ..< v.len: + yield v[int(i)] + ## Convert a string to seq[byte] proc toBytes*(s: string): seq[byte] = if s.len == 0: diff --git a/nim-bindings/src/libchat.nim b/nim-bindings/src/libchat.nim index ae9748c..f02d504 100644 --- a/nim-bindings/src/libchat.nim +++ b/nim-bindings/src/libchat.nim @@ -1,4 +1,5 @@ import std/options +import std/sequtils import results import bindings @@ -31,22 +32,21 @@ proc getBuffer*(ctx: LibChat): seq[byte] = newSeq[byte](ctx.buffer_size) ## Generate a Introduction Bundle -proc createIntroductionBundle*(ctx: LibChat): Result[string, string] = +proc createIntroductionBundle*(ctx: LibChat): Result[seq[byte], string] = if ctx.handle == nil: return err("Context handle is nil") - var buffer = ctx.getBuffer() - var slice = buffer.toSlice() - let len = create_intro_bundle(ctx.handle, slice) + let res = create_intro_bundle(ctx.handle) - if len < 0: - return err("Failed to create intro bundle: " & $len) + if res.error_code != ErrNone: + result = err("Failed to create private convo: " & $res.error_code) + destroy_intro_result(res) + return - buffer.setLen(len) - return ok(cast[string](buffer)) + return ok(res.intro_bytes.toSeq()) ## Create a Private Convo -proc createNewPrivateConvo*(ctx: LibChat, bundle: string, content: seq[byte]): Result[(string, seq[PayloadResult]), string] = +proc createNewPrivateConvo*(ctx: LibChat, bundle: seq[byte], content: seq[byte]): Result[(string, seq[PayloadResult]), string] = if ctx.handle == nil: return err("Context handle is nil") @@ -92,25 +92,22 @@ proc sendContent*(ctx: LibChat, convoId: string, content: seq[byte]): Result[seq let res = bindings.send_content( ctx.handle, - convoId.toSlice(), + convoId.toReprCString, content.toSlice() ) if res.error_code != 0: result = err("Failed to send content: " & $res.error_code) - destroy_payload_result(res) + destroy_send_content_result(res) return - # Convert payloads to Nim types - var payloads = newSeq[PayloadResult](res.payloads.len) - for i in 0 ..< res.payloads.len: - let p = res.payloads[int(i)] - payloads[int(i)] = PayloadResult( - address: $p.address, - data: p.data.toSeq() - ) - destroy_payload_result(res) + let payloads = res.payloads.toSeq().mapIt(PayloadResult( + address: $it.address, + data: it.data.toSeq() + )) + + destroy_send_content_result(res) return ok(payloads) type @@ -130,30 +127,19 @@ proc handlePayload*(ctx: LibChat, payload: seq[byte]): Result[Option[ContentResu var contentBuf = newSeq[byte](ctx.buffer_size) var conversationIdLen: uint32 = 0 - let bytesWritten = bindings.handle_payload( + let res = bindings.handle_payload( ctx.handle, payload.toSlice(), - conversationIdBuf.toSlice(), - addr conversationIdLen, - contentBuf.toSlice() ) - if bytesWritten < 0: - return err("Failed to handle payload: " & $bytesWritten) + if res.error_code != ErrNone: + return err("Failed to handle payload: " & $res.error_code) - if bytesWritten == 0: + let content = res.content.toSeq() + if content.len == 0: return ok(none(ContentResult)) - conversationIdBuf.setLen(conversationIdLen) - contentBuf.setLen(bytesWritten) - return ok(some(ContentResult( - conversationId: cast[string](conversationIdBuf), - data: contentBuf + conversationId: $res.convo_id, + data: content ))) - - -proc `=destroy`(x: var LibChat) = - # Automatically free handle when the destructor is called - if x.handle != nil: - x.destroy()