From dd2eddbe9280870485974edd611e224ae585b76a Mon Sep 17 00:00:00 2001 From: Brian Pearce Date: Wed, 4 Oct 2023 08:58:49 +0200 Subject: [PATCH] fix(chatffi): return and read from ptrs (#5827) Description --- A unit struct was being returned within a c_rep struct. This caused the unit struct to have no body in the header file and caused build errors in ios. This unit struct should have been a pointer with read functions and not returned as a rust unit struct. This PR moves it over to a pointer and corrects it's readers. Motivation and Context --- Don't produce broken builds. A lot of types _around_ this fix will be removed soon after this is merged but it this corrects a broken build and it would be nicer to work on top of this PR, then ignore it and fix the grand scheme. How Has This Been Tested? --- Built and provided to the mobile team. Confirmed no build errors. What process can a PR reviewer use to test or verify this change? --- Skim the code, but honestly don't look too close it's about to churn. Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify --- base_layer/chat_ffi/chat.h | 78 ++++++++++++- base_layer/chat_ffi/src/message_metadata.rs | 110 ++++++++++++++++-- base_layer/chat_ffi/src/types/byte_vector.rs | 1 + .../chat_ffi/src/types/chat_ffi_message.rs | 30 ++++- 4 files changed, 202 insertions(+), 17 deletions(-) diff --git a/base_layer/chat_ffi/chat.h b/base_layer/chat_ffi/chat.h index abb0eef7e8..8b325144b5 100644 --- a/base_layer/chat_ffi/chat.h +++ b/base_layer/chat_ffi/chat.h @@ -39,7 +39,7 @@ struct ChatFFIMessage { const char *from_address; uint64_t stored_at; const char *message_id; - struct ChatMessageMetadataVector metadata; + struct ChatMessageMetadataVector *metadata; int metadata_len; }; @@ -49,6 +49,11 @@ typedef void (*CallbackDeliveryConfirmationReceived)(struct Confirmation*); typedef void (*CallbackReadConfirmationReceived)(struct Confirmation*); +struct ChatFFIMessageMetadata { + struct ChatByteVector *data; + int metadata_type; +}; + #ifdef __cplusplus extern "C" { #endif // __cplusplus @@ -315,6 +320,61 @@ void add_chat_message_metadata(struct Message *message, struct ChatByteVector *data, int *error_out); +/** + * Reads the message metadata of a message and returns a ptr to the metadata at the given position + * + * ## Arguments + * `message` - A pointer to a message + * `position` - The index of the array of metadata + * `error_out` - Pointer to an int which will be modified + * + * ## Returns + * `()` - Does not return a value, equivalent to void in C + * + * ## Safety + * `message` should be destroyed eventually + * the returned `ChatFFIMessageMetadata` should be destroyed eventually + */ +struct ChatFFIMessageMetadata *read_chat_metadata_at_position(struct ChatFFIMessage *message, + unsigned int position, + int *error_out); + +/** + * Returns the enum int representation of a metadata type + * + * ## Arguments + * `msg_metadata` - A pointer to a message metadat + * `error_out` - Pointer to an int which will be modified + * + * ## Returns + * `metadata_type` - An int8 that maps to MessageMetadataType enum + * '0' -> Reply + * '1' -> TokenRequest + * + * ## Safety + * `msg_metadata` should be destroyed eventually + */ +int read_chat_metadata_type(struct ChatFFIMessageMetadata *msg_metadata, int *error_out); + +/** + * Returns a ptr to a ByteVector + * + * ## Arguments + * `msg_metadata` - A pointer to a message metadata + * `error_out` - Pointer to an int which will be modified + * + * ## Returns + * `*mut ` - An int8 that maps to MessageMetadataType enum + * '0' -> Reply + * '1' -> TokenRequest + * + * ## Safety + * `msg_metadata` should be destroyed eventually + * the returned `ChatByteVector` should be destroyed eventually + */ +struct ChatByteVector *read_chat_metadata_data(struct ChatFFIMessageMetadata *msg_metadata, + int *error_out); + /** * Sends a read confirmation for a given message * @@ -423,7 +483,7 @@ void destroy_chat_ffi_liveness_data(struct ChatFFIContactsLivenessData *address) * Frees memory for a ChatFFIMessage * * ## Arguments - * `transport` - The pointer to a ChatFFIMessage + * `address` - The pointer to a ChatFFIMessage * * ## Returns * `()` - Does not return a value, equivalent to void in C @@ -433,6 +493,20 @@ void destroy_chat_ffi_liveness_data(struct ChatFFIContactsLivenessData *address) */ void destroy_chat_ffi_message(struct ChatFFIMessage *address); +/** + * Frees memory for a ChatMessageMetadataVector + * + * ## Arguments + * `address` - The pointer to a ChatMessageMetadataVector + * + * ## Returns + * `()` - Does not return a value, equivalent to void in C + * + * # Safety + * None + */ +void destroy_chat_message_metadata_vector(struct ChatMessageMetadataVector *address); + /** * Creates a ChatByteVector * diff --git a/base_layer/chat_ffi/src/message_metadata.rs b/base_layer/chat_ffi/src/message_metadata.rs index 2f77fdba32..2bcd44b9c0 100644 --- a/base_layer/chat_ffi/src/message_metadata.rs +++ b/base_layer/chat_ffi/src/message_metadata.rs @@ -33,7 +33,7 @@ use crate::{ #[derive(Debug, PartialEq, Clone)] #[repr(C)] pub struct ChatFFIMessageMetadata { - pub data: ChatByteVector, + pub data: *mut ChatByteVector, pub metadata_type: c_int, } @@ -110,7 +110,20 @@ pub unsafe extern "C" fn add_chat_message_metadata( (*message).push(metadata); } -#[allow(dead_code)] // Not dead code? False positive +/// Reads the message metadata of a message and returns a ptr to the metadata at the given position +/// +/// ## Arguments +/// `message` - A pointer to a message +/// `position` - The index of the array of metadata +/// `error_out` - Pointer to an int which will be modified +/// +/// ## Returns +/// `()` - Does not return a value, equivalent to void in C +/// +/// ## Safety +/// `message` should be destroyed eventually +/// the returned `ChatFFIMessageMetadata` should be destroyed eventually +#[no_mangle] pub unsafe extern "C" fn read_chat_metadata_at_position( message: *mut ChatFFIMessage, position: c_uint, @@ -133,7 +146,74 @@ pub unsafe extern "C" fn read_chat_metadata_at_position( ptr::swap(error_out, &mut error as *mut c_int); return ptr::null_mut(); } - Box::into_raw(Box::new(message.metadata.0[len as usize].clone())) + + let md_vec = &(*(message).metadata); + + let md = Box::new(md_vec.0[len as usize].clone()); + + Box::into_raw(md) +} + +/// Returns the enum int representation of a metadata type +/// +/// ## Arguments +/// `msg_metadata` - A pointer to a message metadat +/// `error_out` - Pointer to an int which will be modified +/// +/// ## Returns +/// `metadata_type` - An int8 that maps to MessageMetadataType enum +/// '0' -> Reply +/// '1' -> TokenRequest +/// +/// ## Safety +/// `msg_metadata` should be destroyed eventually +#[no_mangle] +pub unsafe extern "C" fn read_chat_metadata_type( + msg_metadata: *mut ChatFFIMessageMetadata, + error_out: *mut c_int, +) -> c_int { + let mut error = 0; + ptr::swap(error_out, &mut error as *mut c_int); + + if msg_metadata.is_null() { + error = LibChatError::from(InterfaceError::NullError("message".to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return -1; + } + + let md = &(*msg_metadata); + md.metadata_type +} + +/// Returns a ptr to a ByteVector +/// +/// ## Arguments +/// `msg_metadata` - A pointer to a message metadata +/// `error_out` - Pointer to an int which will be modified +/// +/// ## Returns +/// `*mut ` - An int8 that maps to MessageMetadataType enum +/// '0' -> Reply +/// '1' -> TokenRequest +/// +/// ## Safety +/// `msg_metadata` should be destroyed eventually +/// the returned `ChatByteVector` should be destroyed eventually +#[no_mangle] +pub unsafe extern "C" fn read_chat_metadata_data( + msg_metadata: *mut ChatFFIMessageMetadata, + error_out: *mut c_int, +) -> *mut ChatByteVector { + let mut error = 0; + ptr::swap(error_out, &mut error as *mut c_int); + + if msg_metadata.is_null() { + error = LibChatError::from(InterfaceError::NullError("message".to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); + } + + (*msg_metadata).data } #[cfg(test)] @@ -144,11 +224,8 @@ mod test { use tari_common_types::tari_address::TariAddress; use tari_contacts::contacts_service::types::MessageBuilder; - use super::add_chat_message_metadata; - use crate::{ - message_metadata::read_chat_metadata_at_position, - types::{chat_byte_vector_create, ChatFFIMessage}, - }; + use super::*; + use crate::types::{chat_byte_vector_create, ChatFFIMessage}; #[test] fn test_metadata_adding() { @@ -183,16 +260,27 @@ mod test { let data_bytes = data.as_bytes(); let len = u32::try_from(data.len()).expect("Can't cast from usize"); let data = chat_byte_vector_create(data_bytes.as_ptr(), len as c_uint, error_out); + let md_type = 0 as c_int; - add_chat_message_metadata(message_ptr, 0 as c_int, data, error_out); + add_chat_message_metadata(message_ptr, md_type, data, error_out); let chat_ffi_msg = ChatFFIMessage::try_from((*message_ptr).clone()).expect("A ChatFFI Message from a Message"); let chat_ffi_msg_ptr = Box::into_raw(Box::new(chat_ffi_msg)); - let metadata = &(*read_chat_metadata_at_position(chat_ffi_msg_ptr, 0, error_out)); + let metadata_ptr = read_chat_metadata_at_position(chat_ffi_msg_ptr, 0, error_out); + + let metadata_type = read_chat_metadata_type(metadata_ptr, error_out); + let metadata_byte_vector = read_chat_metadata_data(metadata_ptr, error_out); + + let mut metadata_data = vec![]; + + for i in 0..len { + metadata_data.push(chat_byte_vector_get_at(metadata_byte_vector, i, error_out)); + } - assert_eq!(metadata.data.0, data_bytes); + assert_eq!(metadata_type, md_type); + assert_eq!(metadata_data, data_bytes); } } } diff --git a/base_layer/chat_ffi/src/types/byte_vector.rs b/base_layer/chat_ffi/src/types/byte_vector.rs index 655b605a88..9986fc579e 100644 --- a/base_layer/chat_ffi/src/types/byte_vector.rs +++ b/base_layer/chat_ffi/src/types/byte_vector.rs @@ -121,6 +121,7 @@ pub unsafe extern "C" fn chat_byte_vector_get_at( ptr::swap(error_out, &mut error as *mut c_int); return 0u8; } + (*ptr).0[position as usize] } diff --git a/base_layer/chat_ffi/src/types/chat_ffi_message.rs b/base_layer/chat_ffi/src/types/chat_ffi_message.rs index d977afe185..a9a8ade771 100644 --- a/base_layer/chat_ffi/src/types/chat_ffi_message.rs +++ b/base_layer/chat_ffi/src/types/chat_ffi_message.rs @@ -36,7 +36,7 @@ pub struct ChatFFIMessage { pub from_address: *const c_char, pub stored_at: u64, pub message_id: *const c_char, - pub metadata: ChatMessageMetadataVector, + pub metadata: *mut ChatMessageMetadataVector, pub metadata_len: c_int, } @@ -61,8 +61,11 @@ impl TryFrom for ChatFFIMessage { let mut chat_message_metadata_bytes = vec![]; for md in v.metadata.clone() { + let data_ptr = Box::into_raw(Box::new(ChatByteVector( + md.data.clone().into_iter().map(|f| f as c_uchar).collect(), + ))); chat_message_metadata_bytes.push(ChatFFIMessageMetadata { - data: ChatByteVector(md.data.clone().into_iter().map(|f| f as c_uchar).collect()), + data: data_ptr, metadata_type: i32::from(md.metadata_type.as_byte()) as c_int, }); } @@ -72,12 +75,14 @@ impl TryFrom for ChatFFIMessage { Err(e) => return Err(e.to_string()), }; + let msg_md = Box::into_raw(Box::new(ChatMessageMetadataVector(chat_message_metadata_bytes))); + Ok(Self { body: body.as_ptr(), from_address: address.as_ptr(), stored_at: v.stored_at, message_id: id.as_ptr(), - metadata: ChatMessageMetadataVector(chat_message_metadata_bytes), + metadata: msg_md, metadata_len: metadata_length, }) } @@ -86,7 +91,7 @@ impl TryFrom for ChatFFIMessage { /// Frees memory for a ChatFFIMessage /// /// ## Arguments -/// `transport` - The pointer to a ChatFFIMessage +/// `address` - The pointer to a ChatFFIMessage /// /// ## Returns /// `()` - Does not return a value, equivalent to void in C @@ -99,3 +104,20 @@ pub unsafe extern "C" fn destroy_chat_ffi_message(address: *mut ChatFFIMessage) drop(Box::from_raw(address)) } } + +/// Frees memory for a ChatMessageMetadataVector +/// +/// ## Arguments +/// `address` - The pointer to a ChatMessageMetadataVector +/// +/// ## Returns +/// `()` - Does not return a value, equivalent to void in C +/// +/// # Safety +/// None +#[no_mangle] +pub unsafe extern "C" fn destroy_chat_message_metadata_vector(address: *mut ChatMessageMetadataVector) { + if !address.is_null() { + drop(Box::from_raw(address)) + } +}