From 14aa17de01d58fd201bcc71b7e87c2c38baf189f Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Mon, 20 Nov 2023 20:02:31 +0100 Subject: [PATCH 1/6] iox-#2108 Add static functions to create an 'access_rights' object from a value --- .../include/iox/detail/filesystem.inl | 30 ++- .../filesystem/include/iox/filesystem.hpp | 206 ++++++------------ 2 files changed, 83 insertions(+), 153 deletions(-) diff --git a/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl b/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl index b076ceccfb..aa0f879075 100644 --- a/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl +++ b/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl @@ -1,4 +1,5 @@ // Copyright (c) 2022 by Apex.AI Inc. All rights reserved. +// Copyright (c) 2023 by Mathias Kraus . 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. @@ -188,53 +189,62 @@ inline bool doesEndWithPathSeparator(const iox::string& name) no return false; } -constexpr access_rights::value_type access_rights::value() const noexcept +inline constexpr access_rights access_rights::from_value_sanitized(const access_rights::value_type value) noexcept +{ + if (value != detail::UNKNOWN) + { + return access_rights{static_cast(value & detail::MASK)}; + } + return access_rights{value}; +} + +inline constexpr access_rights::value_type access_rights::value() const noexcept { return m_value; } -constexpr bool operator==(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr bool operator==(const access_rights lhs, const access_rights rhs) noexcept { return lhs.value() == rhs.value(); } -constexpr bool operator!=(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr bool operator!=(const access_rights lhs, const access_rights rhs) noexcept { return !(lhs == rhs); } -constexpr access_rights operator|(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr access_rights operator|(const access_rights lhs, const access_rights rhs) noexcept { return access_rights(lhs.value() | rhs.value()); } -constexpr access_rights operator&(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr access_rights operator&(const access_rights lhs, const access_rights rhs) noexcept { return access_rights(lhs.value() & rhs.value()); } -constexpr access_rights operator^(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr access_rights operator^(const access_rights lhs, const access_rights rhs) noexcept { return access_rights(lhs.value() ^ rhs.value()); } -constexpr access_rights operator~(const access_rights value) noexcept +inline constexpr access_rights operator~(const access_rights value) noexcept { // AXIVION Next Construct AutosarC++19_03-A4.7.1, AutosarC++19_03-M0.3.1, FaultDetection-IntegerOverflow : Cast is safe and required due to integer promotion return access_rights(static_cast(~value.value())); } -constexpr access_rights operator|=(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr access_rights operator|=(const access_rights lhs, const access_rights rhs) noexcept { return operator|(lhs, rhs); } -constexpr access_rights operator&=(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr access_rights operator&=(const access_rights lhs, const access_rights rhs) noexcept { return operator&(lhs, rhs); } -constexpr access_rights operator^=(const access_rights lhs, const access_rights rhs) noexcept +inline constexpr access_rights operator^=(const access_rights lhs, const access_rights rhs) noexcept { return operator^(lhs, rhs); } diff --git a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp index 61ab3aba24..19547fba9e 100644 --- a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp +++ b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp @@ -1,4 +1,5 @@ // Copyright (c) 2022 by Apex.AI Inc. All rights reserved. +// Copyright (c) 2023 by Mathias Kraus . 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. @@ -116,136 +117,55 @@ class access_rights final ~access_rights() noexcept = default; + /// @brief Creates an 'access_rights' object from a sanitized value, ensuring that only the bits defined in + /// 'iox::perms::mask' are set when the value is not 'iox::perm::unknown'. + /// @param[in] value for the 'access_rights' + /// @return the 'access_rights' object + static constexpr access_rights from_value_sanitized(const value_type value) noexcept; + + /// @brief Creates an 'access_rights' object from an unchecked value. The user has to ensure that only the bits + /// defined in 'iox::perms::mask' are set when the value is not 'iox::perm::unknown'. + /// @param[in] value for the 'access_rights' + /// @return the 'access_rights' object + static constexpr access_rights unsafe_from_value_unchecked(const value_type value) noexcept + { + // the code cannot be moved to the '*.inl' file since the function is used in the 'perms' namespace to define + // the 'access_rights' constants + return access_rights{value}; + } + constexpr value_type value() const noexcept; struct detail { // AXIVION DISABLE STYLE AutosarC++19_03-M2.13.2 : Filesystem permissions are defined in octal representation - // the code cannot be moved to the '*.inl' file since the functions are used in the 'perms' namespace to define - // the respective constants - - /// @note Implementation detail! Please use 'iox::perms::none'. - static constexpr access_rights none() noexcept - { - constexpr value_type VALUE{0}; - return access_rights{VALUE}; - } - - /// @note Implementation detail! Please use 'iox::perms::owner_read'. - static constexpr access_rights owner_read() noexcept - { - constexpr value_type VALUE{0400}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::owner_write'. - static constexpr access_rights owner_write() noexcept - { - constexpr value_type VALUE{0200}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::owner_exec'. - static constexpr access_rights owner_exec() noexcept - { - constexpr value_type VALUE{0100}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::owner_all'. - static constexpr access_rights owner_all() noexcept - { - constexpr value_type VALUE{0700}; - return access_rights{VALUE}; - } - - /// @note Implementation detail! Please use 'iox::perms::group_read'. - static constexpr access_rights group_read() noexcept - { - constexpr value_type VALUE{040}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::group_write'. - static constexpr access_rights group_write() noexcept - { - constexpr value_type VALUE{020}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::group_exec'. - static constexpr access_rights group_exec() noexcept - { - constexpr value_type VALUE{010}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::group_all'. - static constexpr access_rights group_all() noexcept - { - constexpr value_type VALUE{070}; - return access_rights{VALUE}; - } - - /// @note Implementation detail! Please use 'iox::perms::others_read'. - static constexpr access_rights others_read() noexcept - { - constexpr value_type VALUE{04}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::others_write'. - static constexpr access_rights others_write() noexcept - { - constexpr value_type VALUE{02}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::others_exec'. - static constexpr access_rights others_exec() noexcept - { - constexpr value_type VALUE{01}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::others_all'. - static constexpr access_rights others_all() noexcept - { - constexpr value_type VALUE{07}; - return access_rights{VALUE}; - } - - /// @note Implementation detail! Please use 'iox::perms::all'. - static constexpr access_rights all() noexcept - { - constexpr value_type VALUE{0777}; - return access_rights{VALUE}; - } - - /// @note Implementation detail! Please use 'iox::perms::set_uid'. - static constexpr access_rights set_uid() noexcept - { - constexpr value_type VALUE{04000}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::set_gid'. - static constexpr access_rights set_gid() noexcept - { - constexpr value_type VALUE{02000}; - return access_rights{VALUE}; - } - /// @note Implementation detail! Please use 'iox::perms::sticky_bit'. - static constexpr access_rights sticky_bit() noexcept - { - constexpr value_type VALUE{01000}; - return access_rights{VALUE}; - } - - /// @note Implementation detail! Please use 'iox::perms::mask'. - static constexpr access_rights mask() noexcept - { - constexpr value_type VALUE{07777}; - return access_rights{VALUE}; - } - - /// @note Implementation detail! Please use 'iox::perms::unknown'. - static constexpr access_rights unknown() noexcept - { - constexpr value_type VALUE{0xFFFFU}; - return access_rights{VALUE}; - } + static constexpr value_type NONE{0}; + + static constexpr value_type OWNER_READ{0400}; + static constexpr value_type OWNER_WRITE{0200}; + static constexpr value_type OWNER_EXEC{0100}; + static constexpr value_type OWNER_ALL{0700}; + + static constexpr value_type GROUP_READ{040}; + static constexpr value_type GROUP_WRITE{020}; + static constexpr value_type GROUP_EXEC{010}; + static constexpr value_type GROUP_ALL{070}; + + static constexpr value_type OTHERS_READ{04}; + static constexpr value_type OTHERS_WRITE{02}; + static constexpr value_type OTHERS_EXEC{01}; + static constexpr value_type OTHERS_ALL{07}; + + static constexpr value_type ALL{0777}; + + static constexpr value_type SET_UID{04000}; + static constexpr value_type SET_GID{02000}; + static constexpr value_type STICKY_BIT{01000}; + + static constexpr value_type MASK{07777}; + + static constexpr value_type UNKNOWN{0xFFFFU}; // AXIVION ENABLE STYLE AutosarC++19_03-M2.13.2 }; @@ -278,55 +198,55 @@ namespace perms // AXIVION DISABLE STYLE AutosarC++19_03-A2.10.5 : Name reuse is intentional since they refer to the same value. Additionally, different namespaces are used. /// @brief Deny everything -static constexpr access_rights none{access_rights::detail::none()}; +static constexpr auto none{access_rights::unsafe_from_value_unchecked(access_rights::detail::NONE)}; /// @brief owner has read permission -static constexpr access_rights owner_read{access_rights::detail::owner_read()}; +static constexpr auto owner_read{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_READ)}; /// @brief owner has write permission -static constexpr access_rights owner_write{access_rights::detail::owner_write()}; +static constexpr auto owner_write{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_WRITE)}; /// @brief owner has execution permission -static constexpr access_rights owner_exec{access_rights::detail::owner_exec()}; +static constexpr auto owner_exec{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_EXEC)}; /// @brief owner has all permissions -static constexpr access_rights owner_all{access_rights::detail::owner_all()}; +static constexpr auto owner_all{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_ALL)}; /// @brief group has read permission -static constexpr access_rights group_read{access_rights::detail::group_read()}; +static constexpr auto group_read{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_READ)}; /// @brief group has write permission -static constexpr access_rights group_write{access_rights::detail::group_write()}; +static constexpr auto group_write{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_WRITE)}; /// @brief group has execution permission -static constexpr access_rights group_exec{access_rights::detail::group_exec()}; +static constexpr auto group_exec{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_EXEC)}; /// @brief group has all permissions -static constexpr access_rights group_all{access_rights::detail::group_all()}; +static constexpr auto group_all{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_ALL)}; /// @brief others have read permission -static constexpr access_rights others_read{access_rights::detail::others_read()}; +static constexpr auto others_read{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_READ)}; /// @brief others have write permission -static constexpr access_rights others_write{access_rights::detail::others_write()}; +static constexpr auto others_write{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_WRITE)}; /// @brief others have execution permission -static constexpr access_rights others_exec{access_rights::detail::others_exec()}; +static constexpr auto others_exec{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_EXEC)}; /// @brief others have all permissions -static constexpr access_rights others_all{access_rights::detail::others_all()}; +static constexpr auto others_all{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_ALL)}; /// @brief all permissions for everyone -static constexpr access_rights all{access_rights::detail::all()}; +static constexpr auto all{access_rights::unsafe_from_value_unchecked(access_rights::detail::ALL)}; /// @brief set uid bit /// @note introduction into setgit/setuid: https://en.wikipedia.org/wiki/Setuid // AXIVION Next Construct AutosarC++19_03-M2.10.1: The constant is in a namespace and mimics the C++17 STL equivalent -static constexpr access_rights set_uid{access_rights::detail::set_uid()}; +static constexpr auto set_uid{access_rights::unsafe_from_value_unchecked(access_rights::detail::SET_UID)}; /// @brief set gid bit /// @note introduction into setgit/setuid: https://en.wikipedia.org/wiki/Setuid // AXIVION Next Construct AutosarC++19_03-M2.10.1: The constant is in a namespace and mimics the C++17 STL equivalent -static constexpr access_rights set_gid{access_rights::detail::set_gid()}; +static constexpr auto set_gid{access_rights::unsafe_from_value_unchecked(access_rights::detail::SET_GID)}; /// @brief set sticky bit /// @note sticky bit introduction: https://en.wikipedia.org/wiki/Sticky_bit -static constexpr access_rights sticky_bit{access_rights::detail::sticky_bit()}; +static constexpr auto sticky_bit{access_rights::unsafe_from_value_unchecked(access_rights::detail::STICKY_BIT)}; /// @brief all permissions for everyone as well as uid, gid and sticky bit -static constexpr access_rights mask{access_rights::detail::mask()}; +static constexpr auto mask{access_rights::unsafe_from_value_unchecked(access_rights::detail::MASK)}; /// @brief unknown permissions -static constexpr access_rights unknown{access_rights::detail::unknown()}; +static constexpr auto unknown{access_rights::unsafe_from_value_unchecked(access_rights::detail::UNKNOWN)}; // AXIVION ENABLE STYLE AutosarC++19_03-A2.10.5 } // namespace perms From a2b7c3521ff85d700ae1a79cb37dc34a3184070a Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Mon, 20 Nov 2023 21:14:51 +0100 Subject: [PATCH 2/6] iox-#2108 Use sane value for 'iox::perms::unknown' --- .../filesystem/include/iox/detail/filesystem.inl | 6 +++--- iceoryx_hoofs/filesystem/include/iox/filesystem.hpp | 13 ++++++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl b/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl index aa0f879075..91959e11fd 100644 --- a/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl +++ b/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl @@ -191,11 +191,11 @@ inline bool doesEndWithPathSeparator(const iox::string& name) no inline constexpr access_rights access_rights::from_value_sanitized(const access_rights::value_type value) noexcept { - if (value != detail::UNKNOWN) + if ((value & detail::UNKNOWN) == detail::UNKNOWN) { - return access_rights{static_cast(value & detail::MASK)}; + return access_rights{detail::UNKNOWN}; } - return access_rights{value}; + return access_rights{static_cast(value & detail::MASK)}; } inline constexpr access_rights::value_type access_rights::value() const noexcept diff --git a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp index 19547fba9e..e57b4e3bc1 100644 --- a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp +++ b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp @@ -118,13 +118,15 @@ class access_rights final ~access_rights() noexcept = default; /// @brief Creates an 'access_rights' object from a sanitized value, ensuring that only the bits defined in - /// 'iox::perms::mask' are set when the value is not 'iox::perm::unknown'. + /// 'iox::perms::mask' are set when the 'iox::perm::unknown' bit is not set. If the 'iox::perm::unknown' bit is set + /// all other bits will be reset. /// @param[in] value for the 'access_rights' /// @return the 'access_rights' object static constexpr access_rights from_value_sanitized(const value_type value) noexcept; /// @brief Creates an 'access_rights' object from an unchecked value. The user has to ensure that only the bits - /// defined in 'iox::perms::mask' are set when the value is not 'iox::perm::unknown'. + /// defined in 'iox::perms::mask' are set when the 'iox::perm::unknown' bit is not set. If the 'iox::perm::unknown' + /// bit is set all other bits should be reset. /// @param[in] value for the 'access_rights' /// @return the 'access_rights' object static constexpr access_rights unsafe_from_value_unchecked(const value_type value) noexcept @@ -165,7 +167,12 @@ class access_rights final static constexpr value_type MASK{07777}; - static constexpr value_type UNKNOWN{0xFFFFU}; + // intentionally different from 'std::filesystem::perms::unknown' to prevent unexpected results. Combining a + // permission set to 'std::filesystem::perms::unknown' with other permission flags using bitwise OR may result + // in a value that is not representative of a valid permission state. By setting the value to '0x8000' only the + // MSB is 1 and all bits representing permissions are set to 0 and a bitwise OR will therefore also always + // result in a 0. + static constexpr value_type UNKNOWN{0x8000U}; // AXIVION ENABLE STYLE AutosarC++19_03-M2.13.2 }; From 1a10a2ce92c27d9298c617b9415aee4806fda8c8 Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Mon, 20 Nov 2023 21:26:16 +0100 Subject: [PATCH 3/6] iox-#2108 Add tests for new functions --- .../test_filesystem_filesystem.cpp | 70 +++++++++++++++---- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp b/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp index 91f6c191ba..8d9dced0e9 100644 --- a/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp +++ b/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp @@ -591,14 +591,56 @@ TEST(filesystem_test_isValidPathEntry, StringWithRelativeComponentsIsInvalidWhen iox::RelativePathComponents::REJECT)); } +TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueUnknown) +{ + ::testing::Test::RecordProperty("TEST_ID", "6e510e0e-1a0c-40a3-bab0-941a78d745d8"); + constexpr auto TEST_VALUE = access_rights::detail::UNKNOWN; + + EXPECT_THAT(access_rights::from_value_sanitized(TEST_VALUE).value(), Eq(TEST_VALUE)); +} + +TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueUnknownWithExtraBitsSet) +{ + ::testing::Test::RecordProperty("TEST_ID", "1af33cc5-c870-4fef-ad23-6525296d6792"); + constexpr auto TEST_VALUE_SANITIZED = access_rights::detail::UNKNOWN; + constexpr auto TEST_VALUE = TEST_VALUE_SANITIZED | static_cast(0x0101U); + + EXPECT_THAT(access_rights::from_value_sanitized(TEST_VALUE).value(), Eq(TEST_VALUE_SANITIZED)); +} + +TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueInRangeOfPermsMask) +{ + ::testing::Test::RecordProperty("TEST_ID", "5a6c2ece-9cd7-4779-ad0b-16372aebd407"); + constexpr auto TEST_VALUE = access_rights::detail::OWNER_READ; + + EXPECT_THAT(access_rights::from_value_sanitized(TEST_VALUE).value(), Eq(TEST_VALUE)); +} + +TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueOutOfRangeOfPermsMask) +{ + ::testing::Test::RecordProperty("TEST_ID", "8a291709-a75c-4afa-96d8-d57a0d40696c"); + constexpr auto TEST_VALUE_SANITIZED = access_rights::detail::OWNER_EXEC; + constexpr auto TEST_VALUE = TEST_VALUE_SANITIZED | static_cast(010000); + + EXPECT_THAT(access_rights::from_value_sanitized(TEST_VALUE).value(), Eq(TEST_VALUE_SANITIZED)); +} + +TEST(filesystem_test, accessRightsUnsafeFromValueUncheckedWorkForAnyValue) +{ + ::testing::Test::RecordProperty("TEST_ID", "5f34cef3-f778-4eed-bb78-42decdb8f7bb"); + constexpr auto TEST_VALUE = access_rights::detail::OWNER_WRITE | static_cast(010000); + + EXPECT_THAT(access_rights::unsafe_from_value_unchecked(TEST_VALUE).value(), Eq(TEST_VALUE)); +} + TEST(filesystem_test, permsBinaryOrEqualToBinaryOrOfUnderlyingType) { ::testing::Test::RecordProperty("TEST_ID", "0b72fcec-c2b3-4a45-801f-542ff3195a2f"); constexpr access_rights TEST_VALUE_LHS = perms::others_write; constexpr access_rights TEST_VALUE_RHS = perms::group_all; - constexpr auto BASE_VALUE_LHS = TEST_VALUE_LHS.value(); - constexpr auto BASE_VALUE_RHS = TEST_VALUE_RHS.value(); + constexpr auto BASE_VALUE_LHS = iox::access_rights::detail::OTHERS_WRITE; + constexpr auto BASE_VALUE_RHS = iox::access_rights::detail::GROUP_ALL; EXPECT_THAT((TEST_VALUE_LHS | TEST_VALUE_RHS).value(), Eq(BASE_VALUE_LHS | BASE_VALUE_RHS)); } @@ -609,8 +651,8 @@ TEST(filesystem_test, permsBinaryAndEqualToBinaryAndOfUnderlyingType) constexpr access_rights TEST_VALUE_LHS = perms::others_read; constexpr access_rights TEST_VALUE_RHS = perms::mask; - constexpr auto BASE_VALUE_LHS = TEST_VALUE_LHS.value(); - constexpr auto BASE_VALUE_RHS = TEST_VALUE_RHS.value(); + constexpr auto BASE_VALUE_LHS = iox::access_rights::detail::OTHERS_READ; + constexpr auto BASE_VALUE_RHS = iox::access_rights::detail::MASK; EXPECT_THAT((TEST_VALUE_LHS & TEST_VALUE_RHS).value(), Eq(BASE_VALUE_LHS & BASE_VALUE_RHS)); } @@ -621,8 +663,8 @@ TEST(filesystem_test, permsBinaryExclusiveOrEqualToBinaryExclusiveOrOfUnderlying constexpr access_rights TEST_VALUE_LHS = perms::set_gid; constexpr access_rights TEST_VALUE_RHS = perms::set_uid; - constexpr auto BASE_VALUE_LHS = TEST_VALUE_LHS.value(); - constexpr auto BASE_VALUE_RHS = TEST_VALUE_RHS.value(); + constexpr auto BASE_VALUE_LHS = iox::access_rights::detail::SET_GID; + constexpr auto BASE_VALUE_RHS = iox::access_rights::detail::SET_UID; EXPECT_THAT((TEST_VALUE_LHS ^ TEST_VALUE_RHS).value(), Eq(BASE_VALUE_LHS ^ BASE_VALUE_RHS)); } @@ -632,8 +674,8 @@ TEST(filesystem_test, permsBinaryComplementEqualToBinaryComplementOfUnderlyingTy ::testing::Test::RecordProperty("TEST_ID", "c313cf42-4cf0-4836-95ff-129111a707b0"); constexpr access_rights TEST_VALUE = perms::owner_read; - constexpr access_rights::value_type BASE_VALUE = 0x0100; - constexpr access_rights::value_type EXPECTED_VALUE = 0xFEFF; + constexpr auto BASE_VALUE = iox::access_rights::detail::OWNER_READ; + constexpr auto EXPECTED_VALUE = static_cast(~BASE_VALUE); ASSERT_THAT(TEST_VALUE.value(), Eq(BASE_VALUE)); @@ -646,8 +688,8 @@ TEST(filesystem_test, permsBinaryOrAssignmentEqualToBinaryOrAssignmentOfUnderlyi constexpr access_rights TEST_VALUE = perms::sticky_bit; constexpr access_rights TEST_VALUE_RHS = perms::group_read; - auto sutBaseValue = TEST_VALUE.value(); - constexpr auto BASE_VALUE_RHS = TEST_VALUE_RHS.value(); + auto sutBaseValue = iox::access_rights::detail::STICKY_BIT; + constexpr auto BASE_VALUE_RHS = iox::access_rights::detail::GROUP_READ; access_rights sut = TEST_VALUE; @@ -660,8 +702,8 @@ TEST(filesystem_test, permsBinaryAndAssignmentEqualToBinaryAndAssignmentOfUnderl constexpr access_rights TEST_VALUE = perms::others_exec; constexpr access_rights TEST_VALUE_RHS = perms::others_all; - auto sutBaseValue = TEST_VALUE.value(); - constexpr auto BASE_VALUE_RHS = TEST_VALUE_RHS.value(); + auto sutBaseValue = iox::access_rights::detail::OTHERS_EXEC; + constexpr auto BASE_VALUE_RHS = iox::access_rights::detail::OTHERS_ALL; access_rights sut = TEST_VALUE; @@ -674,8 +716,8 @@ TEST(filesystem_test, permsBinaryExclusiveOrAssignmentEqualToBinaryExclusiveOrAs constexpr access_rights TEST_VALUE = perms::none; constexpr access_rights TEST_VALUE_RHS = perms::owner_all; - auto sutBaseValue = TEST_VALUE.value(); - constexpr auto BASE_VALUE_RHS = TEST_VALUE_RHS.value(); + auto sutBaseValue = iox::access_rights::detail::NONE; + constexpr auto BASE_VALUE_RHS = iox::access_rights::detail::OWNER_ALL; access_rights sut = TEST_VALUE; From 310db674ed230f8dfaa1347a9ec824a35a24e7ca Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Mon, 20 Nov 2023 21:43:28 +0100 Subject: [PATCH 4/6] iox-#2108 Remove friend declaration to 'FileManagementInterface' --- iceoryx_hoofs/filesystem/include/iox/filesystem.hpp | 2 -- .../design/include/iox/detail/file_management_interface.inl | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp index e57b4e3bc1..f6e7c991ce 100644 --- a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp +++ b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp @@ -189,8 +189,6 @@ class access_rights final friend constexpr access_rights operator^=(const access_rights lhs, const access_rights rhs) noexcept; private: - template - friend struct FileManagementInterface; explicit constexpr access_rights(value_type value) noexcept : m_value(value) { diff --git a/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl b/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl index f68eefb2e2..05181fd0d7 100644 --- a/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl +++ b/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl @@ -45,8 +45,8 @@ inline expected FileManagementInterface:: // st_mode also contains the file type, since we only would like to acquire the permissions // we have to remove the file type - constexpr uint16_t ONLY_FILE_PERMISSIONS = 0777; - return ok(access_rights(static_cast(result->st_mode & ONLY_FILE_PERMISSIONS))); + auto permissions_only = static_cast(result->st_mode & iox::perms::all.value()); + return ok(access_rights::unsafe_from_value_unchecked(permissions_only)); } template From 023d9b24a1dd822aa6da1a2d3cc2033c00aa4d4e Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Mon, 20 Nov 2023 21:49:26 +0100 Subject: [PATCH 5/6] iox-#2108 Update release notes --- doc/website/release-notes/iceoryx-unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/website/release-notes/iceoryx-unreleased.md b/doc/website/release-notes/iceoryx-unreleased.md index 775fa0968b..83a446460b 100644 --- a/doc/website/release-notes/iceoryx-unreleased.md +++ b/doc/website/release-notes/iceoryx-unreleased.md @@ -107,6 +107,7 @@ - FixedPositionContainer fails to compile on QNX QCC [#2084](https://github.com/eclipse-iceoryx/iceoryx/issues/2084) - Chunk fails to be released when more than 4 GiB of chunks have been allocated [#2087](https://github.com/eclipse-iceoryx/iceoryx/issues/2087) - Fix clang-tidy errors from full-scan nightly build [#2060](https://github.com/eclipse-iceoryx/iceoryx/issues/2060) +- Add public functions to create an 'access_rights' object from integer values [#2108](https://github.com/eclipse-iceoryx/iceoryx/issues/2108) **Refactoring:** From 7dd7cab5846008511dca25e3b6f66724dc077de8 Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Wed, 22 Nov 2023 15:31:53 +0100 Subject: [PATCH 6/6] iox-#2108 Remove 'unsafe_from_value_unchecked' and refactor 'from_value_sanitized' --- .../include/iox/detail/filesystem.inl | 10 --- .../filesystem/include/iox/filesystem.hpp | 90 +++++++++++-------- .../iox/detail/file_management_interface.inl | 2 +- .../test_filesystem_filesystem.cpp | 27 +----- 4 files changed, 53 insertions(+), 76 deletions(-) diff --git a/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl b/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl index 91959e11fd..52f72afa50 100644 --- a/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl +++ b/iceoryx_hoofs/filesystem/include/iox/detail/filesystem.inl @@ -1,5 +1,4 @@ // Copyright (c) 2022 by Apex.AI Inc. All rights reserved. -// Copyright (c) 2023 by Mathias Kraus . 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. @@ -189,15 +188,6 @@ inline bool doesEndWithPathSeparator(const iox::string& name) no return false; } -inline constexpr access_rights access_rights::from_value_sanitized(const access_rights::value_type value) noexcept -{ - if ((value & detail::UNKNOWN) == detail::UNKNOWN) - { - return access_rights{detail::UNKNOWN}; - } - return access_rights{static_cast(value & detail::MASK)}; -} - inline constexpr access_rights::value_type access_rights::value() const noexcept { return m_value; diff --git a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp index f6e7c991ce..da55c29aa0 100644 --- a/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp +++ b/iceoryx_hoofs/filesystem/include/iox/filesystem.hpp @@ -118,22 +118,14 @@ class access_rights final ~access_rights() noexcept = default; /// @brief Creates an 'access_rights' object from a sanitized value, ensuring that only the bits defined in - /// 'iox::perms::mask' are set when the 'iox::perm::unknown' bit is not set. If the 'iox::perm::unknown' bit is set - /// all other bits will be reset. + /// 'iox::perms::mask' are set and all other bits are reset /// @param[in] value for the 'access_rights' /// @return the 'access_rights' object - static constexpr access_rights from_value_sanitized(const value_type value) noexcept; - - /// @brief Creates an 'access_rights' object from an unchecked value. The user has to ensure that only the bits - /// defined in 'iox::perms::mask' are set when the 'iox::perm::unknown' bit is not set. If the 'iox::perm::unknown' - /// bit is set all other bits should be reset. - /// @param[in] value for the 'access_rights' - /// @return the 'access_rights' object - static constexpr access_rights unsafe_from_value_unchecked(const value_type value) noexcept + static constexpr access_rights from_value_sanitized(const value_type value) noexcept { // the code cannot be moved to the '*.inl' file since the function is used in the 'perms' namespace to define // the 'access_rights' constants - return access_rights{value}; + return access_rights{static_cast(value & detail::MASK)}; } constexpr value_type value() const noexcept; @@ -167,12 +159,16 @@ class access_rights final static constexpr value_type MASK{07777}; - // intentionally different from 'std::filesystem::perms::unknown' to prevent unexpected results. Combining a - // permission set to 'std::filesystem::perms::unknown' with other permission flags using bitwise OR may result - // in a value that is not representative of a valid permission state. By setting the value to '0x8000' only the - // MSB is 1 and all bits representing permissions are set to 0 and a bitwise OR will therefore also always - // result in a 0. - static constexpr value_type UNKNOWN{0x8000U}; + static constexpr access_rights unknown() noexcept + { + // Intentionally different from 'std::filesystem::perms::unknown' (which is 0xFFFF) to prevent unexpected + // results. Combining a permission set to 'std::filesystem::perms::unknown' with other permission flags + // using bitwise AND may have an unexpected result, e.g. 'std::filesystem::perms::unknown & + // std::filesystem::perms::mask' results in having all permission flags set to 1. By using '0x8000' only the + // MSB is 1 and all permission bits are set to 0 and a bitwise AND will therefore also always result in a 0. + constexpr value_type UNKNOWN{0x8000U}; + return access_rights{UNKNOWN}; + } // AXIVION ENABLE STYLE AutosarC++19_03-M2.13.2 }; @@ -203,55 +199,71 @@ namespace perms // AXIVION DISABLE STYLE AutosarC++19_03-A2.10.5 : Name reuse is intentional since they refer to the same value. Additionally, different namespaces are used. /// @brief Deny everything -static constexpr auto none{access_rights::unsafe_from_value_unchecked(access_rights::detail::NONE)}; +/// @note the underlying value is '0' +static constexpr auto none{access_rights::from_value_sanitized(access_rights::detail::NONE)}; /// @brief owner has read permission -static constexpr auto owner_read{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_READ)}; +/// @note the underlying value is '0o400' +static constexpr auto owner_read{access_rights::from_value_sanitized(access_rights::detail::OWNER_READ)}; /// @brief owner has write permission -static constexpr auto owner_write{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_WRITE)}; +/// @note the underlying value is '0o200' +static constexpr auto owner_write{access_rights::from_value_sanitized(access_rights::detail::OWNER_WRITE)}; /// @brief owner has execution permission -static constexpr auto owner_exec{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_EXEC)}; +/// @note the underlying value is '0o100' +static constexpr auto owner_exec{access_rights::from_value_sanitized(access_rights::detail::OWNER_EXEC)}; /// @brief owner has all permissions -static constexpr auto owner_all{access_rights::unsafe_from_value_unchecked(access_rights::detail::OWNER_ALL)}; +/// @note the underlying value is '0o700' +static constexpr auto owner_all{access_rights::from_value_sanitized(access_rights::detail::OWNER_ALL)}; /// @brief group has read permission -static constexpr auto group_read{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_READ)}; +/// @note the underlying value is '0o040' +static constexpr auto group_read{access_rights::from_value_sanitized(access_rights::detail::GROUP_READ)}; /// @brief group has write permission -static constexpr auto group_write{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_WRITE)}; +/// @note the underlying value is '0o020' +static constexpr auto group_write{access_rights::from_value_sanitized(access_rights::detail::GROUP_WRITE)}; /// @brief group has execution permission -static constexpr auto group_exec{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_EXEC)}; +/// @note the underlying value is '0o010' +static constexpr auto group_exec{access_rights::from_value_sanitized(access_rights::detail::GROUP_EXEC)}; /// @brief group has all permissions -static constexpr auto group_all{access_rights::unsafe_from_value_unchecked(access_rights::detail::GROUP_ALL)}; +/// @note the underlying value is '0o070' +static constexpr auto group_all{access_rights::from_value_sanitized(access_rights::detail::GROUP_ALL)}; /// @brief others have read permission -static constexpr auto others_read{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_READ)}; +/// @note the underlying value is '0o004' +static constexpr auto others_read{access_rights::from_value_sanitized(access_rights::detail::OTHERS_READ)}; /// @brief others have write permission -static constexpr auto others_write{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_WRITE)}; +/// @note the underlying value is '0o002' +static constexpr auto others_write{access_rights::from_value_sanitized(access_rights::detail::OTHERS_WRITE)}; /// @brief others have execution permission -static constexpr auto others_exec{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_EXEC)}; +/// @note the underlying value is '0o001' +static constexpr auto others_exec{access_rights::from_value_sanitized(access_rights::detail::OTHERS_EXEC)}; /// @brief others have all permissions -static constexpr auto others_all{access_rights::unsafe_from_value_unchecked(access_rights::detail::OTHERS_ALL)}; +/// @note the underlying value is '0o007' +static constexpr auto others_all{access_rights::from_value_sanitized(access_rights::detail::OTHERS_ALL)}; /// @brief all permissions for everyone -static constexpr auto all{access_rights::unsafe_from_value_unchecked(access_rights::detail::ALL)}; +/// @note the underlying value is '0o777' +static constexpr auto all{access_rights::from_value_sanitized(access_rights::detail::ALL)}; /// @brief set uid bit -/// @note introduction into setgit/setuid: https://en.wikipedia.org/wiki/Setuid +/// @note the underlying value is '0o4000'; introduction into setgit/setuid: https://en.wikipedia.org/wiki/Setuid // AXIVION Next Construct AutosarC++19_03-M2.10.1: The constant is in a namespace and mimics the C++17 STL equivalent -static constexpr auto set_uid{access_rights::unsafe_from_value_unchecked(access_rights::detail::SET_UID)}; +static constexpr auto set_uid{access_rights::from_value_sanitized(access_rights::detail::SET_UID)}; /// @brief set gid bit -/// @note introduction into setgit/setuid: https://en.wikipedia.org/wiki/Setuid +/// @note the underlying value is '0o2000'; introduction into setgit/setuid: https://en.wikipedia.org/wiki/Setuid // AXIVION Next Construct AutosarC++19_03-M2.10.1: The constant is in a namespace and mimics the C++17 STL equivalent -static constexpr auto set_gid{access_rights::unsafe_from_value_unchecked(access_rights::detail::SET_GID)}; +static constexpr auto set_gid{access_rights::from_value_sanitized(access_rights::detail::SET_GID)}; /// @brief set sticky bit -/// @note sticky bit introduction: https://en.wikipedia.org/wiki/Sticky_bit -static constexpr auto sticky_bit{access_rights::unsafe_from_value_unchecked(access_rights::detail::STICKY_BIT)}; +/// @note the underlying value is '0o1000'; sticky bit introduction: https://en.wikipedia.org/wiki/Sticky_bit +static constexpr auto sticky_bit{access_rights::from_value_sanitized(access_rights::detail::STICKY_BIT)}; /// @brief all permissions for everyone as well as uid, gid and sticky bit -static constexpr auto mask{access_rights::unsafe_from_value_unchecked(access_rights::detail::MASK)}; +/// @note the underlying value is '0o7777' +static constexpr auto mask{access_rights::from_value_sanitized(access_rights::detail::MASK)}; /// @brief unknown permissions -static constexpr auto unknown{access_rights::unsafe_from_value_unchecked(access_rights::detail::UNKNOWN)}; +/// @note the underlying value is '0x8000' +static constexpr auto unknown{access_rights::detail::unknown()}; // AXIVION ENABLE STYLE AutosarC++19_03-A2.10.5 } // namespace perms diff --git a/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl b/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl index 05181fd0d7..ccf2761c3c 100644 --- a/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl +++ b/iceoryx_hoofs/posix/design/include/iox/detail/file_management_interface.inl @@ -46,7 +46,7 @@ inline expected FileManagementInterface:: // st_mode also contains the file type, since we only would like to acquire the permissions // we have to remove the file type auto permissions_only = static_cast(result->st_mode & iox::perms::all.value()); - return ok(access_rights::unsafe_from_value_unchecked(permissions_only)); + return ok(access_rights::from_value_sanitized(permissions_only)); } template diff --git a/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp b/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp index 8d9dced0e9..500b1c19f2 100644 --- a/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp +++ b/iceoryx_hoofs/test/moduletests/test_filesystem_filesystem.cpp @@ -591,23 +591,6 @@ TEST(filesystem_test_isValidPathEntry, StringWithRelativeComponentsIsInvalidWhen iox::RelativePathComponents::REJECT)); } -TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueUnknown) -{ - ::testing::Test::RecordProperty("TEST_ID", "6e510e0e-1a0c-40a3-bab0-941a78d745d8"); - constexpr auto TEST_VALUE = access_rights::detail::UNKNOWN; - - EXPECT_THAT(access_rights::from_value_sanitized(TEST_VALUE).value(), Eq(TEST_VALUE)); -} - -TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueUnknownWithExtraBitsSet) -{ - ::testing::Test::RecordProperty("TEST_ID", "1af33cc5-c870-4fef-ad23-6525296d6792"); - constexpr auto TEST_VALUE_SANITIZED = access_rights::detail::UNKNOWN; - constexpr auto TEST_VALUE = TEST_VALUE_SANITIZED | static_cast(0x0101U); - - EXPECT_THAT(access_rights::from_value_sanitized(TEST_VALUE).value(), Eq(TEST_VALUE_SANITIZED)); -} - TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueInRangeOfPermsMask) { ::testing::Test::RecordProperty("TEST_ID", "5a6c2ece-9cd7-4779-ad0b-16372aebd407"); @@ -619,20 +602,12 @@ TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueInRangeOfPermsM TEST(filesystem_test, accessRightsFromValueSanitizedWorksForValueOutOfRangeOfPermsMask) { ::testing::Test::RecordProperty("TEST_ID", "8a291709-a75c-4afa-96d8-d57a0d40696c"); - constexpr auto TEST_VALUE_SANITIZED = access_rights::detail::OWNER_EXEC; + constexpr auto TEST_VALUE_SANITIZED = access_rights::detail::OWNER_WRITE; constexpr auto TEST_VALUE = TEST_VALUE_SANITIZED | static_cast(010000); EXPECT_THAT(access_rights::from_value_sanitized(TEST_VALUE).value(), Eq(TEST_VALUE_SANITIZED)); } -TEST(filesystem_test, accessRightsUnsafeFromValueUncheckedWorkForAnyValue) -{ - ::testing::Test::RecordProperty("TEST_ID", "5f34cef3-f778-4eed-bb78-42decdb8f7bb"); - constexpr auto TEST_VALUE = access_rights::detail::OWNER_WRITE | static_cast(010000); - - EXPECT_THAT(access_rights::unsafe_from_value_unchecked(TEST_VALUE).value(), Eq(TEST_VALUE)); -} - TEST(filesystem_test, permsBinaryOrEqualToBinaryOrOfUnderlyingType) { ::testing::Test::RecordProperty("TEST_ID", "0b72fcec-c2b3-4a45-801f-542ff3195a2f");