From 0e98d3d6f1d615a8ed89e411ec1acc69500b5729 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Fri, 22 Jul 2022 06:09:47 +0200 Subject: [PATCH 1/3] iox-#1196 Add posix to clang-tidy diff scan list and move test files to follow file naming guideline Signed-off-by: Christian Eltzschig --- .clang-tidy-diff-scans.txt | 9 ++++----- ..._access_control.cpp => test_posix_access_control.cpp} | 0 .../{test_allocator.cpp => test_posix_allocator.cpp} | 0 ...st_shared_memory.cpp => test_posix_shared_memory.cpp} | 0 ...ry_object.cpp => test_posix_shared_memory_object.cpp} | 0 5 files changed, 4 insertions(+), 5 deletions(-) rename iceoryx_hoofs/test/moduletests/{test_access_control.cpp => test_posix_access_control.cpp} (100%) rename iceoryx_hoofs/test/moduletests/{test_allocator.cpp => test_posix_allocator.cpp} (100%) rename iceoryx_hoofs/test/moduletests/{test_shared_memory.cpp => test_posix_shared_memory.cpp} (100%) rename iceoryx_hoofs/test/moduletests/{test_shared_memory_object.cpp => test_posix_shared_memory_object.cpp} (100%) diff --git a/.clang-tidy-diff-scans.txt b/.clang-tidy-diff-scans.txt index 6220a7f6ac..60eca3641c 100644 --- a/.clang-tidy-diff-scans.txt +++ b/.clang-tidy-diff-scans.txt @@ -7,11 +7,10 @@ ./iceoryx_hoofs/test/moduletests/test_unit* ./iceoryx_hoofs/source/units/* -./iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/* -./iceoryx_hoofs/source/posix_wrapper/shared_memory_object/* -./iceoryx_hoofs/test/moduletests/test_allocator.cpp -./iceoryx_hoofs/test/moduletests/test_shared_memory.cpp -./iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp +./iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/* +./iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/* +./iceoryx_hoofs/source/posix_wrapper/* +./iceoryx_hoofs/test/moduletests/test_posix* # IMPORTANT: # after the first # everything is considered a comment, add new files and diff --git a/iceoryx_hoofs/test/moduletests/test_access_control.cpp b/iceoryx_hoofs/test/moduletests/test_posix_access_control.cpp similarity index 100% rename from iceoryx_hoofs/test/moduletests/test_access_control.cpp rename to iceoryx_hoofs/test/moduletests/test_posix_access_control.cpp diff --git a/iceoryx_hoofs/test/moduletests/test_allocator.cpp b/iceoryx_hoofs/test/moduletests/test_posix_allocator.cpp similarity index 100% rename from iceoryx_hoofs/test/moduletests/test_allocator.cpp rename to iceoryx_hoofs/test/moduletests/test_posix_allocator.cpp diff --git a/iceoryx_hoofs/test/moduletests/test_shared_memory.cpp b/iceoryx_hoofs/test/moduletests/test_posix_shared_memory.cpp similarity index 100% rename from iceoryx_hoofs/test/moduletests/test_shared_memory.cpp rename to iceoryx_hoofs/test/moduletests/test_posix_shared_memory.cpp diff --git a/iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp b/iceoryx_hoofs/test/moduletests/test_posix_shared_memory_object.cpp similarity index 100% rename from iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp rename to iceoryx_hoofs/test/moduletests/test_posix_shared_memory_object.cpp From c7db02cab441035cf902e393fb9f9c3bfe60dd1f Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Fri, 22 Jul 2022 06:45:04 +0200 Subject: [PATCH 2/3] iox-#1196 Fix overlooked warnings Signed-off-by: Christian Eltzschig --- .clang-tidy-diff-scans.txt | 2 ++ .../posix_wrapper/shared_memory_object.hpp | 2 +- .../source/posix_wrapper/message_queue.cpp | 2 ++ .../source/posix_wrapper/named_pipe.cpp | 17 ++++++++++--- .../posix_wrapper/shared_memory_object.cpp | 25 ++++++++++++++----- .../test_posix_semaphore_interface.cpp | 2 ++ 6 files changed, 40 insertions(+), 10 deletions(-) diff --git a/.clang-tidy-diff-scans.txt b/.clang-tidy-diff-scans.txt index 60eca3641c..5003fc53ed 100644 --- a/.clang-tidy-diff-scans.txt +++ b/.clang-tidy-diff-scans.txt @@ -9,7 +9,9 @@ ./iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/* ./iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/* +./iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/* ./iceoryx_hoofs/source/posix_wrapper/* +./iceoryx_hoofs/source/posix_wrapper/shared_memory_object/* ./iceoryx_hoofs/test/moduletests/test_posix* # IMPORTANT: diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp index 44fb33d45d..75495f3a88 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp @@ -49,7 +49,7 @@ class SharedMemoryObject public: using Builder = SharedMemoryObjectBuilder; - static constexpr void* NO_ADDRESS_HINT = nullptr; + static constexpr const void* const NO_ADDRESS_HINT = nullptr; SharedMemoryObject(const SharedMemoryObject&) = delete; SharedMemoryObject& operator=(const SharedMemoryObject&) = delete; SharedMemoryObject(SharedMemoryObject&&) noexcept = default; diff --git a/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp b/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp index fff4a58cb6..18be394cb8 100644 --- a/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp @@ -121,6 +121,8 @@ MessageQueue& MessageQueue::operator=(MessageQueue&& other) noexcept } CreationPattern_t::operator=(std::move(other)); + /// NOLINTJUSTIFICATION will be fixed with the builder pattern + /// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) m_name = std::move(other.m_name); m_attributes = other.m_attributes; m_mqDescriptor = other.m_mqDescriptor; diff --git a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp index ac106d5eec..f6541be68f 100644 --- a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp @@ -137,6 +137,8 @@ NamedPipe& NamedPipe::operator=(NamedPipe&& rhs) noexcept IOX_DISCARD_RESULT(destroy()); CreationPattern_t::operator=(std::move(rhs)); + /// NOLINTJUSTIFICATION will be fixed with the builder pattern + /// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) m_sharedMemory = std::move(rhs.m_sharedMemory); m_data = rhs.m_data; rhs.m_data = nullptr; @@ -153,9 +155,18 @@ NamedPipe::~NamedPipe() noexcept template IpcChannelName_t NamedPipe::convertName(const Prefix& p, const IpcChannelName_t& name) noexcept { - return IpcChannelName_t(cxx::TruncateToCapacity, - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) - cxx::concatenate(p, (name.c_str()[0] == '/') ? *name.substr(1) : name).c_str()); + IpcChannelName_t channelName = p; + + if (name[0] == '/') + { + name.substr(1).and_then([&](auto& s) { channelName.append(cxx::TruncateToCapacity, s); }); + } + else + { + channelName.append(cxx::TruncateToCapacity, name); + } + + return channelName; } cxx::expected NamedPipe::destroy() noexcept diff --git a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp index 58800a82b2..b0912964cf 100644 --- a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp @@ -34,15 +34,22 @@ namespace iox { namespace posix { -constexpr void* SharedMemoryObject::NO_ADDRESS_HINT; +constexpr const void* const SharedMemoryObject::NO_ADDRESS_HINT; constexpr uint64_t SIGBUS_ERROR_MESSAGE_LENGTH = 1024U + platform::IOX_MAX_SHM_NAME_LENGTH; +/// NOLINTJUSTIFICATION global variables are only accessible from within this compilation unit +/// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables) +/// +/// NOLINTJUSTIFICATION c array required to print a signal safe error message in memsetSigbusHandler +/// NOLINTNEXTLINE(hicpp-avoid-c-arrays,cppcoreguidelines-avoid-c-arrays) static char sigbusErrorMessage[SIGBUS_ERROR_MESSAGE_LENGTH]; static std::mutex sigbusHandlerMutex; +/// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables) static void memsetSigbusHandler(int) noexcept { - auto result = write(STDERR_FILENO, sigbusErrorMessage, strnlen(sigbusErrorMessage, SIGBUS_ERROR_MESSAGE_LENGTH)); + auto result = + write(STDERR_FILENO, &sigbusErrorMessage[0], strnlen(&sigbusErrorMessage[0], SIGBUS_ERROR_MESSAGE_LENGTH)); IOX_DISCARD_RESULT(result); _exit(EXIT_FAILURE); } @@ -52,7 +59,10 @@ cxx::expected SharedMemoryObjectBui auto printErrorDetails = [this] { LogError() << "Unable to create a shared memory object with the following properties [ name = " << m_name << ", sizeInBytes = " << m_memorySizeInBytes << ", access mode = " << asStringLiteral(m_accessMode) - << ", open mode = " << asStringLiteral(m_openMode) << ", baseAddressHint = " + << ", open mode = " << asStringLiteral(m_openMode) + << ", baseAddressHint = " + // NOLINTJUSTIFICATION baseAddressHint printed on failure to make debugging easier as uint64 hex + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) << ((m_baseAddressHint) ? log::HexFormat(reinterpret_cast(*m_baseAddressHint)) : log::HexFormat(static_cast(0U))) << ((m_baseAddressHint) ? "" : " (no hint set)") @@ -108,8 +118,11 @@ cxx::expected SharedMemoryObjectBui return cxx::error(SharedMemoryObjectError::INTERNAL_LOGIC_FAILURE); } - snprintf( - sigbusErrorMessage, + // NOLINTJUSTIFICATION snprintf required to populate char array so that it can be used signal safe in + // a possible signal call + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg,hicpp-vararg) + IOX_DISCARD_RESULT(snprintf( + &sigbusErrorMessage[0], SIGBUS_ERROR_MESSAGE_LENGTH, "While setting the acquired shared memory to zero a fatal SIGBUS signal appeared caused by memset. The " "shared memory object with the following properties [ name = %s, sizeInBytes = %llu, access mode = %s, " @@ -120,7 +133,7 @@ cxx::expected SharedMemoryObjectBui asStringLiteral(m_accessMode), asStringLiteral(m_openMode), (m_baseAddressHint) ? *m_baseAddressHint : nullptr, - std::bitset(static_cast(m_permissions)).to_ulong()); + std::bitset(static_cast(m_permissions)).to_ulong())); memset(memoryMap->getBaseAddress(), 0, m_memorySizeInBytes); } diff --git a/iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp b/iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp index 4647cf67dd..337c8e30e2 100644 --- a/iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp +++ b/iceoryx_hoofs/test/moduletests/test_posix_semaphore_interface.cpp @@ -241,6 +241,8 @@ TYPED_TEST(SemaphoreInterfaceTest, FailingTimedWaitDoesNotChangeSemaphoreValue) { ::testing::Test::RecordProperty("TEST_ID", "cb2f5ded-7c04-4e2d-ab41-bbe231a520c7"); constexpr uint64_t NUMBER_OF_DECREMENTS = 4U; + // NOLINTJUSTIFICATION user defined literal no risk of number-letter mixup + // NOLINTNEXTLINE(hicpp-uppercase-literal-suffix,readability-uppercase-literal-suffix) constexpr iox::units::Duration timeToWait = 2_us; for (uint64_t i = 0; i < NUMBER_OF_DECREMENTS; ++i) From 28200bae47b75f3684b6666cf5376772dbfecdbe Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Mon, 25 Jul 2022 13:59:53 +0200 Subject: [PATCH 3/3] iox-#1196 Add issue numbers to suppression todos Signed-off-by: Christian Eltzschig --- iceoryx_hoofs/source/posix_wrapper/message_queue.cpp | 2 +- iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp b/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp index 18be394cb8..a4f4cfe323 100644 --- a/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/message_queue.cpp @@ -121,7 +121,7 @@ MessageQueue& MessageQueue::operator=(MessageQueue&& other) noexcept } CreationPattern_t::operator=(std::move(other)); - /// NOLINTJUSTIFICATION will be fixed with the builder pattern + /// NOLINTJUSTIFICATION iox-#1036 will be fixed with the builder pattern /// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) m_name = std::move(other.m_name); m_attributes = other.m_attributes; diff --git a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp index f6541be68f..527868627f 100644 --- a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp @@ -137,7 +137,7 @@ NamedPipe& NamedPipe::operator=(NamedPipe&& rhs) noexcept IOX_DISCARD_RESULT(destroy()); CreationPattern_t::operator=(std::move(rhs)); - /// NOLINTJUSTIFICATION will be fixed with the builder pattern + /// NOLINTJUSTIFICATION iox-#1036 will be fixed with the builder pattern /// NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) m_sharedMemory = std::move(rhs.m_sharedMemory); m_data = rhs.m_data;