Merge pull request #538 from orecham/iox2-491-fix-bug-in-enum-string-…

…literal-generation

[#491] Ensure enum string literals are null terminated

iceoryx2-bb/derive-macros/src/lib.rs

@@ -15,9 +15,8 @@
 extern crate proc_macro;
 
 use proc_macro::TokenStream;
-use proc_macro2::Literal;
 use quote::quote;
-use syn::{parse_macro_input, Data, DeriveInput, Expr, ExprLit, Fields, Lit};
+use syn::{parse_macro_input, Data, DeriveInput, Fields};
 
 /// Implements the [`iceoryx2_bb_elementary::placement_default::PlacementDefault`] trait when all
 /// fields of the struct implement it.
@@ -99,124 +98,3 @@ pub fn placement_default_derive(input: TokenStream) -> TokenStream {
 
     TokenStream::from(expanded)
 }
-
-/// Implements the [`iceoryx2_bb_elementary::AsStringLiteral`] trait for enums to provide a string representation of each enum variant.
-///
-/// The string representation can be customized using the `CustomString` attribute, otherwise it will
-/// convert the variant name to lowercase and replace underscores with spaces.
-///
-/// # Example
-/// ```
-/// use iceoryx2_bb_derive_macros::StringLiteral;
-/// use iceoryx2_bb_elementary::AsStringLiteral;
-///
-/// #[derive(StringLiteral)]
-/// enum MyEnum {
-///     #[CustomString = "custom variant one"]
-///     VariantOne,
-///     VariantTwo,
-/// }
-///
-/// let v1 = MyEnum::VariantOne;
-/// assert_eq!(v1.as_str_literal(), "custom variant one");
-///
-/// let v2 = MyEnum::VariantTwo;
-/// assert_eq!(v2.as_str_literal(), "variant two");
-/// ```
-#[proc_macro_derive(StringLiteral, attributes(CustomString))]
-pub fn string_literal_derive(input: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(input as DeriveInput);
-    let name = &input.ident;
-    let (impl_generics, type_generics, where_clause) = input.generics.split_for_impl();
-
-    // Generate implementation converting enums to a string representation
-    let as_string_literal_impl = match input.data {
-        Data::Enum(ref data_enum) => {
-            let enum_to_string_mapping = data_enum.variants.iter().map(|variant| {
-                let enum_name = &variant.ident;
-                let enum_string_literal = variant
-                    .attrs
-                    .iter()
-                    .find_map(|attr| {
-                        if !attr.path().is_ident("CustomString") {
-                            return None;
-                        }
-                        // Get the value of CustomString as a string literal
-                        match attr.meta.require_name_value() {
-                            Ok(meta) => match &meta.value {
-                                Expr::Lit(ExprLit {
-                                    lit: Lit::Str(lit), ..
-                                }) => Some(Literal::string(&lit.value())),
-                                _ => None,
-                            },
-                            _ => None,
-                        }
-                    })
-                    .unwrap_or_else(|| {
-                        // If no CustomString, generates default string literal in the form
-                        // MyEnum::MyVariantName => 'my variant name'
-                        let enum_string_literal = enum_name
-                            .to_string()
-                            .chars()
-                            .fold(String::new(), |mut acc, c| {
-                                if c.is_uppercase() && !acc.is_empty() {
-                                    acc.push('_');
-                                }
-                                acc.push(c);
-                                acc
-                            })
-                            .chars()
-                            .map(|c| match c {
-                                '_' => ' ',
-                                c => c.to_ascii_lowercase(),
-                            })
-                            .collect::<String>();
-                        Literal::string(&enum_string_literal)
-                    });
-
-                // Maps each enum variant to its string representation
-                match &variant.fields {
-                    Fields::Unit => {
-                        quote! {
-                            Self::#enum_name => #enum_string_literal
-                        }
-                    }
-                    Fields::Unnamed(_) => {
-                        quote! {
-                            Self::#enum_name(..) => #enum_string_literal
-                        }
-                    }
-                    Fields::Named(_) => {
-                        quote! {
-                            Self::#enum_name{..} => #enum_string_literal
-                        }
-                    }
-                }
-            });
-
-            // Generate the mapping for the enum variant
-            quote! {
-                fn as_str_literal(&self) -> &'static str {
-                    match self {
-                        #(#enum_to_string_mapping,)*
-                    }
-                }
-            }
-        }
-        _ => {
-            // Does not work for non-enum types
-            let err =
-                syn::Error::new_spanned(&input, "AsStringLiteral can only be derived for enums");
-            return err.to_compile_error().into();
-        }
-    };
-
-    // Implement the trait with the generated implementation
-    let expanded = quote! {
-        impl #impl_generics AsStringLiteral for #name #type_generics #where_clause {
-            #as_string_literal_impl
-        }
-    };
-
-    TokenStream::from(expanded)
-}

iceoryx2-bb/elementary/src/as_string_literal.rs → iceoryx2-bb/elementary/src/as_cstr.rs

@@ -10,6 +10,17 @@
 //
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
-pub trait AsStringLiteral {
-    fn as_str_literal(&self) -> &'static str;
+use std::ffi::CStr;
+
+/// Trait for types that can be represented as a C-style string.
+///
+/// This trait provides a method to obtain a reference to a static C-style string
+/// representation of the implementing type.
+///
+/// # Safety
+///
+/// Implementations of this trait must ensure that the returned `CStr` is valid
+/// and remains valid for the entire lifetime of the program.
+pub trait AsCStr {
+    fn as_const_cstr(&self) -> &'static CStr;
 }

iceoryx2-bb/elementary/src/lib.rs

@@ -15,8 +15,8 @@
 #[macro_use]
 pub mod enum_gen;
 
-mod as_string_literal;
-pub use as_string_literal::*;
+mod as_cstr;
+pub use as_cstr::*;
 
 pub mod alignment;
 pub mod allocator;

iceoryx2-bb/testing/src/assert.rs

@@ -197,7 +197,7 @@ macro_rules! assert_that {
                 }
             }
             if does_contain {
-                assert_that!(message_contains_match $lhs, core::stringify!($predicate));
+                assert_that!(message_not_contains_match $lhs, core::stringify!($predicate));
             }
         }
     };
@@ -274,6 +274,15 @@ macro_rules! assert_that {
             assert_that![color_end]
         );
     };
+    [message_not_contains_match $lhs:expr, $predicate:expr] => {
+        core::panic!(
+            "assertion failed: {}expr: {} contains element matching predicate: {}{}",
+            assert_that![color_start],
+            core::stringify!($lhs),
+            $predicate,
+            assert_that![color_end]
+        );
+    };
     [message_property $lhs:expr, $lval:expr, $property:expr, $rhs:expr] => {
         core::panic!(
             "assertion failed: {}expr: {}.{} == {};  value: {} == {}{}",

iceoryx2-ffi/ffi-macros/Cargo.toml

@@ -18,3 +18,4 @@ proc-macro = true
 proc-macro2 = { workspace = true }
 quote = { workspace = true }
 syn = { workspace = true }
+iceoryx2-bb-elementary = { workspace = true }

iceoryx2-ffi/ffi-macros/src/lib.rs

@@ -13,7 +13,10 @@
 use proc_macro::TokenStream;
 use proc_macro2::TokenTree;
 use quote::{format_ident, quote};
-use syn::{parse_macro_input, punctuated::Punctuated, DeriveInput, LitStr, Meta, Token};
+use syn::{
+    parse_macro_input, punctuated::Punctuated, Data, DeriveInput, Expr, ExprLit, Fields, Lit,
+    LitStr, Meta, Token,
+};
 
 #[proc_macro_attribute]
 pub fn iceoryx2_ffi(args: TokenStream, input: TokenStream) -> TokenStream {
@@ -217,3 +220,99 @@ fn parse_attribute_args(args: TokenStream) -> Args {
 
     Args { rust_type }
 }
+
+/// Implements the [`iceoryx2_bb_elementary::AsCStr`] trait for enums to provide a string representation of each enum variant.
+///
+/// The string representation can be customized using the `CStr` attribute, otherwise it will
+/// convert the variant name to lowercase and replace underscores with spaces.
+///
+/// # Example
+/// ```
+/// use iceoryx2_ffi_macros::CStrRepr;
+/// use iceoryx2_bb_elementary::AsCStr;
+///
+/// #[derive(CStrRepr)]
+/// enum MyEnum {
+///     #[CStr = "custom variant one"]
+///     VariantOne,
+///     VariantTwo,
+/// }
+///
+/// let v1 = MyEnum::VariantOne;
+/// assert_eq!(v1.as_const_cstr(), c"custom variant one");
+///
+/// let v2 = MyEnum::VariantTwo;
+/// assert_eq!(v2.as_const_cstr(), c"variant two");
+/// ```
+#[proc_macro_derive(CStrRepr, attributes(CStr))]
+pub fn string_literal_derive(input: TokenStream) -> TokenStream {
+    let input = parse_macro_input!(input as DeriveInput);
+    let name = &input.ident;
+    let (impl_generics, type_generics, where_clause) = input.generics.split_for_impl();
+
+    let as_cstr_impl = match input.data {
+        Data::Enum(ref data_enum) => {
+            let enum_to_string_mapping = data_enum.variants.iter().map(|variant| {
+                let null_terminated = |s: &str| {
+                    quote! {
+                        // This code appends the null termination which cannot be confirmed at compile time,
+                        // thus the code is ensured safe.
+                        unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(concat!(#s, "\0").as_bytes()) }
+                    }
+                };
+
+                let enum_name = &variant.ident;
+                let cstr_literal = variant
+                    .attrs
+                    .iter()
+                    .find(|attr| attr.path().is_ident("CStr"))
+                    .and_then(|attr| match attr.meta.require_name_value() {
+                        Ok(meta) => match &meta.value {
+                            Expr::Lit(ExprLit { lit: Lit::Str(lit), .. }) => Some(null_terminated(&lit.value())),
+                            _ => None,
+                        },
+                        _ => None,
+                    })
+                    .unwrap_or_else(|| {
+                        // No explicity `CStr` is provided.
+                        // Convert variant name from 'UpperCamelCase' to 'lowercase with spaces'.
+                        let enum_string = enum_name.to_string()
+                            .char_indices()
+                            .fold(String::new(), |mut acc, (i, c)| {
+                                if i > 0 && c.is_uppercase() {
+                                    acc.push(' ');
+                                }
+                                acc.push(c.to_ascii_lowercase());
+                                acc
+                            });
+                        null_terminated(&enum_string)
+                    });
+
+                let pattern = match &variant.fields {
+                    Fields::Unit => quote!(Self::#enum_name),
+                    Fields::Unnamed(_) => quote!(Self::#enum_name(..)),
+                    Fields::Named(_) => quote!(Self::#enum_name{..}),
+                };
+                quote! { #pattern => #cstr_literal }
+            });
+
+            quote! {
+                fn as_const_cstr(&self) -> &'static ::std::ffi::CStr {
+                    match self {
+                        #(#enum_to_string_mapping,)*
+                    }
+                }
+            }
+        }
+        _ => {
+            let err = syn::Error::new_spanned(&input, "AsCStrRepr can only be derived for enums");
+            return err.to_compile_error().into();
+        }
+    };
+
+    TokenStream::from(quote! {
+        impl #impl_generics AsCStr for #name #type_generics #where_clause {
+            #as_cstr_impl
+        }
+    })
+}

iceoryx2-ffi/ffi/src/api/config.rs

@@ -17,13 +17,13 @@ use core::ffi::{c_char, c_int};
 use core::time::Duration;
 use iceoryx2::config::{Config, ConfigCreationError};
 use iceoryx2_bb_container::semantic_string::*;
-use iceoryx2_bb_derive_macros::StringLiteral;
 use iceoryx2_bb_elementary::static_assert::*;
-use iceoryx2_bb_elementary::AsStringLiteral;
+use iceoryx2_bb_elementary::AsCStr;
 use iceoryx2_bb_system_types::file_name::FileName;
 use iceoryx2_bb_system_types::file_path::FilePath;
 use iceoryx2_bb_system_types::path::Path;
 use iceoryx2_ffi_macros::iceoryx2_ffi;
+use iceoryx2_ffi_macros::CStrRepr;
 use std::mem::ManuallyDrop;
 
 use crate::IOX2_OK;
@@ -34,7 +34,7 @@ use super::{HandleToType, IntoCInt};
 
 /// Failures occurring while creating a new [`iox2_config_t`] object with [`iox2_config_from_file()`].
 #[repr(C)]
-#[derive(Copy, Clone, StringLiteral)]
+#[derive(Copy, Clone, CStrRepr)]
 pub enum iox2_config_creation_error_e {
     /// The config file could not be opened.
     FAILED_TO_OPEN_CONFIG_FILE = IOX2_OK as isize + 1,
@@ -154,7 +154,7 @@ impl HandleToType for iox2_config_h_ref {
 pub unsafe extern "C" fn iox2_config_creation_error_string(
     error: iox2_config_creation_error_e,
 ) -> *const c_char {
-    error.as_str_literal().as_ptr() as *const c_char
+    error.as_const_cstr().as_ptr() as *const c_char
 }
 
 /// This function casts a [`iox2_config_h`] into a [`iox2_config_ptr`]

iceoryx2-ffi/ffi/src/api/listener.rs

@@ -20,12 +20,12 @@ use crate::iox2_file_descriptor_ptr;
 
 use iceoryx2::port::listener::Listener;
 use iceoryx2::prelude::*;
-use iceoryx2_bb_derive_macros::StringLiteral;
 use iceoryx2_bb_elementary::static_assert::*;
-use iceoryx2_bb_elementary::AsStringLiteral;
+use iceoryx2_bb_elementary::AsCStr;
 use iceoryx2_bb_posix::file_descriptor::{FileDescriptor, FileDescriptorBased};
 use iceoryx2_cal::event::ListenerWaitError;
 use iceoryx2_ffi_macros::iceoryx2_ffi;
+use iceoryx2_ffi_macros::CStrRepr;
 
 use core::ffi::{c_char, c_int};
 use core::mem::ManuallyDrop;
@@ -34,7 +34,7 @@ use core::time::Duration;
 // BEGIN types definition
 
 #[repr(C)]
-#[derive(Copy, Clone, StringLiteral)]
+#[derive(Copy, Clone, CStrRepr)]
 pub enum iox2_listener_wait_error_e {
     CONTRACT_VIOLATION = IOX2_OK as isize + 1,
     INTERNAL_FAILURE,
@@ -158,7 +158,7 @@ pub type iox2_listener_wait_all_callback =
 pub unsafe extern "C" fn iox2_listener_wait_error_string(
     error: iox2_listener_wait_error_e,
 ) -> *const c_char {
-    error.as_str_literal().as_ptr() as *const c_char
+    error.as_const_cstr().as_ptr() as *const c_char
 }
 
 /// This function needs to be called to destroy the listener!