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.

Sign up for GitHub

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

[#210] Fix node and service name C API #405

Merged
merged 2 commits into from
Sep 27, 2024
Merged

[#210] Fix node and service name C API #405

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf19ee1
[#210] Fix node and service name C API
elBoberido Sep 27, 2024
3870c48
[#210] Better naming
elBoberido Sep 27, 2024
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
6 changes: 3 additions & 3 deletions iceoryx2-ffi/cxx/src/node_name.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
namespace iox2 {
auto NodeNameView::to_string() const -> iox::string<IOX2_NODE_NAME_LENGTH> {
size_t len = 0;
const auto* c_ptr = iox2_node_name_as_c_str(m_ptr, &len);
const auto* c_ptr = iox2_node_name_as_str(m_ptr, &len);
return { iox::TruncateToCapacity, c_ptr, len };
}

auto NodeNameView::to_owned() const -> NodeName {
size_t len = 0;
const auto* c_ptr = iox2_node_name_as_c_str(m_ptr, &len);
const auto* c_ptr = iox2_node_name_as_str(m_ptr, &len);
return NodeName::create_impl(c_ptr, len).expect("NodeNameView contains always valid NodeName");
}

Expand Down Expand Up @@ -68,7 +68,7 @@ auto NodeName::operator=(const NodeName& rhs) -> NodeName& {

const auto* ptr = iox2_cast_node_name_ptr(rhs.m_handle);
size_t len = 0;
const auto* c_ptr = iox2_node_name_as_c_str(ptr, &len);
const auto* c_ptr = iox2_node_name_as_str(ptr, &len);
IOX_ASSERT(iox2_node_name_new(nullptr, c_ptr, len, &m_handle) == IOX2_OK,
"NodeName shall always contain a valid value.");
}
Expand Down
6 changes: 3 additions & 3 deletions iceoryx2-ffi/cxx/src/service_name.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
namespace iox2 {
auto ServiceNameView::to_string() const -> iox::string<IOX2_NODE_NAME_LENGTH> {
size_t len = 0;
const auto* c_ptr = iox2_service_name_as_c_str(m_ptr, &len);
const auto* c_ptr = iox2_service_name_as_str(m_ptr, &len);
return { iox::TruncateToCapacity, c_ptr, len };
}

auto ServiceNameView::to_owned() const -> ServiceName {
size_t len = 0;
const auto* c_ptr = iox2_service_name_as_c_str(m_ptr, &len);
const auto* c_ptr = iox2_service_name_as_str(m_ptr, &len);
return ServiceName::create_impl(c_ptr, len).expect("ServiceNameView always contains valid ServiceName");
}

Expand Down Expand Up @@ -67,7 +67,7 @@ auto ServiceName::operator=(const ServiceName& rhs) -> ServiceName& {

const auto* ptr = iox2_cast_service_name_ptr(rhs.m_handle);
size_t len = 0;
const auto* c_ptr = iox2_service_name_as_c_str(ptr, &len);
const auto* c_ptr = iox2_service_name_as_str(ptr, &len);
IOX_ASSERT(iox2_service_name_new(nullptr, c_ptr, len, &m_handle) == IOX2_OK,
"ServiceName shall always contain a valid value.");
}
Expand Down
10 changes: 6 additions & 4 deletions iceoryx2-ffi/ffi/src/api/node_name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,24 +158,26 @@ pub unsafe extern "C" fn iox2_cast_node_name_ptr(
(*node_name_handle.as_type()).value.as_ref()
}

/// This function gives access to the node name as a C-style string
/// This function gives access to the node name as a non-zero-terminated char string
///
/// # Arguments
///
/// * `node_name_ptr` obtained by e.g. [`iox2_cast_node_name_ptr`] or a function returning a [`iox2_node_name_ptr`]
/// * `node_name_len` can be used to get the length of the C-style string if not `NULL`
/// * `node_name_len` must be used to get the length of the char string
///
/// Returns zero terminated C-style string
/// Returns a non-zero-terminated char string
///
/// # Safety
///
/// * The `node_name_ptr` must be a valid pointer to a node name.
/// * The `node_name_len` must be a valid pointer to a size_t.
#[no_mangle]
pub unsafe extern "C" fn iox2_node_name_as_c_str(
pub unsafe extern "C" fn iox2_node_name_as_str(
elBoberido marked this conversation as resolved.
Show resolved Hide resolved
node_name_ptr: iox2_node_name_ptr,
node_name_len: *mut c_size_t,
) -> *const c_char {
debug_assert!(!node_name_ptr.is_null());
debug_assert!(!node_name_len.is_null());

let node_name = &*node_name_ptr;

Expand Down
10 changes: 6 additions & 4 deletions iceoryx2-ffi/ffi/src/api/service_name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,24 +158,26 @@ pub unsafe extern "C" fn iox2_cast_service_name_ptr(
(*service_name_handle.as_type()).value.as_ref()
}

/// This function gives access to the node name as a C-style string
/// This function gives access to the node name as a non-zero-terminated char string
///
/// # Arguments
///
/// * `service_name_ptr` obtained by e.g. [`iox2_cast_service_name_ptr`] or a function returning a [`iox2_service_name_ptr`]
/// * `service_name_len` can be used to get the length of the C-style string if not `NULL`
/// * `service_name_len` must be used to get the length of the char string
///
/// Returns zero terminated C-style string
/// Returns non-zero-terminated char string
///
/// # Safety
///
/// * The `service_name_ptr` must be a valid pointer to a node name.
/// * The `service_name_len` must be a valid pointer to a size_t.
#[no_mangle]
pub unsafe extern "C" fn iox2_service_name_as_c_str(
pub unsafe extern "C" fn iox2_service_name_as_str(
service_name_ptr: iox2_service_name_ptr,
service_name_len: *mut c_size_t,
) -> *const c_char {
debug_assert!(!service_name_ptr.is_null());
debug_assert!(!service_name_len.is_null());

let service_name = &*service_name_ptr;

Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-ffi/ffi/src/tests/node_name_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ fn basic_node_name_test() -> Result<(), Box<dyn std::error::Error>> {
assert_that!(ret_val, eq(IOX2_OK));

let mut node_name_len = 0;
let node_name_c_str = iox2_node_name_as_c_str(
let node_name_c_ptr = iox2_node_name_as_str(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elBoberido The name node_name_c_ptr causes me to scratch my head on first read. It's because it is just a pointer to chars (i.e. not null terminated)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elBoberido Maybe node_name_chars ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the function then be named iox2_node_name_as_chars?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elBoberido Works for me

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked the function name since str has this implicit null-termination in the C world and chars is great for just a char array.

iox2_cast_node_name_ptr(node_name_handle),
&mut node_name_len,
);

let slice = slice::from_raw_parts(node_name_c_str as *const _, node_name_len as _);
let slice = slice::from_raw_parts(node_name_c_ptr as *const _, node_name_len as _);
let node_name = str::from_utf8(slice)?;

assert_that!(node_name, eq(expected_node_name.as_str()));
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-ffi/ffi/src/tests/node_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ mod node {
let node_name = iox2_node_name(iox2_cast_node_ref_h(node_handle));

let mut node_name_len = 0;
let node_name_c_str = iox2_node_name_as_c_str(node_name, &mut node_name_len);
let node_name_c_ptr = iox2_node_name_as_str(node_name, &mut node_name_len);

let slice = slice::from_raw_parts(node_name_c_str as *const _, node_name_len as _);
let slice = slice::from_raw_parts(node_name_c_ptr as *const _, node_name_len as _);
let node_name_str = str::from_utf8(slice)?;

assert_that!(node_name_str, eq("hypnotoad"));
Expand Down
4 changes: 2 additions & 2 deletions iceoryx2-ffi/ffi/src/tests/service_name_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ fn basic_service_name_test() -> Result<(), Box<dyn std::error::Error>> {
assert_that!(ret_val, eq(IOX2_OK));

let mut service_name_len = 0;
let service_name_c_str = iox2_service_name_as_c_str(
let service_name_c_ptr = iox2_service_name_as_str(
iox2_cast_service_name_ptr(service_name_handle),
&mut service_name_len,
);

let slice = slice::from_raw_parts(service_name_c_str as *const _, service_name_len as _);
let slice = slice::from_raw_parts(service_name_c_ptr as *const _, service_name_len as _);
let service_name = str::from_utf8(slice)?;

assert_that!(service_name, eq(expected_service_name.as_str()));
Expand Down