♻️ Make Message::as_string simpler

quickfix-ffi/quickfix-bind/include/quickfix_bind.h

@@ -134,7 +134,8 @@ const char *FixMessage_getField(const FixMessage_t *obj, int32_t tag);
 int8_t FixMessage_setField(FixMessage_t *obj, int32_t tag, const char *value);
 int8_t FixMessage_removeField(FixMessage_t *obj, int32_t tag);
 int8_t FixMessage_addGroup(FixMessage_t *obj, const FixGroup_t *group);
-int8_t FixMessage_toBuffer(const FixMessage_t *obj, char *buffer, uint64_t length);
+int64_t FixMessage_getStringLen(const FixMessage_t *obj);
+int8_t FixMessage_readString(const FixMessage_t *obj, char *buffer, int64_t buffer_len);
 void FixMessage_delete(const FixMessage_t *obj);
 
 FixHeader_t *FixHeader_new();

quickfix-ffi/quickfix-bind/src/quickfix_bind.cpp

@@ -281,21 +281,22 @@ int64_t FixDictionary_getStringLen(const Dictionary *obj, const char *key) {
   RETURN_VAL_IF_NULL(obj, ERRNO_INVAL);
   RETURN_VAL_IF_NULL(key, ERRNO_INVAL);
 
-  CATCH_OR_RETURN_ERRNO({ return obj->getString(key).size(); })
+  CATCH_OR_RETURN_ERRNO({ return obj->getString(key).size() + 1; })
 }
 
 int8_t FixDictionary_readString(const Dictionary *obj, const char *key, char *buffer, int64_t buffer_len) {
   RETURN_VAL_IF_NULL(obj, ERRNO_INVAL);
   RETURN_VAL_IF_NULL(key, ERRNO_INVAL);
+  RETURN_VAL_IF_NULL(buffer, ERRNO_INVAL);
 
   CATCH_OR_RETURN_ERRNO({
     auto value = obj->getString(key);
-    if (buffer_len <= value.length()) {
+    if (buffer_len <= value.size()) {
       return ERRNO_BUFFER_TO_SMALL;
     }
 
     strncpy(buffer, value.c_str(), buffer_len);
-    buffer[value.length()] = '\0';
+    buffer[value.size()] = '\0';
 
     return 0;
   })
@@ -604,24 +605,27 @@ int8_t FixMessage_addGroup(Message *obj, const Group *group) {
   })
 }
 
-int8_t FixMessage_toBuffer(const Message *obj, char *buffer, uint64_t length) {
-  if (length == 0)
-    return 0;
+int64_t FixMessage_getStringLen(const FixMessage_t *obj) {
+  RETURN_VAL_IF_NULL(obj, ERRNO_INVAL);
+
+  CATCH_OR_RETURN_ERRNO({ return obj->toString().size() + 1; });
+}
 
+int8_t FixMessage_readString(const FixMessage_t *obj, char *buffer, int64_t buffer_len) {
   RETURN_VAL_IF_NULL(obj, ERRNO_INVAL);
   RETURN_VAL_IF_NULL(buffer, ERRNO_INVAL);
 
   CATCH_OR_RETURN_ERRNO({
-    auto repr = obj->toString();
-    if (length <= repr.length()) {
+    auto value = obj->toString();
+    if (buffer_len <= value.size()) {
       return ERRNO_BUFFER_TO_SMALL;
     }
 
-    strncpy(buffer, repr.c_str(), length);
-    buffer[repr.length()] = '\0';
+    strncpy(buffer, value.c_str(), buffer_len);
+    buffer[value.size()] = '\0';
 
     return 0;
-  });
+  })
 }
 
 void FixMessage_delete(const Message *obj) {

quickfix-ffi/src/lib.rs

@@ -319,8 +319,15 @@ extern "C" {
     #[must_use]
     pub fn FixMessage_addGroup(obj: FixMessage_t, group: FixGroup_t) -> i8;
 
+    pub fn FixMessage_getStringLen(obj: FixMessage_t) -> i64;
+
     #[must_use]
-    pub fn FixMessage_toBuffer(obj: FixMessage_t, buffer: *mut ffi::c_char, length: u64) -> i8;
+    pub fn FixMessage_readString(
+        obj: FixMessage_t,
+        buffer: *mut ffi::c_char,
+        buffer_len: i64,
+    ) -> i8;
+
     pub fn FixMessage_delete(obj: FixMessage_t);
 
     // Header

quickfix/src/dictionary.rs

@@ -1,7 +1,4 @@
-use std::{
-    ffi::{CStr, CString},
-    fmt,
-};
+use std::{ffi::CString, fmt};
 
 use quickfix_ffi::{
     FixDictionary_delete, FixDictionary_getBool, FixDictionary_getDay, FixDictionary_getDouble,
@@ -61,13 +58,11 @@ impl PropertyContainer<String> for Dictionary {
     fn ffi_get(&self, key: CString) -> Result<String, QuickFixError> {
         unsafe {
             // Prepare output buffer
-            let mut buffer_len = FixDictionary_getStringLen(self.0, key.as_ptr());
+            let buffer_len = FixDictionary_getStringLen(self.0, key.as_ptr());
             if buffer_len < 0 {
                 return Err(QuickFixError::InvalidFunctionReturnCode(buffer_len as i8));
             }
 
-            buffer_len += 1; // Add null end char
-
             // Allocate buffer on rust side
             let mut buffer = vec![0_u8; buffer_len as usize];
             assert_eq!(buffer.len(), buffer_len as usize);
@@ -85,7 +80,7 @@ impl PropertyContainer<String> for Dictionary {
             // NOTE: Here, I deliberately made the choice to drop C weird string / invalid UTF8 string
             //       content. If this happen, there is not so much we can do about ...
             //       Returning no error is sometime nicer, than an incomprehensible error.
-            let text = CStr::from_bytes_with_nul(&buffer).unwrap_or_default();
+            let text = CString::from_vec_with_nul(buffer).unwrap_or_default();
             Ok(text.to_string_lossy().to_string())
         }
     }

quickfix/src/message.rs

@@ -3,15 +3,15 @@ use std::{ffi::CString, fmt, mem::ManuallyDrop};
 use quickfix_ffi::{
     FixMessage_addGroup, FixMessage_copyGroup, FixMessage_copyHeader, FixMessage_copyTrailer,
     FixMessage_delete, FixMessage_fromString, FixMessage_getField, FixMessage_getGroupRef,
-    FixMessage_getHeaderRef, FixMessage_getTrailerRef, FixMessage_new, FixMessage_removeField,
-    FixMessage_setField, FixMessage_t, FixMessage_toBuffer,
+    FixMessage_getHeaderRef, FixMessage_getStringLen, FixMessage_getTrailerRef, FixMessage_new,
+    FixMessage_readString, FixMessage_removeField, FixMessage_setField, FixMessage_t,
 };
 
 use crate::{
     group::Group,
     header::Header,
     trailer::Trailer,
-    utils::{ffi_code_to_result, read_buffer_to_string, read_checked_cstr},
+    utils::{ffi_code_to_result, read_checked_cstr},
     AsFixValue, FieldMap, QuickFixError,
 };
 
@@ -32,18 +32,41 @@ impl Message {
             .ok_or(QuickFixError::NullFunctionReturn)
     }
 
-    /// Try reading underlying struct buffer as a string of 1 page size.
+    /// Try reading underlying struct buffer as a string.
+    ///
+    /// # Performances
+    ///
+    /// Do not use this method in latency sensitive code.
+    ///
+    /// String will be generated twice in C++ code:
+    /// - Once for getting a safe buffer length.
+    /// - Then to copy buffer to rust "memory".
     pub fn as_string(&self) -> Result<String, QuickFixError> {
-        self.as_string_with_len(4096 /* 1 page */)
-    }
-
-    /// Try reading underlying struct buffer with a custom buffer size.
-    pub fn as_string_with_len(&self, max_len: usize) -> Result<String, QuickFixError> {
-        let mut buffer = vec![0_u8; max_len];
-        let buffer_ptr = buffer.as_mut_ptr() as *mut i8;
-        match unsafe { FixMessage_toBuffer(self.0, buffer_ptr, max_len as u64) } {
-            0 => Ok(read_buffer_to_string(&buffer)),
-            code => Err(QuickFixError::InvalidFunctionReturnCode(code)),
+        unsafe {
+            // Prepare output buffer
+            let buffer_len = FixMessage_getStringLen(self.0);
+            if buffer_len < 0 {
+                return Err(QuickFixError::InvalidFunctionReturnCode(buffer_len as i8));
+            }
+
+            // Allocate buffer on rust side
+            let mut buffer = vec![0_u8; buffer_len as usize];
+            assert_eq!(buffer.len(), buffer_len as usize);
+
+            // Read text
+            ffi_code_to_result(FixMessage_readString(
+                self.0,
+                buffer.as_mut_ptr().cast(),
+                buffer_len,
+            ))?;
+
+            // Convert to String
+            //
+            // NOTE: Here, I deliberately made the choice to drop C weird string / invalid UTF8 string
+            //       content. If this happen, there is not so much we can do about ...
+            //       Returning no error is sometime nicer, than an incomprehensible error.
+            let text = CString::from_vec_with_nul(buffer).unwrap_or_default();
+            Ok(text.to_string_lossy().to_string())
         }
     }
 

quickfix/src/utils.rs

@@ -5,12 +5,6 @@ use std::{
 
 use crate::QuickFixError;
 
-#[inline(always)]
-pub fn read_buffer_to_string(buffer: &[u8]) -> String {
-    let null_index = buffer.iter().position(|x| *x == 0).unwrap_or(buffer.len());
-    String::from_utf8_lossy(&buffer[..null_index]).to_string()
-}
-
 #[inline(always)]
 pub fn read_checked_cstr(val: NonNull<ffi::c_char>) -> String {
     let cstr = unsafe { CStr::from_ptr(val.as_ptr()) };

quickfix/tests/test_messages.rs

@@ -4,19 +4,9 @@ use quickfix::*;
 fn test_read_empy_message() {
     let msg = Message::new();
     assert_eq!(msg.as_string().as_deref(), Ok("9=0\u{1}10=167\u{1}"));
-}
 
-#[test]
-fn test_as_string() {
     let msg = Message::try_from_text("9=0\u{1}10=000\u{1}").unwrap();
-    assert_eq!(
-        msg.as_string_with_len(512).as_deref(),
-        Ok("9=0\u{1}10=167\u{1}")
-    );
-    assert_eq!(
-        msg.as_string_with_len(6),
-        Err(QuickFixError::InvalidFunctionReturnCode(-3))
-    );
+    assert_eq!(msg.as_string().as_deref(), Ok("9=0\u{1}10=167\u{1}"));
 }
 
 #[test]