Add Vec type, remove Buffer, add SendResult_drop

The Vec type is a generalization of Buffer. In particular, this PR
exposes an API to create and manage Vec<Tag> and replaces existing uses
of Buffer with Vec<u8>. It includes a helper to parse DD_TAGS as well.

One of the APIs, ddprof_ffi_Buffer_reset, was removed. Previously this
was required when using the failure case of the SendResult type, but
now there is ddprof_ffi_SendResult_drop, which should be used instead.

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<u8>),
 }
 
 #[repr(C)]
 pub enum NewProfileExporterV3Result {
     Ok(*mut ProfileExporterV3),
-    Err(Buffer),
+    Err(crate::Vec<u8>),
 }
 
 #[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<dyn std::error::Error>) -> 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,15 +239,15 @@ 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,
     };
 
     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<Tag<'a>> {
+    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<'a>>, 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<Tag<'a>>) -> Slice<'a, Tag<'a>> {
+    vec.as_slice()
+}
+
+#[export_name = "ddprof_ffi_Vec_tag_drop"]
+pub extern "C" fn vec_tag_drop(vec: crate::Vec<Tag>) {
+    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<Tag, ParseTagsError> {
+    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<crate::Vec<Tag>, 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<Tag<'a>>),
+    Err(crate::Vec<u8>),
+}
+
+#[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<Tag<'a>>) -> 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)]

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<SystemTime> 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<u8>) -> 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<u8> {
-        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<u8> =
-            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<T>> for Slice<'a, T> {
-    fn from(value: &Vec<T>) -> Self {
+impl<'a, T> From<&std::vec::Vec<T>> for Slice<'a, T> {
+    fn from(value: &std::vec::Vec<T>) -> Self {
         let ptr = value.as_ptr();
         let len = value.len();
         Slice::new(ptr, len)

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<u8>,
 }
 
 impl TryFrom<ddprof_profiles::EncodedProfile> for EncodedProfile {
@@ -335,7 +335,7 @@ impl TryFrom<ddprof_profiles::EncodedProfile> for EncodedProfile {
     fn try_from(value: ddprof_profiles::EncodedProfile) -> Result<Self, Self::Error> {
         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<Buffer>) {
-    std::mem::drop(buffer)
-}
-
 #[cfg(test)]
 mod test {
     use crate::profiles::*;