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

Iox #260 remove std string from posh part 1 #373

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
6 changes: 4 additions & 2 deletions iceoryx_binding_c/include/iceoryx_binding_c/runnable.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,17 @@ void iox_runnable_destroy(iox_runnable_t const self);
/// @param[in] name pointer to a memory location where the name can be written to
/// @param[in] nameCapacity size of the memory location where the name is written to
/// @return the actual length of the runnable name, if the return value is greater
/// then nameCapacity the name is truncated
/// then nameCapacity the name is truncated.
elBoberido marked this conversation as resolved.
Show resolved Hide resolved
/// If name is a nullptr, 0 will be returned.
uint64_t iox_runnable_get_name(iox_runnable_t const self, char* const name, const uint64_t nameCapacity);

/// @brief acquires the name of the process in which the runnable is stored
/// @param[in] self handle to the runnable
/// @param[in] name pointer to a memory location where the name can be written to
/// @param[in] nameCapacity size of the memory location where the name is written to
/// @return the actual length of the process name, if the return value is greater
/// then nameCapacity the name is truncated
/// then nameCapacity the name is truncated.
/// If name is a nullptr, 0 will be returned.
uint64_t iox_runnable_get_process_name(iox_runnable_t const self, char* const name, const uint64_t nameCapacity);

#endif
1 change: 1 addition & 0 deletions iceoryx_binding_c/include/iceoryx_binding_c/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ void iox_runtime_register(const char* const name);
/// @return The length of the instance-name. If the instance-name is longer then nameLength a
/// number greater nameLength is returned and the instance-name, truncated
/// to nameLength, is written into the memory location of name.
/// If name is a nullptr, 0 will be returned.
uint64_t iox_runtime_get_instance_name(char* const name, const uint64_t nameLength);

#endif
17 changes: 15 additions & 2 deletions iceoryx_binding_c/source/c_runnable.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved.
// Copyright (c) 2020 by Robert Bosch GmbH, Apex.AI Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -56,15 +56,28 @@ void iox_runnable_destroy(iox_runnable_t const self)

uint64_t iox_runnable_get_name(iox_runnable_t const self, char* const name, const uint64_t nameCapacity)
{
if (name == nullptr)
{
return 0U;
}

auto nameAsString = RunnableBindingExtension(self).getRunnableName();
strncpy(name, nameAsString.c_str(), nameCapacity);
name[nameCapacity - 1U] = '\0'; // strncpy doesn't add a null-termination if destination is smaller than source

return nameAsString.size();
}

uint64_t iox_runnable_get_process_name(iox_runnable_t const self, char* const name, const uint64_t nameCapacity)
{
if (name == nullptr)
{
return 0U;
}

auto nameAsString = RunnableBindingExtension(self).getProcessName();
strncpy(name, nameAsString.c_str(), nameCapacity);
name[nameCapacity - 1U] = '\0'; // strncpy doesn't add a null-termination if destination is smaller than source

return nameAsString.size();
}

27 changes: 22 additions & 5 deletions iceoryx_binding_c/source/c_runtime.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved.
// Copyright (c) 2020 by Robert Bosch GmbH, Apex.AI Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -23,13 +23,30 @@ extern "C" {

void iox_runtime_register(const char* const name)
{
PoshRuntime::getInstance(name);
if (name == nullptr)
{
LogError() << "Application name is a nullptr!";
std::terminate();
dkroenke marked this conversation as resolved.
Show resolved Hide resolved
}
else if (strnlen(name, iox::MAX_PROCESS_NAME_LENGTH + 1) > MAX_PROCESS_NAME_LENGTH)
dkroenke marked this conversation as resolved.
Show resolved Hide resolved
{
LogError() << "Application name has more than 100 characters!";
std::terminate();
}

PoshRuntime::getInstance(ProcessName_t(iox::cxx::TruncateToCapacity, name));
}

uint64_t iox_runtime_get_instance_name(char* const name, const uint64_t nameLength)
{
if (name == nullptr)
{
return 0U;
}

auto instanceName = PoshRuntime::getInstance().getInstanceName();
uint64_t instanceNameSize = instanceName.size();
strncpy(name, instanceName.c_str(), std::min(nameLength, instanceNameSize));
return instanceNameSize;
std::strncpy(name, instanceName.c_str(), nameLength);
name[nameLength - 1U] = '\0'; // strncpy doesn't add a null-termination if destination is smaller than source

return instanceName.size();
}
48 changes: 47 additions & 1 deletion iceoryx_binding_c/test/integrationtests/test_runnable.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved.
// Copyright (c) 2020 by Robert Bosch GmbH, Apex.AI Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -50,9 +50,55 @@ TEST_F(iox_runnable_test, createdRunnableHasCorrectRunnableName)
EXPECT_EQ(std::string(name), m_runnableName);
}

TEST_F(iox_runnable_test, getRunnableNameBufferIsNullptr)
{
auto nameLength = iox_runnable_get_name(m_sut, nullptr, 100);

ASSERT_THAT(nameLength, Eq(0U));
}

TEST_F(iox_runnable_test, getRunnableNameBufferIsLessThanRunnableNameLength)
{
constexpr uint64_t RUNNABLE_NAME_BUFFER_LENGTH{10};
char truncatedRunnableName[RUNNABLE_NAME_BUFFER_LENGTH];
for (auto& c : truncatedRunnableName)
{
c = '#';
}
auto nameLength = iox_runnable_get_name(m_sut, truncatedRunnableName, RUNNABLE_NAME_BUFFER_LENGTH);

std::string expectedRunnableName = "hypnotoad";

ASSERT_THAT(nameLength, Eq(m_runnableName.size()));
EXPECT_THAT(truncatedRunnableName, StrEq(expectedRunnableName));
}

TEST_F(iox_runnable_test, createdRunnableHasCorrectProcessName)
{
char name[100];
ASSERT_EQ(iox_runnable_get_process_name(m_sut, name, 100), m_processName.size());
EXPECT_EQ(std::string(name), m_processName);
}

TEST_F(iox_runnable_test, getRunnableProcessNameBufferIsNullptr)
{
auto nameLength = iox_runnable_get_process_name(m_sut, nullptr, 100);

ASSERT_THAT(nameLength, Eq(0U));
}

TEST_F(iox_runnable_test, getRunnableProcessNameBufferIsLessThanRunnableProcessNameLength)
{
constexpr uint64_t PROCESS_NAME_BUFFER_LENGTH{10};
char truncatedProcessName[PROCESS_NAME_BUFFER_LENGTH];
for (auto& c : truncatedProcessName)
{
c = '#';
}
auto nameLength = iox_runnable_get_process_name(m_sut, truncatedProcessName, PROCESS_NAME_BUFFER_LENGTH);

std::string expectedProcessName = "/stoepsel";

ASSERT_THAT(nameLength, Eq(m_processName.size()));
EXPECT_THAT(truncatedProcessName, StrEq(expectedProcessName));
}
102 changes: 102 additions & 0 deletions iceoryx_binding_c/test/integrationtests/test_runtime.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright (c) 2020 by Apex.AI Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

extern "C" {
#include "iceoryx_binding_c/runtime.h"
}

#include "iceoryx_posh/iceoryx_posh_types.hpp"
#include "testutils/roudi_gtest.hpp"

using namespace iox;
using namespace iox::runtime;

class BindingC_Runtime_test : public RouDi_GTest
{
public:
void SetUp()
{
}

void TearDown()
{
}
};

TEST_F(BindingC_Runtime_test, SuccessfulRegistration)
{
constexpr char EXPECTED_APP_NAME[iox::MAX_PROCESS_NAME_LENGTH + 1] = "/chucky";
iox_runtime_register(EXPECTED_APP_NAME);

char actualAppName[iox::MAX_PROCESS_NAME_LENGTH + 1];
auto nameLength = iox_runtime_get_instance_name(actualAppName, iox::MAX_PROCESS_NAME_LENGTH + 1);

ASSERT_THAT(nameLength, Eq(strnlen(EXPECTED_APP_NAME, iox::MAX_PROCESS_NAME_LENGTH + 1)));
EXPECT_THAT(actualAppName, StrEq(EXPECTED_APP_NAME));
}

TEST_F(BindingC_Runtime_test, AppNameLengthIsMax)
{
std::string maxName(iox::MAX_PROCESS_NAME_LENGTH, 's');
maxName.front() = '/';

iox_runtime_register(maxName.c_str());

char actualAppName[iox::MAX_PROCESS_NAME_LENGTH + 1];
auto nameLength = iox_runtime_get_instance_name(actualAppName, iox::MAX_PROCESS_NAME_LENGTH + 1);

ASSERT_THAT(nameLength, Eq(iox::MAX_PROCESS_NAME_LENGTH));
}

TEST_F(BindingC_Runtime_test, AppNameLengthIsOutOfLimit)
{
std::string tooLongName(iox::MAX_PROCESS_NAME_LENGTH + 1, 's');
tooLongName.insert(0, 1, '/');

EXPECT_DEATH({ iox_runtime_register(tooLongName.c_str()); }, "Application name has more than 100 characters!");
}

TEST_F(BindingC_Runtime_test, AppNameIsNullptr)
{
EXPECT_DEATH({ iox_runtime_register(nullptr); }, "Application name is a nullptr!");
}

TEST_F(BindingC_Runtime_test, GetInstanceNameIsNullptr)
{
constexpr char EXPECTED_APP_NAME[iox::MAX_PROCESS_NAME_LENGTH + 1] = "/chucky";
iox_runtime_register(EXPECTED_APP_NAME);

char actualAppName[iox::MAX_PROCESS_NAME_LENGTH + 1];
auto nameLength = iox_runtime_get_instance_name(nullptr, iox::MAX_PROCESS_NAME_LENGTH + 1);

ASSERT_THAT(nameLength, Eq(0U));
}

TEST_F(BindingC_Runtime_test, GetInstanceNameLengthIsLessThanAppNameLength)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about tests when the app name length is max or is exceeding the allowed name length?

Copy link
Member Author

Choose a reason for hiding this comment

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

like the test in line 49 and 62 ;)

{
constexpr char ACTUAL_APP_NAME[iox::MAX_PROCESS_NAME_LENGTH + 1] = "/chucky";
constexpr char EXPECTED_APP_NAME[iox::MAX_PROCESS_NAME_LENGTH + 1] = "/chuck";
iox_runtime_register(ACTUAL_APP_NAME);

constexpr uint64_t APP_NAME_BUFFER_LENGTH{7};
char truncatedAppName[APP_NAME_BUFFER_LENGTH];
for (auto& c : truncatedAppName)
{
c = '#';
}
auto nameLength = iox_runtime_get_instance_name(truncatedAppName, APP_NAME_BUFFER_LENGTH);

ASSERT_THAT(nameLength, Eq(strnlen(ACTUAL_APP_NAME, iox::MAX_PROCESS_NAME_LENGTH + 1)));
EXPECT_THAT(truncatedAppName, StrEq(EXPECTED_APP_NAME));
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,8 @@ class ServiceDescription
ServiceDescription& operator=(ServiceDescription&&) = default;

/// @brief serialization of the capro description.
/// @todo operator Serialization() replaces getServiceString()
operator cxx::Serialization() const;

/// @brief Generate a servicestring filled with the member variables for
/// service communication
/// @return stringstream filled with member vars ()
std::string getServiceString() const noexcept;

/// @brief Returns true if it contains a service description which does not have
/// events, otherwise it returns false
bool hasServiceOnlyDescription() const noexcept;
Expand Down
2 changes: 1 addition & 1 deletion iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ struct DefaultChunkQueueConfig

// alias for cxx::string
using ConfigFilePathString_t = cxx::string<1024>;
using ProcessName_t = cxx::string<100>;
using ProcessName_t = cxx::string<MAX_PROCESS_NAME_LENGTH>;
using RunnableName_t = cxx::string<100>;
elBoberido marked this conversation as resolved.
Show resolved Hide resolved

namespace runtime
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "iceoryx_posh/internal/mepoo/memory_manager.hpp"
#include "iceoryx_posh/internal/mepoo/mepoo_segment.hpp"
#include "iceoryx_posh/mepoo/segment_config.hpp"
#include "iceoryx_utils/cxx/string.hpp"
#include "iceoryx_utils/cxx/vector.hpp"
#include "iceoryx_utils/internal/posix_wrapper/shared_memory_object/allocator.hpp"
#include "iceoryx_utils/posix_wrapper/posix_access_rights.hpp"
Expand Down Expand Up @@ -49,7 +50,9 @@ class SegmentManager
struct SegmentMapping
{
public:
SegmentMapping(const std::string& sharedMemoryName,
using string_t = cxx::string<128>;
elBoberido marked this conversation as resolved.
Show resolved Hide resolved

SegmentMapping(const string_t& sharedMemoryName,
void* startAddress,
uint64_t size,
bool isWritable,
Expand All @@ -65,7 +68,7 @@ class SegmentManager
{
}

std::string m_sharedMemoryName{""};
string_t m_sharedMemoryName{""};
void* m_startAddress{nullptr};
uint64_t m_size{0};
bool m_isWritable{false};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#define IOX_POSH_ROUDI_ENVIRONMENT_ROUDI_ENVIRONMENT_HPP

#include "iceoryx_posh/iceoryx_posh_config.hpp"
#include "iceoryx_posh/iceoryx_posh_types.hpp"
#include "iceoryx_posh/internal/roudi/roudi.hpp"
#include "iceoryx_posh/internal/roudi_environment/runtime_test_interface.hpp"
#include "iceoryx_posh/roudi/iceoryx_roudi_components.hpp"
Expand Down Expand Up @@ -48,7 +49,7 @@ class RouDiEnvironment
void SetInterOpWaitingTime(const std::chrono::milliseconds& v);
void InterOpWait();

void CleanupAppResources(const std::string& name);
void CleanupAppResources(const ProcessName_t& name);

protected:
/// @note this is due to ambiguity of the cTor with the default parameter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#ifndef IOX_POSH_ROUDI_ENVIRONMENT_RUNTIME_TEST_INTERFACE_HPP
#define IOX_POSH_ROUDI_ENVIRONMENT_RUNTIME_TEST_INTERFACE_HPP

#include "iceoryx_posh/iceoryx_posh_types.hpp"

#include <atomic>
#include <map>
#include <mutex>
Expand All @@ -39,10 +41,10 @@ class RuntimeTestInterface

static std::mutex s_runtimeAccessMutex;

static std::map<std::string, runtime::PoshRuntime*> s_runtimes;
static std::map<ProcessName_t, runtime::PoshRuntime*> s_runtimes;

/// This is a replacement for the PoshRuntime::GetInstance factory method
/// @param [in] f_name ist the name of the runtime
/// @param [in] name ist the name of the runtime
/// @return a reference to a PoshRuntime
/// @note The runtime is stored in a vector and a thread local storage.
///
Expand All @@ -60,7 +62,7 @@ class RuntimeTestInterface
/// - FindService, OfferService and StopOfferService
/// This means that iox::runtime::PoshRuntimeImp::GetInstance(...) must be called before the above classes
/// are created or functions are called, to make the correct runtime active.
static runtime::PoshRuntime& runtimeFactoryGetInstance(const std::string& f_name);
static runtime::PoshRuntime& runtimeFactoryGetInstance(const ProcessName_t& name);

public:
RuntimeTestInterface(RuntimeTestInterface&& rhs);
Expand All @@ -75,7 +77,7 @@ class RuntimeTestInterface

void cleanupRuntimes();

void eraseRuntime(const std::string& f_name);
void eraseRuntime(const ProcessName_t& name);
};

} // namespace roudi
Expand Down
Loading