Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#61] fix undefined behavior in string #62

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/release-notes/iceoryx2-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

### Bugfixes

* Fix undefined behavior in `FixedSizeByteString::new_unchecked` [#61](https://github.com/eclipse-iceoryx/iceoryx2/issues/61)
* Fix suffix of static config [#66](https://github.com/eclipse-iceoryx/iceoryx2/issues/66)

### Refactoring
Expand Down
15 changes: 2 additions & 13 deletions iceoryx2-bb/container/src/byte_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,23 +207,12 @@ impl<const CAPACITY: usize> FixedSizeByteString<CAPACITY> {
///
/// * `bytes` len must be smaller or equal than [`FixedSizeByteString::capacity()`]
///
pub const unsafe fn new_unchecked(bytes: &[u8]) -> Self {
pub unsafe fn new_unchecked(bytes: &[u8]) -> Self {
if CAPACITY < bytes.len() {
panic!("Insufficient capacity to store bytes.");
}

let mut new_self = Self::new();
new_self.len = bytes.len();
std::ptr::copy(
bytes.as_ptr(),
new_self.data.as_ptr() as *mut u8,
bytes.len(),
);

let zero = 0u8;
std::ptr::copy(&zero, new_self.data.as_ptr().add(bytes.len()) as *mut u8, 1);

new_self
Self::from_bytes_truncated(bytes)
}

/// Creates a new [`FixedSizeByteString`] from a byte slice
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/container/src/semantic_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ macro_rules! semantic_string {
}

impl $string_name {
pub const unsafe fn new_unchecked(bytes: &[u8]) -> Self {
pub unsafe fn new_unchecked(bytes: &[u8]) -> Self {
Self {
value: iceoryx2_bb_container::byte_string::FixedSizeByteString::new_unchecked(bytes),
}
Expand Down
17 changes: 11 additions & 6 deletions iceoryx2-bb/posix/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ pub const ADAPTIVE_WAIT_INITIAL_WAITING_TIME: Duration = Duration::from_micros(1
pub const ADAPTIVE_WAIT_FINAL_WAITING_TIME: Duration = Duration::from_millis(10);

// directories
pub const TEMP_DIRECTORY: Path =
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEMP_DIRECTORY) };
pub const TEST_DIRECTORY: Path =
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEST_DIRECTORY) };
pub const SHARED_MEMORY_DIRECTORY: Path =
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::SHARED_MEMORY_DIRECTORY) };
pub fn temp_directory() -> Path {
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEMP_DIRECTORY) }
}

pub fn test_directory() -> Path {
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::TEST_DIRECTORY) }
}

pub fn shared_memory_directory() -> Path {
unsafe { Path::new_unchecked(iceoryx2_pal_configuration::SHARED_MEMORY_DIRECTORY) }
}

// TODO unable to verify?
pub const ACL_LIST_CAPACITY: u32 = 25;
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/directory_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl TestFixture {
}

fn generate_directory_name(&mut self) -> Path {
let mut directory = TEST_DIRECTORY;
let mut directory = test_directory();
directory.push(PATH_SEPARATOR).unwrap();
directory.push_bytes(b"dir_tests_").unwrap();
directory
Expand All @@ -107,7 +107,7 @@ impl TestFixture {

#[test]
fn directory_temp_directory_does_exist() {
assert_that!(Directory::does_exist(&TEST_DIRECTORY).unwrap(), eq true);
assert_that!(Directory::does_exist(&test_directory()).unwrap(), eq true);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/posix/tests/file_descriptor_set_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn generate_socket_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/posix/tests/file_descriptor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ trait GenericTestBuilder {

impl GenericTestBuilder for File {
fn sut() -> Self {
let name = FilePath::from_path_and_file(&TEST_DIRECTORY, &generate_name()).unwrap();
let name = FilePath::from_path_and_file(&test_directory(), &generate_name()).unwrap();

let file_content = [170u8; 2048];

Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/file_lock_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

use iceoryx2_bb_container::semantic_string::SemanticString;
use iceoryx2_bb_posix::config::*;
use iceoryx2_bb_posix::config;
use iceoryx2_bb_posix::file::*;
use iceoryx2_bb_posix::file_lock::*;
use iceoryx2_bb_posix::process::*;
Expand All @@ -38,7 +38,7 @@ fn generate_file_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&config::test_directory(), &file).unwrap()
}

const TIMEOUT: Duration = Duration::from_millis(10);
Expand Down
2 changes: 1 addition & 1 deletion iceoryx2-bb/posix/tests/file_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn generate_file_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

struct TestFixture {
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/metadata_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use iceoryx2_pal_posix::posix::POSIX_SUPPORT_USERS_AND_GROUPS;
#[test]
fn metadata_reads_basic_stats_correctly() {
let file_name =
FilePath::from_path_and_file(&TEST_DIRECTORY, &FileName::new(b"meta_test").unwrap())
FilePath::from_path_and_file(&test_directory(), &FileName::new(b"meta_test").unwrap())
.unwrap();

let mut file = FileBuilder::new(&file_name)
Expand All @@ -50,7 +50,7 @@ fn metadata_reads_owner_and_permission_stats_correctly() {
test_requires!(POSIX_SUPPORT_USERS_AND_GROUPS && POSIX_SUPPORT_PERMISSIONS);

let file_name =
FilePath::from_path_and_file(&TEST_DIRECTORY, &FileName::new(b"meta_test_123").unwrap())
FilePath::from_path_and_file(&test_directory(), &FileName::new(b"meta_test_123").unwrap())
.unwrap();

let mut file = FileBuilder::new(&file_name)
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/socket_ancillary_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use iceoryx2_bb_container::semantic_string::SemanticString;
use iceoryx2_bb_elementary::unique_id::*;
use iceoryx2_bb_posix::config::*;
use iceoryx2_bb_posix::config;
use iceoryx2_bb_posix::file::*;
use iceoryx2_bb_posix::file_descriptor::*;
use iceoryx2_bb_posix::process::ProcessId;
Expand All @@ -29,7 +29,7 @@ fn generate_file_name() -> FilePath {
file.push_bytes(UniqueId::new().value().to_string().as_bytes())
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&config::test_directory(), &file).unwrap()
}

struct TestFixture {
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-bb/posix/tests/unix_datagram_socket_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn generate_socket_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

fn generate_file_name() -> FilePath {
Expand All @@ -57,7 +57,7 @@ fn generate_file_name() -> FilePath {
)
.unwrap();

FilePath::from_path_and_file(&TEST_DIRECTORY, &file).unwrap()
FilePath::from_path_and_file(&test_directory(), &file).unwrap()
}

struct TestFixture {
Expand Down
10 changes: 5 additions & 5 deletions iceoryx2-cal/src/communication_channel/message_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path_hint: DEFAULT_PATH_HINT,
suffix: Channel::<()>::default_suffix(),
prefix: Channel::<()>::default_prefix(),
path_hint: Channel::<()>::default_path_hint(),
}
}
}
Expand Down Expand Up @@ -190,7 +190,7 @@ struct SharedConfiguration {
}

#[derive(Debug)]
pub struct Creator<T> {
pub struct Creator<T: Copy + Debug> {
channel_name: FileName,
enable_safe_overflow: bool,
buffer_size: usize,
Expand Down Expand Up @@ -291,7 +291,7 @@ impl<T: Copy + Debug> CommunicationChannelCreator<T, Channel<T>> for Creator<T>
}

#[derive(Debug)]
pub struct Connector<T> {
pub struct Connector<T: Copy + Debug> {
channel_name: FileName,
config: Configuration,
_phantom_data: PhantomData<T>,
Expand Down
15 changes: 5 additions & 10 deletions iceoryx2-cal/src/communication_channel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ pub mod unix_datagram;

use std::fmt::Debug;

use iceoryx2_bb_posix::config::TEMP_DIRECTORY;
use iceoryx2_bb_system_types::file_name::FileName;
use iceoryx2_bb_system_types::path::Path;

Expand All @@ -81,15 +80,6 @@ use crate::named_concept::{NamedConcept, NamedConceptBuilder, NamedConceptMgmt};
/// The buffer size which the receiver has at least by default
pub const DEFAULT_RECEIVER_BUFFER_SIZE: usize = 8;

/// The default suffix of every communication channel
pub const DEFAULT_SUFFIX: FileName = unsafe { FileName::new_unchecked(b".com") };

/// The default prefix of every communication channel
pub const DEFAULT_PREFIX: FileName = unsafe { FileName::new_unchecked(b"iox2_") };

/// The default path hint for every communication channel
pub const DEFAULT_PATH_HINT: Path = TEMP_DIRECTORY;

/// Describes failures when sending data
#[derive(Debug, Clone, Copy, Eq, Hash, PartialEq)]
pub enum CommunicationChannelSendError {
Expand Down Expand Up @@ -207,4 +197,9 @@ pub trait CommunicationChannel<T>: Sized + Debug + NamedConceptMgmt {
fn has_configurable_buffer_size() -> bool {
false
}

/// The default suffix of every communication channel
fn default_suffix() -> FileName {
unsafe { FileName::new_unchecked(b".com") }
}
}
6 changes: 3 additions & 3 deletions iceoryx2-cal/src/communication_channel/posix_shared_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
path_hint: DEFAULT_PATH_HINT,
prefix: DEFAULT_PREFIX,
suffix: Channel::default_suffix(),
path_hint: Channel::default_path_hint(),
prefix: Channel::default_prefix(),
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions iceoryx2-cal/src/communication_channel/process_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path_hint: DEFAULT_PATH_HINT,
suffix: Channel::default_suffix(),
prefix: Channel::default_prefix(),
path_hint: Channel::default_path_hint(),
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions iceoryx2-cal/src/communication_channel/unix_datagram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ pub struct Configuration {
impl Default for Configuration {
fn default() -> Self {
Self {
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path_hint: DEFAULT_PATH_HINT,
suffix: Channel::<()>::default_suffix(),
prefix: Channel::<()>::default_prefix(),
path_hint: Channel::<()>::default_path_hint(),
}
}
}
Expand Down Expand Up @@ -165,7 +165,7 @@ impl<T: Copy + Debug> CommunicationChannel<T> for Channel<T> {
}

#[derive(Debug)]
pub struct Creator<T> {
pub struct Creator<T: Copy + Debug> {
channel_name: FileName,
enable_safe_overflow: bool,
buffer_size: usize,
Expand Down Expand Up @@ -240,7 +240,7 @@ impl<T: Copy + Debug> CommunicationChannelCreator<T, Channel<T>> for Creator<T>
}

#[derive(Debug)]
pub struct Connector<T> {
pub struct Connector<T: Copy + Debug> {
channel_name: FileName,
config: Configuration,
_phantom_data: PhantomData<T>,
Expand Down
16 changes: 5 additions & 11 deletions iceoryx2-cal/src/dynamic_storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,13 @@
use std::fmt::Debug;

use iceoryx2_bb_memory::bump_allocator::BumpAllocator;
use iceoryx2_bb_posix::config::TEMP_DIRECTORY;
use iceoryx2_bb_system_types::file_name::FileName;
use iceoryx2_bb_system_types::path::Path;

use crate::static_storage::file::{NamedConcept, NamedConceptBuilder, NamedConceptMgmt};

pub mod posix_shared_memory;
pub mod process_local;

/// The default suffix of every dynamic storage
pub const DEFAULT_SUFFIX: FileName = unsafe { FileName::new_unchecked(b".dyn") };

/// The default prefix of every dynamic storage
pub const DEFAULT_PREFIX: FileName = unsafe { FileName::new_unchecked(b"iox2_") };

/// The default path hint for every dynamic storage
pub const DEFAULT_PATH_HINT: Path = TEMP_DIRECTORY;

/// Describes failures when creating a new [`DynamicStorage`]
#[derive(Debug, Clone, Copy, Eq, Hash, PartialEq)]
pub enum DynamicStorageCreateError {
Expand Down Expand Up @@ -155,4 +144,9 @@ pub trait DynamicStorage<T: Send + Sync>: Sized + Debug + NamedConceptMgmt + Nam
/// can be accessed by multiple processes concurrently therefore it must be constant or
/// thread-safe.
fn get(&self) -> &T;

/// The default suffix of every dynamic storage
fn default_suffix() -> FileName {
unsafe { FileName::new_unchecked(b".dyn") }
}
}
10 changes: 5 additions & 5 deletions iceoryx2-cal/src/dynamic_storage/posix_shared_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ const IS_INITIALIZED_STATE_VALUE: u64 = 0xbeefaffedeadbeef;

/// The builder of [`Storage`].
#[derive(Debug)]
pub struct Builder<T: Debug> {
pub struct Builder<T: Send + Sync + Debug> {
storage_name: FileName,
supplementary_size: usize,
has_ownership: bool,
config: Configuration,
_phantom_data: PhantomData<T>,
}

#[derive(Clone, Debug)]
#[derive(Debug, Clone)]
pub struct Configuration {
suffix: FileName,
prefix: FileName,
Expand All @@ -83,9 +83,9 @@ struct Data<T: Send + Sync + Debug> {
impl Default for Configuration {
fn default() -> Self {
Self {
path: DEFAULT_PATH_HINT,
suffix: DEFAULT_SUFFIX,
prefix: DEFAULT_PREFIX,
path: Storage::<()>::default_path_hint(),
suffix: Storage::<()>::default_suffix(),
prefix: Storage::<()>::default_prefix(),
}
}
}
Expand Down
Loading