Skip to content

Commit

Permalink
fix(chatffi): return and read from ptrs (#5827)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
brianp authored Oct 4, 2023
1 parent d81fe7d commit dd2eddb
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 17 deletions.
78 changes: 76 additions & 2 deletions base_layer/chat_ffi/chat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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
Expand All @@ -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
*
Expand Down
110 changes: 99 additions & 11 deletions base_layer/chat_ffi/src/message_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
Expand All @@ -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)]
Expand All @@ -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() {
Expand Down Expand Up @@ -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);
}
}
}
1 change: 1 addition & 0 deletions base_layer/chat_ffi/src/types/byte_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}

Expand Down
30 changes: 26 additions & 4 deletions base_layer/chat_ffi/src/types/chat_ffi_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -61,8 +61,11 @@ impl TryFrom<Message> 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,
});
}
Expand All @@ -72,12 +75,14 @@ impl TryFrom<Message> 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,
})
}
Expand All @@ -86,7 +91,7 @@ impl TryFrom<Message> 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
Expand All @@ -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))
}
}

0 comments on commit dd2eddb

Please sign in to comment.