diff --git a/ddprof-ffi/src/exporter.rs b/ddprof-ffi/src/exporter.rs index f6df498..e4ec85f 100644 --- a/ddprof-ffi/src/exporter.rs +++ b/ddprof-ffi/src/exporter.rs @@ -1,25 +1,29 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. -use crate::{Buffer, ByteSlice, CharSlice, Slice, Timespec}; +#![allow(renamed_and_removed_lints)] +#![allow(clippy::box_vec)] + +use crate::{ByteSlice, CharSlice, Slice, Timespec}; use ddprof_exporter as exporter; use exporter::ProfileExporterV3; use std::borrow::Cow; use std::convert::TryInto; -use std::io::Write; +use std::error::Error; +use std::fmt::{Debug, Display, Formatter}; use std::ptr::NonNull; use std::str::FromStr; #[repr(C)] pub enum SendResult { HttpResponse(HttpStatus), - Failure(Buffer), + Failure(crate::Vec), } #[repr(C)] pub enum NewProfileExporterV3Result { Ok(*mut ProfileExporterV3), - Err(Buffer), + Err(crate::Vec), } #[export_name = "ddprof_ffi_NewProfileExporterV3Result_dtor"] @@ -35,17 +39,6 @@ pub unsafe extern "C" fn new_profile_exporter_v3_result_dtor(result: NewProfileE } } -/// Clears the contents of the Buffer, leaving length and capacity of 0. -/// # Safety -/// The `buffer` must be created by Rust, or null. -#[export_name = "ddprof_ffi_Buffer_reset"] -pub unsafe extern "C" fn buffer_reset(buffer: *mut Buffer) { - match buffer.as_mut() { - None => {} - Some(buff) => buff.reset(), - } -} - #[repr(C)] #[derive(Copy, Clone)] pub struct Tag<'a> { @@ -162,16 +155,6 @@ fn try_to_endpoint( } } -fn error_into_buffer(err: Box) -> Buffer { - let mut vec = Vec::new(); - /* Ignore the possible but highly unlikely write failure into a - * Vec. In case this happens, it will be an empty message, which - * will be confusing but safe, and I'm not sure how else to handle - * it. */ - let _ = write!(vec, "{}", err); - Buffer::from_vec(vec) -} - #[export_name = "ddprof_ffi_ProfileExporterV3_new"] pub extern "C" fn profile_exporter_new( family: CharSlice, @@ -185,7 +168,7 @@ pub extern "C" fn profile_exporter_new( ProfileExporterV3::new(converted_family, converted_tags, converted_endpoint) }() { Ok(exporter) => NewProfileExporterV3Result::Ok(Box::into_raw(Box::new(exporter))), - Err(err) => NewProfileExporterV3Result::Err(error_into_buffer(err)), + Err(err) => NewProfileExporterV3Result::Err(err.into()), } } @@ -256,7 +239,7 @@ pub unsafe extern "C" fn profile_exporter_send( let exp_ptr = match exporter { None => { let buf: &[u8] = b"Failed to export: exporter was null"; - return SendResult::Failure(Buffer::from_vec(Vec::from(buf))); + return SendResult::Failure(crate::Vec::from(Vec::from(buf))); } Some(e) => e, }; @@ -264,7 +247,7 @@ pub unsafe extern "C" fn profile_exporter_send( let request_ptr = match request { None => { let buf: &[u8] = b"Failed to export: request was null"; - return SendResult::Failure(Buffer::from_vec(Vec::from(buf))); + return SendResult::Failure(crate::Vec::from(Vec::from(buf))); } Some(req) => req, }; @@ -275,8 +258,135 @@ pub unsafe extern "C" fn profile_exporter_send( Ok(HttpStatus(response.status().as_u16())) }() { Ok(code) => SendResult::HttpResponse(code), - Err(err) => SendResult::Failure(error_into_buffer(err)), + Err(err) => SendResult::Failure(err.into()), + } +} + +#[export_name = "ddprof_ffi_SendResult_drop"] +pub unsafe extern "C" fn send_result_drop(result: SendResult) { + std::mem::drop(result) +} + +#[export_name = "ddprof_ffi_Vec_tag_new"] +pub extern "C" fn vec_tag_new<'a>() -> crate::Vec> { + crate::Vec::default() +} + +/// Pushes the tag into the vec. +#[export_name = "ddprof_ffi_Vec_tag_push"] +pub unsafe extern "C" fn vec_tag_push<'a>(vec: &mut crate::Vec>, tag: Tag<'a>) { + vec.push(tag) +} + +#[allow(clippy::ptr_arg)] +#[export_name = "ddprof_ffi_Vec_tag_as_slice"] +pub extern "C" fn vec_tag_as_slice<'a>(vec: &'a crate::Vec>) -> Slice<'a, Tag<'a>> { + vec.as_slice() +} + +#[export_name = "ddprof_ffi_Vec_tag_drop"] +pub extern "C" fn vec_tag_drop(vec: crate::Vec) { + std::mem::drop(vec) +} + +struct ParseTagsError { + message: &'static str, +} + +impl Debug for ParseTagsError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Tag Error: {}", self.message) + } +} + +impl Display for ParseTagsError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "The tags string was malformed.") + } +} + +impl Error for ParseTagsError {} + +fn parse_tag_chunk(chunk: &str) -> Result { + if let Some(first_colon_position) = chunk.find(':') { + if first_colon_position == 0 { + return Err(ParseTagsError { + message: "Tag cannot start with a colon.", + }); + } + let name = &chunk[..first_colon_position]; + let value = &chunk[(first_colon_position + 1)..]; + Ok(Tag { + name: CharSlice::from(name), + value: CharSlice::from(value), + }) + } else { + Err(ParseTagsError { + message: "Unable to find a colon in the tag.", + }) + } +} + +/// Parse a string of tags typically provided by environment variables +/// The tags are expected to be either space or comma separated: +/// "key1:value1,key2:value2" +/// "key1:value1 key2:value2" +/// Tag names and values are required and may not be empty. +fn parse_tags(str: &str) -> Result, ParseTagsError> { + /* The choice between spaces and comma as the separator is made by splitting + * by both and seeing which way matches the number of colons in the string. + * + * This is also done in a way to avoid allocations. + */ + let colon_count = str.matches(':').count(); + let comma_separated = str.split(',').map(parse_tag_chunk).filter(Result::is_ok); + let space_separated = str.split(' ').map(parse_tag_chunk).filter(Result::is_ok); + + let seq = if comma_separated.clone().count() == colon_count { + comma_separated + } else if space_separated.clone().count() == colon_count { + space_separated + } else { + return Err(ParseTagsError { + message: "Unable to identify the tag separator; is the tags string malformed?", + }); + }; + + let vec: Vec<_> = seq.flat_map(Result::ok).collect(); + Ok(crate::Vec::from(vec)) +} + +#[repr(C)] +pub enum VecTagResult<'a> { + Ok(crate::Vec>), + Err(crate::Vec), +} + +#[export_name = "ddprof_ffi_VecTagResult_drop"] +pub extern "C" fn vec_tag_result_drop(result: VecTagResult) { + std::mem::drop(result) +} + +#[export_name = "ddprof_ffi_Vec_tag_parse"] +pub extern "C" fn vec_tag_parse(string: CharSlice) -> VecTagResult { + match string.try_into() { + Ok(str) => match parse_tags(str) { + Ok(vec) => VecTagResult::Ok(vec), + Err(err) => VecTagResult::Err(crate::Vec::from(&err as &dyn Error)), + }, + + Err(err) => VecTagResult::Err(crate::Vec::from(&err as &dyn Error)), + } +} + +#[allow(clippy::ptr_arg)] +#[export_name = "ddprof_ffi_Vec_tag_clone"] +pub extern "C" fn vec_tag_clone<'a>(vec: &'a crate::Vec>) -> VecTagResult { + let mut clone = Vec::new(); + for tag in vec.into_iter() { + clone.push(tag.to_owned()) } + VecTagResult::Ok(crate::Vec::from(clone)) } #[cfg(test)] diff --git a/ddprof-ffi/src/lib.rs b/ddprof-ffi/src/lib.rs index 6d3224b..ee3ceb3 100644 --- a/ddprof-ffi/src/lib.rs +++ b/ddprof-ffi/src/lib.rs @@ -13,6 +13,9 @@ use libc::size_t; mod exporter; mod profiles; +mod vec; + +pub use vec::*; /// Represents time since the Unix Epoch in seconds plus nanoseconds. #[repr(C)] @@ -43,59 +46,6 @@ impl TryFrom for Timespec { } } -/// Buffer holds the raw parts of a Rust Vec; it should only be created from -/// Rust, never from C. -#[repr(C)] -pub struct Buffer { - ptr: *const u8, - len: size_t, - capacity: size_t, -} - -impl Buffer { - pub fn from_vec(vec: Vec) -> Self { - let buffer = Self { - ptr: vec.as_ptr(), - len: vec.len(), - capacity: vec.capacity(), - }; - std::mem::forget(vec); - buffer - } - - /// # Safety - /// This operation is only safe if the buffer was created from using one of - /// the associated methods on `Buffer`. - pub unsafe fn as_slice(&self) -> &[u8] { - std::slice::from_raw_parts(self.ptr, self.len) - } - - /// # Safety - /// This operation is only safe if the buffer was created from using one of - /// the associated methods on `Buffer`. - pub unsafe fn into_vec(self) -> Vec { - let ptr = self.ptr as *mut u8; - let vec = Vec::from_raw_parts(ptr, self.len, self.capacity); - std::mem::forget(self); - vec - } - - /// # Safety - /// This operation is only safe if the buffer was created from using one of - /// the associated methods on `Buffer`. - pub unsafe fn reset(&mut self) { - *self = Self::from_vec(Vec::new()); - } -} - -impl Drop for Buffer { - fn drop(&mut self) { - let vec: Vec = - unsafe { Vec::from_raw_parts(self.ptr as *mut u8, self.len, self.capacity) }; - std::mem::drop(vec) - } -} - #[repr(C)] #[derive(Copy, Clone)] pub struct Slice<'a, T> { @@ -172,8 +122,8 @@ impl<'a, T> From<&'a [T]> for Slice<'a, T> { } } -impl<'a, T> From<&Vec> for Slice<'a, T> { - fn from(value: &Vec) -> Self { +impl<'a, T> From<&std::vec::Vec> for Slice<'a, T> { + fn from(value: &std::vec::Vec) -> Self { let ptr = value.as_ptr(); let len = value.len(); Slice::new(ptr, len) diff --git a/ddprof-ffi/src/profiles.rs b/ddprof-ffi/src/profiles.rs index 0bb677f..7e32a89 100644 --- a/ddprof-ffi/src/profiles.rs +++ b/ddprof-ffi/src/profiles.rs @@ -1,7 +1,7 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. -use crate::{Buffer, CharSlice, Slice, Timespec}; +use crate::{CharSlice, Slice, Timespec}; use ddprof_profiles as profiles; use std::convert::{TryFrom, TryInto}; use std::str::Utf8Error; @@ -326,7 +326,7 @@ pub extern "C" fn ddprof_ffi_Profile_add( pub struct EncodedProfile { start: Timespec, end: Timespec, - buffer: Buffer, + buffer: crate::Vec, } impl TryFrom for EncodedProfile { @@ -335,7 +335,7 @@ impl TryFrom for EncodedProfile { fn try_from(value: ddprof_profiles::EncodedProfile) -> Result { let start = value.start.try_into()?; let end = value.end.try_into()?; - let buffer = Buffer::from_vec(value.buffer); + let buffer = value.buffer.into(); Ok(Self { start, end, buffer }) } } @@ -379,14 +379,6 @@ pub extern "C" fn ddprof_ffi_Profile_reset(profile: &mut ddprof_profiles::Profil profile.reset().is_some() } -#[no_mangle] -/// # Safety -/// Only pass buffers which were created by ddprof routines; do not create one -/// in C and then pass it in. Only call this once per buffer. -pub unsafe extern "C" fn ddprof_ffi_Buffer_free(buffer: Box) { - std::mem::drop(buffer) -} - #[cfg(test)] mod test { use crate::profiles::*; diff --git a/ddprof-ffi/src/vec.rs b/ddprof-ffi/src/vec.rs new file mode 100644 index 0000000..23dfeec --- /dev/null +++ b/ddprof-ffi/src/vec.rs @@ -0,0 +1,150 @@ +extern crate alloc; + +use crate::Slice; +use std::io::Write; +use std::marker::PhantomData; +use std::mem::ManuallyDrop; + +/// Holds the raw parts of a Rust Vec; it should only be created from Rust, +/// never from C. +/// The names ptr and len were chosen to minimize conversion from a previous +/// Buffer type which this has replaced to become more general. +#[repr(C)] +pub struct Vec { + ptr: *const T, + len: usize, + capacity: usize, + _marker: PhantomData, +} + +impl Drop for Vec { + fn drop(&mut self) { + let vec = + unsafe { alloc::vec::Vec::from_raw_parts(self.ptr as *mut T, self.len, self.capacity) }; + std::mem::drop(vec) + } +} + +impl From> for alloc::vec::Vec { + fn from(vec: Vec) -> Self { + let v = ManuallyDrop::new(vec); + unsafe { alloc::vec::Vec::from_raw_parts(v.ptr as *mut T, v.len, v.capacity) } + } +} + +impl From> for Vec { + fn from(vec: alloc::vec::Vec) -> Self { + let mut v = ManuallyDrop::new(vec); + Self { + ptr: v.as_mut_ptr(), + len: v.len(), + capacity: v.capacity(), + _marker: PhantomData::default(), + } + } +} + +impl From<&dyn std::error::Error> for Vec { + fn from(err: &dyn std::error::Error) -> Self { + let mut vec = vec![]; + write!(vec, "{}", err).expect("write to vec to always succeed"); + Self::from(vec) + } +} + +impl From> for Vec { + fn from(err: Box) -> Self { + Self::from(&*err) + } +} + +impl<'a, T> IntoIterator for &'a Vec { + type Item = &'a T; + type IntoIter = core::slice::Iter<'a, T>; + + fn into_iter(self) -> Self::IntoIter { + unsafe { self.as_slice().into_slice() }.iter() + } +} + +impl Vec { + fn replace(&mut self, mut vec: ManuallyDrop>) { + self.ptr = vec.as_mut_ptr(); + self.len = vec.len(); + self.capacity = vec.capacity(); + } + + pub fn push(&mut self, value: T) { + // todo: I'm never sure when to propagate unsafe upwards + let mut vec = ManuallyDrop::new(unsafe { + alloc::vec::Vec::from_raw_parts(self.ptr as *mut T, self.len, self.capacity) + }); + vec.push(value); + self.replace(vec); + } + + pub fn len(&self) -> usize { + self.len + } + + pub fn is_empty(&self) -> bool { + self.len > 0 + } + + pub fn as_slice(&self) -> Slice { + Slice::new(self.ptr, self.len) + } +} + +impl Default for Vec { + fn default() -> Self { + Self::from(alloc::vec::Vec::new()) + } +} + +#[cfg(test)] +mod test { + use crate::vec::*; + + #[test] + fn test_default() { + let vec: Vec = Vec::default(); + assert_eq!(vec.len, 0); + assert_eq!(vec.capacity, 0); + } + + #[test] + fn test_from() { + let vec = vec![0]; + + let mut ffi_vec: Vec = Vec::from(vec); + ffi_vec.push(1); + assert_eq!(ffi_vec.len(), 2); + assert!(ffi_vec.capacity >= 2); + } + + #[test] + fn test_as_slice() { + let mut ffi_vec: Vec = Vec::default(); + ffi_vec.push(1); + ffi_vec.push(2); + assert_eq!(ffi_vec.len(), 2); + assert!(ffi_vec.capacity >= 2); + + let slice = ffi_vec.as_slice(); + let first = unsafe { *(slice.ptr) }; + let second = unsafe { *(slice.ptr).add(1) }; + assert_eq!(first, 1); + assert_eq!(second, 2); + } + + #[test] + fn test_iter() { + let vec = vec![0, 2, 4, 6]; + let ffi_vec: Vec = Vec::from(vec.clone()); + + for (a, b) in vec.iter().zip(ffi_vec.into_iter()) { + assert_eq!(a, b) + } + } +}