Skip to content

Commit 6f5396a

Browse files
gleocadiegleocadie
authored
[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
1 parent f2fb806 commit 6f5396a

File tree

12 files changed

+205
-98
lines changed

12 files changed

+205
-98
lines changed

‎crashtracker-ffi/cbindgen.toml

+6
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ renaming_overrides_prefixing = true
3535
"Vec_Tag" = "ddog_Vec_Tag"
3636
"Vec_U8" = "ddog_Vec_U8"
3737
"VoidResult" = "ddog_VoidResult"
38+
"StringWrapper" = "ddog_StringWrapper"
39+
"StringWrapperResult" = "ddog_StringWrapperResult"
40+
"CrashInfoBuilderNewResult" = " ddog_crasht_CrashInfoBuilder_NewResult"
41+
"StackTraceNewResult" = " ddog_crasht_StackTrace_NewResult"
42+
"StackFrameNewResult" = "ddog_crasht_StackFrame_NewResult"
43+
"CrashInfoNewResult" = "ddog_crasht_CrashInfo_NewResult"
3844

3945
[export.mangle]
4046
rename_types = "PascalCase"

‎crashtracker-ffi/src/crash_info/builder.rs

+33-6
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,28 @@ use super::{Metadata, OsInfo, ProcInfo, SigInfo, Span, ThreadData};
55
use ::function_name::named;
66
use datadog_crashtracker::{CrashInfo, CrashInfoBuilder, ErrorKind, StackTrace};
77
use ddcommon_ffi::{
8-
slice::AsBytes, wrap_with_ffi_result, wrap_with_void_ffi_result, CharSlice, Handle, Result,
8+
slice::AsBytes, wrap_with_ffi_result, wrap_with_void_ffi_result, CharSlice, Error, Handle,
99
Slice, Timespec, ToInner, VoidResult,
1010
};
1111

1212
////////////////////////////////////////////////////////////////////////////////////////////////////
1313
// FFI API //
1414
////////////////////////////////////////////////////////////////////////////////////////////////////
1515

16+
#[allow(dead_code)]
17+
#[repr(C)]
18+
pub enum CrashInfoBuilderNewResult {
19+
Ok(Handle<CrashInfoBuilder>),
20+
Err(Error),
21+
}
22+
1623
/// Create a new CrashInfoBuilder, and returns an opaque reference to it.
1724
/// # Safety
1825
/// No safety issues.
1926
#[no_mangle]
2027
#[must_use]
21-
pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_new() -> Result<Handle<CrashInfoBuilder>> {
22-
ddcommon_ffi::Result::Ok(CrashInfoBuilder::new().into())
28+
pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_new() -> CrashInfoBuilderNewResult {
29+
CrashInfoBuilderNewResult::Ok(CrashInfoBuilder::new().into())
2330
}
2431

2532
/// # Safety
@@ -34,15 +41,31 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_drop(builder: *mut Handle<
3441
}
3542
}
3643

44+
#[allow(dead_code)]
45+
#[repr(C)]
46+
pub enum CrashInfoNewResult {
47+
Ok(Handle<CrashInfo>),
48+
Err(Error),
49+
}
50+
3751
/// # Safety
3852
/// The `builder` can be null, but if non-null it must point to a Builder made by this module,
3953
/// which has not previously been dropped.
4054
#[no_mangle]
4155
#[must_use]
42-
#[named]
4356
pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_build(
57+
builder: *mut Handle<CrashInfoBuilder>,
58+
) -> CrashInfoNewResult {
59+
match ddog_crasht_crash_info_builder_build_impl(builder) {
60+
Ok(crash_info) => CrashInfoNewResult::Ok(crash_info),
61+
Err(err) => CrashInfoNewResult::Err(err.into()),
62+
}
63+
}
64+
65+
#[named]
66+
unsafe fn ddog_crasht_crash_info_builder_build_impl(
4467
mut builder: *mut Handle<CrashInfoBuilder>,
45-
) -> Result<Handle<CrashInfo>> {
68+
) -> anyhow::Result<Handle<CrashInfo>> {
4669
wrap_with_ffi_result!({ anyhow::Ok(builder.take()?.build()?.into()) })
4770
}
4871

@@ -313,9 +336,13 @@ pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_stack(
313336
#[named]
314337
pub unsafe extern "C" fn ddog_crasht_CrashInfoBuilder_with_thread(
315338
mut builder: *mut Handle<CrashInfoBuilder>,
316-
thread: ThreadData,
339+
mut thread: ThreadData,
317340
) -> VoidResult {
318341
wrap_with_void_ffi_result!({
342+
if thread.crashed {
343+
let stack = (thread.stack.to_inner_mut())?.clone();
344+
builder.to_inner_mut()?.with_stack(stack)?;
345+
}
319346
builder.to_inner_mut()?.with_thread(thread.try_into()?)?;
320347
})
321348
}

‎crashtracker-ffi/src/crash_info/stackframe.rs

+21-13
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,28 @@
44
use ::function_name::named;
55
use datadog_crashtracker::{BuildIdType, FileType, StackFrame};
66
use ddcommon_ffi::{
7-
slice::AsBytes, wrap_with_void_ffi_result, CharSlice, Handle, Result, ToInner, VoidResult,
7+
slice::AsBytes, utils::ToHexStr, wrap_with_void_ffi_result, CharSlice, Error, Handle, ToInner,
8+
VoidResult,
89
};
910

1011
////////////////////////////////////////////////////////////////////////////////////////////////////
1112
// FFI API //
1213
////////////////////////////////////////////////////////////////////////////////////////////////////
1314

15+
#[allow(dead_code)]
16+
#[repr(C)]
17+
pub enum StackFrameNewResult {
18+
Ok(Handle<StackFrame>),
19+
Err(Error),
20+
}
21+
1422
/// Create a new StackFrame, and returns an opaque reference to it.
1523
/// # Safety
1624
/// No safety issues.
1725
#[no_mangle]
1826
#[must_use]
19-
pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> Result<Handle<StackFrame>> {
20-
ddcommon_ffi::Result::Ok(StackFrame::new().into())
27+
pub unsafe extern "C" fn ddog_crasht_StackFrame_new() -> StackFrameNewResult {
28+
StackFrameNewResult::Ok(StackFrame::new().into())
2129
}
2230

2331
/// # Safety
@@ -41,10 +49,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_drop(frame: *mut Handle<StackFra
4149
#[named]
4250
pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip(
4351
mut frame: *mut Handle<StackFrame>,
44-
ip: CharSlice,
52+
ip: usize,
4553
) -> VoidResult {
4654
wrap_with_void_ffi_result!({
47-
frame.to_inner_mut()?.ip = ip.try_to_string_option()?;
55+
frame.to_inner_mut()?.ip = Some(ip.to_hex_str());
4856
})
4957
}
5058

@@ -57,10 +65,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_ip(
5765
#[named]
5866
pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address(
5967
mut frame: *mut Handle<StackFrame>,
60-
module_base_address: CharSlice,
68+
module_base_address: usize,
6169
) -> VoidResult {
6270
wrap_with_void_ffi_result!({
63-
frame.to_inner_mut()?.module_base_address = module_base_address.try_to_string_option()?;
71+
frame.to_inner_mut()?.module_base_address = Some(module_base_address.to_hex_str());
6472
})
6573
}
6674

@@ -73,10 +81,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_module_base_address(
7381
#[named]
7482
pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp(
7583
mut frame: *mut Handle<StackFrame>,
76-
sp: CharSlice,
84+
sp: usize,
7785
) -> VoidResult {
7886
wrap_with_void_ffi_result!({
79-
frame.to_inner_mut()?.sp = sp.try_to_string_option()?;
87+
frame.to_inner_mut()?.sp = Some(sp.to_hex_str());
8088
})
8189
}
8290

@@ -89,10 +97,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_sp(
8997
#[named]
9098
pub unsafe extern "C" fn ddog_crasht_StackFrame_with_symbol_address(
9199
mut frame: *mut Handle<StackFrame>,
92-
symbol_address: CharSlice,
100+
symbol_address: usize,
93101
) -> VoidResult {
94102
wrap_with_void_ffi_result!({
95-
frame.to_inner_mut()?.symbol_address = symbol_address.try_to_string_option()?;
103+
frame.to_inner_mut()?.symbol_address = Some(symbol_address.to_hex_str());
96104
})
97105
}
98106

@@ -169,10 +177,10 @@ pub unsafe extern "C" fn ddog_crasht_StackFrame_with_path(
169177
#[named]
170178
pub unsafe extern "C" fn ddog_crasht_StackFrame_with_relative_address(
171179
mut frame: *mut Handle<StackFrame>,
172-
relative_address: CharSlice,
180+
relative_address: usize,
173181
) -> VoidResult {
174182
wrap_with_void_ffi_result!({
175-
frame.to_inner_mut()?.relative_address = relative_address.try_to_string_option()?;
183+
frame.to_inner_mut()?.relative_address = Some(relative_address.to_hex_str());
176184
})
177185
}
178186

‎crashtracker-ffi/src/crash_info/stacktrace.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,26 @@
33

44
use ::function_name::named;
55
use datadog_crashtracker::{StackFrame, StackTrace};
6-
use ddcommon_ffi::{wrap_with_void_ffi_result, Handle, Result, ToInner, VoidResult};
6+
use ddcommon_ffi::{wrap_with_void_ffi_result, Error, Handle, ToInner, VoidResult};
77

88
////////////////////////////////////////////////////////////////////////////////////////////////////
99
// FFI API //
1010
////////////////////////////////////////////////////////////////////////////////////////////////////
1111

12+
#[allow(dead_code)]
13+
#[repr(C)]
14+
pub enum StackTraceNewResult {
15+
Ok(Handle<StackTrace>),
16+
Err(Error),
17+
}
18+
1219
/// Create a new StackTrace, and returns an opaque reference to it.
1320
/// # Safety
1421
/// No safety issues.
1522
#[no_mangle]
1623
#[must_use]
17-
pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> Result<Handle<StackTrace>> {
18-
ddcommon_ffi::Result::Ok(StackTrace::new_incomplete().into())
24+
pub unsafe extern "C" fn ddog_crasht_StackTrace_new() -> StackTraceNewResult {
25+
StackTraceNewResult::Ok(StackTrace::new_incomplete().into())
1926
}
2027

2128
/// # Safety

‎crashtracker-ffi/src/demangler/datatypes.rs

-34
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,8 @@
11
// Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
22
// SPDX-License-Identifier: Apache-2.0
33

4-
use ddcommon_ffi::{Error, StringWrapper};
5-
64
#[repr(C)]
75
pub enum DemangleOptions {
86
Complete,
97
NameOnly,
108
}
11-
12-
#[repr(C)]
13-
pub enum StringWrapperResult {
14-
Ok(StringWrapper),
15-
#[allow(dead_code)]
16-
Err(Error),
17-
}
18-
19-
// Useful for testing
20-
impl StringWrapperResult {
21-
pub fn unwrap(self) -> StringWrapper {
22-
match self {
23-
StringWrapperResult::Ok(s) => s,
24-
StringWrapperResult::Err(e) => panic!("{e}"),
25-
}
26-
}
27-
}
28-
29-
impl From<anyhow::Result<String>> for StringWrapperResult {
30-
fn from(value: anyhow::Result<String>) -> Self {
31-
match value {
32-
Ok(x) => Self::Ok(x.into()),
33-
Err(err) => Self::Err(err.into()),
34-
}
35-
}
36-
}
37-
38-
impl From<String> for StringWrapperResult {
39-
fn from(value: String) -> Self {
40-
Self::Ok(value.into())
41-
}
42-
}

‎crashtracker-ffi/src/demangler/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod datatypes;
44
pub use datatypes::*;
55

66
use ::function_name::named;
7-
use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, Result, StringWrapper};
7+
use ddcommon_ffi::{slice::AsBytes, wrap_with_ffi_result, CharSlice, StringWrapperResult};
88
use symbolic_common::Name;
99
use symbolic_demangle::Demangle;
1010

@@ -20,15 +20,15 @@ use symbolic_demangle::Demangle;
2020
pub unsafe extern "C" fn ddog_crasht_demangle(
2121
name: CharSlice,
2222
options: DemangleOptions,
23-
) -> Result<StringWrapper> {
23+
) -> StringWrapperResult {
2424
wrap_with_ffi_result!({
2525
let name = name.to_utf8_lossy();
2626
let name = Name::from(name);
2727
let options = match options {
2828
DemangleOptions::Complete => symbolic_demangle::DemangleOptions::complete(),
2929
DemangleOptions::NameOnly => symbolic_demangle::DemangleOptions::name_only(),
3030
};
31-
anyhow::Ok(name.demangle(options).unwrap_or_default().into())
31+
anyhow::Ok(name.demangle(options).unwrap_or_default())
3232
})
3333
}
3434

‎crashtracker/src/crash_info/stacktrace.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ pub enum BuildIdType {
193193
GNU,
194194
GO,
195195
PDB,
196-
PE,
197196
SHA1,
198197
}
199198

@@ -203,7 +202,7 @@ pub enum BuildIdType {
203202
pub enum FileType {
204203
APK,
205204
ELF,
206-
PDB,
205+
PE,
207206
}
208207

209208
#[cfg(unix)]

‎ddcommon-ffi/src/string.rs

+33
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
use crate::slice::CharSlice;
55
use crate::vec::Vec;
6+
use crate::Error;
67

78
/// A wrapper for returning owned strings from FFI
89
#[derive(Debug)]
@@ -72,3 +73,35 @@ pub unsafe extern "C" fn ddog_StringWrapper_message(s: Option<&StringWrapper>) -
7273
Some(s) => CharSlice::from(s.as_ref()),
7374
}
7475
}
76+
77+
#[repr(C)]
78+
#[allow(dead_code)]
79+
pub enum StringWrapperResult {
80+
Ok(StringWrapper),
81+
Err(Error),
82+
}
83+
84+
// Useful for testing
85+
impl StringWrapperResult {
86+
pub fn unwrap(self) -> StringWrapper {
87+
match self {
88+
StringWrapperResult::Ok(s) => s,
89+
StringWrapperResult::Err(e) => panic!("{e}"),
90+
}
91+
}
92+
}
93+
94+
impl From<anyhow::Result<String>> for StringWrapperResult {
95+
fn from(value: anyhow::Result<String>) -> Self {
96+
match value {
97+
Ok(x) => Self::Ok(x.into()),
98+
Err(err) => Self::Err(err.into()),
99+
}
100+
}
101+
}
102+
103+
impl From<String> for StringWrapperResult {
104+
fn from(value: String) -> Self {
105+
Self::Ok(value.into())
106+
}
107+
}

‎ddcommon-ffi/src/utils.rs

+10
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,13 @@ macro_rules! wrap_with_void_ffi_result {
2828
.into()
2929
}};
3030
}
31+
32+
pub trait ToHexStr {
33+
fn to_hex_str(&self) -> String;
34+
}
35+
36+
impl ToHexStr for usize {
37+
fn to_hex_str(&self) -> String {
38+
format!("0x{:X}", self)
39+
}
40+
}

0 commit comments

Comments
 (0)