[crashtracking] Little improvements to the FFI api (#842)

* Use usize instead of string in the FFI api
* Add code in example
* Move around StringWrapper and co, so only one declaration/definition exist
* Shorten some types name

crashtracker-ffi/cbindgen.toml

@@ -35,6 +35,12 @@ renaming_overrides_prefixing = true
 "Vec_Tag" = "ddog_Vec_Tag"
 "Vec_U8" = "ddog_Vec_U8"
 "VoidResult" = "ddog_VoidResult"
+"StringWrapper" = "ddog_StringWrapper"
+"StringWrapperResult" = "ddog_StringWrapperResult"
+"CrashInfoBuilderNewResult" = " ddog_crasht_CrashInfoBuilder_NewResult"
+"StackTraceNewResult" = " ddog_crasht_StackTrace_NewResult"
+"StackFrameNewResult" = "ddog_crasht_StackFrame_NewResult"
+"CrashInfoNewResult" = "ddog_crasht_CrashInfo_NewResult"
 
 [export.mangle]
 rename_types = "PascalCase"

crashtracker-ffi/src/crash_info/builder.rs

@@ -5,21 +5,28 @@ use super::{Metadata, OsInfo, ProcInfo, SigInfo, Span, ThreadData};
 use ::function_name::named;
 use datadog_crashtracker::{CrashInfo, CrashInfoBuilder, ErrorKind, StackTrace};
 use ddcommon_ffi::{
-    slice::AsBytes, wrap_with_ffi_result, wrap_with_void_ffi_result, CharSlice, Handle, Result,
+    slice::AsBytes, wrap_with_ffi_result, wrap_with_void_ffi_result, CharSlice, Error, Handle,
     Slice, Timespec, ToInner, VoidResult,
 };
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 //                                              FFI API                                           //
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
+#[allow(dead_code)]
+#[repr(C)]
+pub enum CrashInfoBuilderNewResult {
+    Ok(Handle<CrashInfoBuilder>),
+    Err(Error),
+}
+
 /// Create a new CrashInfoBuilder, and returns an opaque reference to it.
 /// # Safety
 /// No safety issues.
 #[no_mangle]
 #[must_use]
-pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_new() -> Result<Handle<CrashInfoBuilder>> {
-    ddcommon_ffi::Result::Ok(CrashInfoBuilder::new().into())
+pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_new() -> CrashInfoBuilderNewResult {
+    CrashInfoBuilderNewResult::Ok(CrashInfoBuilder::new().into())
 }
 
 /// # Safety
@@ -34,15 +41,31 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut Handle<
     }
 }
 
+#[allow(dead_code)]
+#[repr(C)]
+pub enum CrashInfoNewResult {
+    Ok(Handle<CrashInfo>),
+    Err(Error),
+}
+
 /// # Safety
 /// The `builder` can be null, but if non-null it must point to a Builder made by this module,
 /// which has not previously been dropped.
 #[no_mangle]
 #[must_use]
-#[named]
 pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_build(
+    builder: *mut Handle<CrashInfoBuilder>,
+) -> CrashInfoNewResult {
+    match ddog_crasht_crash_info_builder_build_impl(builder) {
+        Ok(crash_info) => CrashInfoNewResult::Ok(crash_info),
+        Err(err) => CrashInfoNewResult::Err(err.into()),
+    }
+}
+
+#[named]
+unsafe fn ddog_crasht_crash_info_builder_build_impl(
     mut builder: *mut Handle<CrashInfoBuilder>,
-) -> Result<Handle<CrashInfo>> {
+) -> anyhow::Result<Handle<CrashInfo>> {
     wrap_with_ffi_result!({ anyhow::Ok(builder.take()?.build()?.into()) })
 }
 
@@ -313,9 +336,13 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack(
 #[named]
 pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread(
     mut builder: *mut Handle<CrashInfoBuilder>,
-    thread: ThreadData,
+    mut thread: ThreadData,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
+        if thread.crashed {
+            let stack = (thread.stack.to_inner_mut())?.clone();
+            builder.to_inner_mut()?.with_stack(stack)?;
+        }
         builder.to_inner_mut()?.with_thread(thread.try_into()?)?;
     })
 }

crashtracker-ffi/src/crash_info/stackframe.rs

@@ -4,20 +4,28 @@
 use ::function_name::named;
 use datadog_crashtracker::{BuildIdType, FileType, StackFrame};
 use ddcommon_ffi::{
-    slice::AsBytes, wrap_with_void_ffi_result, CharSlice, Handle, Result, ToInner, VoidResult,
+    slice::AsBytes, utils::ToHexStr, wrap_with_void_ffi_result, CharSlice, Error, Handle, ToInner,
+    VoidResult,
 };
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 //                                              FFI API                                           //
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
+#[allow(dead_code)]
+#[repr(C)]
+pub enum StackFrameNewResult {
+    Ok(Handle<StackFrame>),
+    Err(Error),
+}
+
 /// Create a new StackFrame, and returns an opaque reference to it.
 /// # Safety
 /// No safety issues.
 #[no_mangle]
 #[must_use]
-pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> Result<Handle<StackFrame>> {
-    ddcommon_ffi::Result::Ok(StackFrame::new().into())
+pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult {
+    StackFrameNewResult::Ok(StackFrame::new().into())
 }
 
 /// # Safety
@@ -41,10 +49,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut Handle<StackFra
 #[named]
 pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip(
     mut frame: *mut Handle<StackFrame>,
-    ip: CharSlice,
+    ip: usize,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
-        frame.to_inner_mut()?.ip = ip.try_to_string_option()?;
+        frame.to_inner_mut()?.ip = Some(ip.to_hex_str());
     })
 }
 
@@ -57,10 +65,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip(
 #[named]
 pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address(
     mut frame: *mut Handle<StackFrame>,
-    module_base_address: CharSlice,
+    module_base_address: usize,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
-        frame.to_inner_mut()?.module_base_address = module_base_address.try_to_string_option()?;
+        frame.to_inner_mut()?.module_base_address = Some(module_base_address.to_hex_str());
     })
 }
 
@@ -73,10 +81,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address(
 #[named]
 pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp(
     mut frame: *mut Handle<StackFrame>,
-    sp: CharSlice,
+    sp: usize,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
-        frame.to_inner_mut()?.sp = sp.try_to_string_option()?;
+        frame.to_inner_mut()?.sp = Some(sp.to_hex_str());
     })
 }
 
@@ -89,10 +97,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp(
 #[named]
 pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address(
     mut frame: *mut Handle<StackFrame>,
-    symbol_address: CharSlice,
+    symbol_address: usize,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
-        frame.to_inner_mut()?.symbol_address = symbol_address.try_to_string_option()?;
+        frame.to_inner_mut()?.symbol_address = Some(symbol_address.to_hex_str());
     })
 }
 
@@ -169,10 +177,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path(
 #[named]
 pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address(
     mut frame: *mut Handle<StackFrame>,
-    relative_address: CharSlice,
+    relative_address: usize,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
-        frame.to_inner_mut()?.relative_address = relative_address.try_to_string_option()?;
+        frame.to_inner_mut()?.relative_address = Some(relative_address.to_hex_str());
     })
 }
 

crashtracker-ffi/src/crash_info/stacktrace.rs

@@ -3,19 +3,26 @@
 
 use ::function_name::named;
 use datadog_crashtracker::{StackFrame, StackTrace};
-use ddcommon_ffi::{wrap_with_void_ffi_result, Handle, Result, ToInner, VoidResult};
+use ddcommon_ffi::{wrap_with_void_ffi_result, Error, Handle, ToInner, VoidResult};
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 //                                              FFI API                                           //
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
+#[allow(dead_code)]
+#[repr(C)]
+pub enum StackTraceNewResult {
+    Ok(Handle<StackTrace>),
+    Err(Error),
+}
+
 /// Create a new StackTrace, and returns an opaque reference to it.
 /// # Safety
 /// No safety issues.
 #[no_mangle]
 #[must_use]
-pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> Result<Handle<StackTrace>> {
-    ddcommon_ffi::Result::Ok(StackTrace::new_incomplete().into())
+pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> StackTraceNewResult {
+    StackTraceNewResult::Ok(StackTrace::new_incomplete().into())
 }
 
 /// # Safety

crashtracker-ffi/src/demangler/datatypes.rs

@@ -1,42 +1,8 @@
 // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
-use ddcommon_ffi::{Error, StringWrapper};
-
 #[repr(C)]
 pub enum DemangleOptions {
     Complete,
     NameOnly,
 }
-
-#[repr(C)]
-pub enum StringWrapperResult {
-    Ok(StringWrapper),
-    #[allow(dead_code)]
-    Err(Error),
-}
-
-// Useful for testing
-impl StringWrapperResult {
-    pub fn unwrap(self) -> StringWrapper {
-        match self {
-            StringWrapperResult::Ok(s) => s,
-            StringWrapperResult::Err(e) => panic!("{e}"),
-        }
-    }
-}
-
-impl From<anyhow::Result<String>> for StringWrapperResult {
-    fn from(value: anyhow::Result<String>) -> Self {
-        match value {
-            Ok(x) => Self::Ok(x.into()),
-            Err(err) => Self::Err(err.into()),
-        }
-    }
-}
-
-impl From<String> for StringWrapperResult {
-    fn from(value: String) -> Self {
-        Self::Ok(value.into())
-    }
-}

crashtracker-ffi/src/demangler/mod.rs

@@ -4,7 +4,7 @@ mod datatypes;
 pub use datatypes::*;
 
 use ::function_name::named;
-use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, Result, StringWrapper};
+use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, StringWrapperResult};
 use symbolic_common::Name;
 use symbolic_demangle::Demangle;
 
@@ -20,15 +20,15 @@ use symbolic_demangle::Demangle;
 pub unsafe extern "C" fn ddog_crasht_demangle(
     name: CharSlice,
     options: DemangleOptions,
-) -> Result<StringWrapper> {
+) -> StringWrapperResult {
     wrap_with_ffi_result!({
         let name = name.to_utf8_lossy();
         let name = Name::from(name);
         let options = match options {
             DemangleOptions::Complete => symbolic_demangle::DemangleOptions::complete(),
             DemangleOptions::NameOnly => symbolic_demangle::DemangleOptions::name_only(),
         };
-        anyhow::Ok(name.demangle(options).unwrap_or_default().into())
+        anyhow::Ok(name.demangle(options).unwrap_or_default())
     })
 }
 

crashtracker/src/crash_info/stacktrace.rs

@@ -193,7 +193,6 @@ pub enum BuildIdType {
     GNU,
     GO,
     PDB,
-    PE,
     SHA1,
 }
 
@@ -203,7 +202,7 @@ pub enum BuildIdType {
 pub enum FileType {
     APK,
     ELF,
-    PDB,
+    PE,
 }
 
 #[cfg(unix)]

ddcommon-ffi/src/string.rs

@@ -3,6 +3,7 @@
 
 use crate::slice::CharSlice;
 use crate::vec::Vec;
+use crate::Error;
 
 /// A wrapper for returning owned strings from FFI
 #[derive(Debug)]
@@ -72,3 +73,35 @@ pub unsafe extern "C" fn ddog_StringWrapper_message(s: Option<&StringWrapper>) -
         Some(s) => CharSlice::from(s.as_ref()),
     }
 }
+
+#[repr(C)]
+#[allow(dead_code)]
+pub enum StringWrapperResult {
+    Ok(StringWrapper),
+    Err(Error),
+}
+
+// Useful for testing
+impl StringWrapperResult {
+    pub fn unwrap(self) -> StringWrapper {
+        match self {
+            StringWrapperResult::Ok(s) => s,
+            StringWrapperResult::Err(e) => panic!("{e}"),
+        }
+    }
+}
+
+impl From<anyhow::Result<String>> for StringWrapperResult {
+    fn from(value: anyhow::Result<String>) -> Self {
+        match value {
+            Ok(x) => Self::Ok(x.into()),
+            Err(err) => Self::Err(err.into()),
+        }
+    }
+}
+
+impl From<String> for StringWrapperResult {
+    fn from(value: String) -> Self {
+        Self::Ok(value.into())
+    }
+}

ddcommon-ffi/src/utils.rs

@@ -28,3 +28,13 @@ macro_rules! wrap_with_void_ffi_result {
         .into()
     }};
 }
+
+pub trait ToHexStr {
+    fn to_hex_str(&self) -> String;
+}
+
+impl ToHexStr for usize {
+    fn to_hex_str(&self) -> String {
+        format!("0x{:X}", self)
+    }
+}