From 7c609052668df7f376b4de30fb7bf1ad4561377b Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 2 Jan 2023 17:54:14 +0100 Subject: [PATCH 1/3] Put ESM::FixedString tests into unnamed namespace --- .../esm/test_fixed_string.cpp | 261 +++++++++--------- 1 file changed, 132 insertions(+), 129 deletions(-) diff --git a/apps/openmw_test_suite/esm/test_fixed_string.cpp b/apps/openmw_test_suite/esm/test_fixed_string.cpp index 078cb8609e..bcf776ca51 100644 --- a/apps/openmw_test_suite/esm/test_fixed_string.cpp +++ b/apps/openmw_test_suite/esm/test_fixed_string.cpp @@ -2,142 +2,145 @@ #include "components/esm/esmcommon.hpp" #include -TEST(EsmFixedString, operator__eq_ne) +namespace { + TEST(EsmFixedString, operator__eq_ne) { - SCOPED_TRACE("asdc == asdc"); - constexpr ESM::NAME name("asdc"); - char s[4] = { 'a', 's', 'd', 'c' }; - std::string ss(s, 4); + { + SCOPED_TRACE("asdc == asdc"); + constexpr ESM::NAME name("asdc"); + char s[4] = { 'a', 's', 'd', 'c' }; + std::string ss(s, 4); - EXPECT_TRUE(name == s); - EXPECT_TRUE(name == ss.c_str()); - EXPECT_TRUE(name == ss); + EXPECT_TRUE(name == s); + EXPECT_TRUE(name == ss.c_str()); + EXPECT_TRUE(name == ss); + } + { + SCOPED_TRACE("asdc == asdcx"); + constexpr ESM::NAME name("asdc"); + char s[5] = { 'a', 's', 'd', 'c', 'x' }; + std::string ss(s, 5); + + EXPECT_TRUE(name != s); + EXPECT_TRUE(name != ss.c_str()); + EXPECT_TRUE(name != ss); + } + { + SCOPED_TRACE("asdc == asdc[NULL]"); + const ESM::NAME name("asdc"); + char s[5] = { 'a', 's', 'd', 'c', '\0' }; + std::string ss(s, 5); + + EXPECT_TRUE(name == s); + EXPECT_TRUE(name == ss.c_str()); + EXPECT_TRUE(name == ss); + } } + TEST(EsmFixedString, operator__eq_ne_const) { - SCOPED_TRACE("asdc == asdcx"); - constexpr ESM::NAME name("asdc"); - char s[5] = { 'a', 's', 'd', 'c', 'x' }; - std::string ss(s, 5); + { + SCOPED_TRACE("asdc == asdc (const)"); + constexpr ESM::NAME name("asdc"); + const char s[4] = { 'a', 's', 'd', 'c' }; + std::string ss(s, 4); - EXPECT_TRUE(name != s); - EXPECT_TRUE(name != ss.c_str()); - EXPECT_TRUE(name != ss); + EXPECT_TRUE(name == s); + EXPECT_TRUE(name == ss.c_str()); + EXPECT_TRUE(name == ss); + } + { + SCOPED_TRACE("asdc == asdcx (const)"); + constexpr ESM::NAME name("asdc"); + const char s[5] = { 'a', 's', 'd', 'c', 'x' }; + std::string ss(s, 5); + + EXPECT_TRUE(name != s); + EXPECT_TRUE(name != ss.c_str()); + EXPECT_TRUE(name != ss); + } + { + SCOPED_TRACE("asdc == asdc[NULL] (const)"); + constexpr ESM::NAME name("asdc"); + const char s[5] = { 'a', 's', 'd', 'c', '\0' }; + std::string ss(s, 5); + + EXPECT_TRUE(name == s); + EXPECT_TRUE(name == ss.c_str()); + EXPECT_TRUE(name == ss); + } } - { - SCOPED_TRACE("asdc == asdc[NULL]"); - const ESM::NAME name("asdc"); - char s[5] = { 'a', 's', 'd', 'c', '\0' }; - std::string ss(s, 5); - EXPECT_TRUE(name == s); - EXPECT_TRUE(name == ss.c_str()); - EXPECT_TRUE(name == ss); + TEST(EsmFixedString, empty_strings) + { + { + SCOPED_TRACE("4 bytes"); + ESM::NAME empty = ESM::NAME(); + EXPECT_TRUE(empty == ""); + EXPECT_TRUE(empty == static_cast(0)); + EXPECT_TRUE(empty != "some string"); + EXPECT_TRUE(empty != static_cast(42)); + } + { + SCOPED_TRACE("32 bytes"); + ESM::NAME32 empty = ESM::NAME32(); + EXPECT_TRUE(empty == ""); + EXPECT_TRUE(empty != "some string"); + } + } + + TEST(EsmFixedString, assign_should_zero_untouched_bytes_for_4) + { + ESM::NAME value; + value = static_cast(0xFFFFFFFFu); + value.assign(std::string(1, 'a')); + EXPECT_EQ(value, static_cast('a')) << value.toInt(); + } + + TEST(EsmFixedString, assign_should_only_truncate_for_4) + { + ESM::NAME value; + value.assign(std::string(5, 'a')); + EXPECT_EQ(value, std::string(4, 'a')); + } + + TEST(EsmFixedString, assign_should_truncate_and_set_last_element_to_zero) + { + ESM::FixedString<17> value; + value.assign(std::string(20, 'a')); + EXPECT_EQ(value, std::string(16, 'a')); + } + + TEST(EsmFixedString, assign_should_truncate_and_set_last_element_to_zero_for_32) + { + ESM::NAME32 value; + value.assign(std::string(33, 'a')); + EXPECT_EQ(value, std::string(31, 'a')); + } + + TEST(EsmFixedString, assign_should_truncate_and_set_last_element_to_zero_for_64) + { + ESM::NAME64 value; + value.assign(std::string(65, 'a')); + EXPECT_EQ(value, std::string(63, 'a')); + } + + TEST(EsmFixedString, assignment_operator_is_supported_for_uint32) + { + ESM::NAME value; + value = static_cast(0xFEDCBA98u); + EXPECT_EQ(value, static_cast(0xFEDCBA98u)) << value.toInt(); + } + + TEST(EsmFixedString, construction_from_uint32_is_supported) + { + constexpr ESM::NAME value(0xFEDCBA98u); + EXPECT_EQ(value, static_cast(0xFEDCBA98u)) << value.toInt(); + } + + TEST(EsmFixedString, construction_from_RecNameInts_is_supported) + { + constexpr ESM::NAME value(ESM::RecNameInts::REC_ACTI); + EXPECT_EQ(value, static_cast(ESM::RecNameInts::REC_ACTI)) << value.toInt(); } } -TEST(EsmFixedString, operator__eq_ne_const) -{ - { - SCOPED_TRACE("asdc == asdc (const)"); - constexpr ESM::NAME name("asdc"); - const char s[4] = { 'a', 's', 'd', 'c' }; - std::string ss(s, 4); - - EXPECT_TRUE(name == s); - EXPECT_TRUE(name == ss.c_str()); - EXPECT_TRUE(name == ss); - } - { - SCOPED_TRACE("asdc == asdcx (const)"); - constexpr ESM::NAME name("asdc"); - const char s[5] = { 'a', 's', 'd', 'c', 'x' }; - std::string ss(s, 5); - - EXPECT_TRUE(name != s); - EXPECT_TRUE(name != ss.c_str()); - EXPECT_TRUE(name != ss); - } - { - SCOPED_TRACE("asdc == asdc[NULL] (const)"); - constexpr ESM::NAME name("asdc"); - const char s[5] = { 'a', 's', 'd', 'c', '\0' }; - std::string ss(s, 5); - - EXPECT_TRUE(name == s); - EXPECT_TRUE(name == ss.c_str()); - EXPECT_TRUE(name == ss); - } -} - -TEST(EsmFixedString, empty_strings) -{ - { - SCOPED_TRACE("4 bytes"); - ESM::NAME empty = ESM::NAME(); - EXPECT_TRUE(empty == ""); - EXPECT_TRUE(empty == static_cast(0)); - EXPECT_TRUE(empty != "some string"); - EXPECT_TRUE(empty != static_cast(42)); - } - { - SCOPED_TRACE("32 bytes"); - ESM::NAME32 empty = ESM::NAME32(); - EXPECT_TRUE(empty == ""); - EXPECT_TRUE(empty != "some string"); - } -} - -TEST(EsmFixedString, assign_should_zero_untouched_bytes_for_4) -{ - ESM::NAME value; - value = static_cast(0xFFFFFFFFu); - value.assign(std::string(1, 'a')); - EXPECT_EQ(value, static_cast('a')) << value.toInt(); -} - -TEST(EsmFixedString, assign_should_only_truncate_for_4) -{ - ESM::NAME value; - value.assign(std::string(5, 'a')); - EXPECT_EQ(value, std::string(4, 'a')); -} - -TEST(EsmFixedString, assign_should_truncate_and_set_last_element_to_zero) -{ - ESM::FixedString<17> value; - value.assign(std::string(20, 'a')); - EXPECT_EQ(value, std::string(16, 'a')); -} - -TEST(EsmFixedString, assign_should_truncate_and_set_last_element_to_zero_for_32) -{ - ESM::NAME32 value; - value.assign(std::string(33, 'a')); - EXPECT_EQ(value, std::string(31, 'a')); -} - -TEST(EsmFixedString, assign_should_truncate_and_set_last_element_to_zero_for_64) -{ - ESM::NAME64 value; - value.assign(std::string(65, 'a')); - EXPECT_EQ(value, std::string(63, 'a')); -} - -TEST(EsmFixedString, assignment_operator_is_supported_for_uint32) -{ - ESM::NAME value; - value = static_cast(0xFEDCBA98u); - EXPECT_EQ(value, static_cast(0xFEDCBA98u)) << value.toInt(); -} - -TEST(EsmFixedString, construction_from_uint32_is_supported) -{ - constexpr ESM::NAME value(0xFEDCBA98u); - EXPECT_EQ(value, static_cast(0xFEDCBA98u)) << value.toInt(); -} - -TEST(EsmFixedString, construction_from_RecNameInts_is_supported) -{ - constexpr ESM::NAME value(ESM::RecNameInts::REC_ACTI); - EXPECT_EQ(value, static_cast(ESM::RecNameInts::REC_ACTI)) << value.toInt(); -} From c80ba92ab72391f98eb42491db102707ace3f874 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 2 Jan 2023 17:21:36 +0100 Subject: [PATCH 2/3] Add more tests for ESM::FixedString --- .../esm/test_fixed_string.cpp | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/apps/openmw_test_suite/esm/test_fixed_string.cpp b/apps/openmw_test_suite/esm/test_fixed_string.cpp index bcf776ca51..f3d4071c28 100644 --- a/apps/openmw_test_suite/esm/test_fixed_string.cpp +++ b/apps/openmw_test_suite/esm/test_fixed_string.cpp @@ -37,6 +37,7 @@ namespace EXPECT_TRUE(name == ss); } } + TEST(EsmFixedString, operator__eq_ne_const) { { @@ -143,4 +144,30 @@ namespace constexpr ESM::NAME value(ESM::RecNameInts::REC_ACTI); EXPECT_EQ(value, static_cast(ESM::RecNameInts::REC_ACTI)) << value.toInt(); } + + TEST(EsmFixedString, equality_operator_for_not_convertible_to_uint32_with_string_literal) + { + const ESM::FixedString<5> value("abcd"); + EXPECT_EQ(value, "abcd"); + } + + TEST(EsmFixedString, equality_operator_for_not_convertible_to_uint32_with_fixed_size_char_array) + { + const ESM::FixedString<5> value("abcd"); + const char other[5] = { 'a', 'b', 'c', 'd', '\0' }; + EXPECT_EQ(value, other); + } + + TEST(EsmFixedString, equality_operator_for_not_convertible_to_uint32_with_const_char_pointer) + { + const ESM::FixedString<5> value("abcd"); + const char other[5] = { 'a', 'b', 'c', 'd', '\0' }; + EXPECT_EQ(value, static_cast(other)); + } + + TEST(EsmFixedString, equality_operator_for_not_convertible_to_uint32_with_string) + { + const ESM::FixedString<5> value("abcd"); + EXPECT_EQ(value, std::string("abcd")); + } } From 2bbed8cc0698e946a084425d8762fb89685b80f7 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 2 Jan 2023 17:03:03 +0100 Subject: [PATCH 3/3] =?UTF-8?q?Fix=20gcc=20warning:=20array=20subscript=20?= =?UTF-8?q?5=20is=20outside=20array=20bounds=20of=20=E2=80=98const=20char?= =?UTF-8?q?=20[5]=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In function ‘bool ESM::operator==(const FixedString&, const T* const&) [with long unsigned int capacity = 5; T = char; = void]’, inlined from ‘testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = ESM::FixedString<5>; T2 = const char*]’ at /home/elsid/dev/googletest/build/gcc/release/install/include/gtest/gtest.h:1358:11, inlined from ‘static testing::AssertionResult testing::internal::EqHelper::Compare(const char*, const char*, const T1&, const T2&) [with T1 = ESM::FixedString<5>; T2 = const char*; typename std::enable_if<((! std::is_integral<_Tp>::value) || (! std::is_pointer<_Dp>::value))>::type* = 0]’ at /home/elsid/dev/googletest/build/gcc/release/install/include/gtest/gtest.h:1377:64, inlined from ‘virtual void {anonymous}::EsmFixedString_equality_operator_for_not_convertible_to_uint32_with_const_char_pointer_Test::TestBody()’ at apps/openmw_test_suite/esm/test_fixed_string.cpp:165:9: components/esm/esmcommon.hpp:134:19: warning: array subscript 5 is outside array bounds of ‘const char [5]’ [-Warray-bounds] 134 | return rhs[capacity] == '\0'; | ~~~^ apps/openmw_test_suite/esm/test_fixed_string.cpp: In member function ‘virtual void {anonymous}::EsmFixedString_equality_operator_for_not_convertible_to_uint32_with_const_char_pointer_Test::TestBody()’: apps/openmw_test_suite/esm/test_fixed_string.cpp:164:20: note: at offset 5 into object ‘other’ of size 5 164 | const char other[5] = { 'a', 'b', 'c', 'd', '\0' }; | ^~~~~ --- .../esm/test_fixed_string.cpp | 15 ++++++++++++++ components/esm/esmcommon.hpp | 20 ++++++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/apps/openmw_test_suite/esm/test_fixed_string.cpp b/apps/openmw_test_suite/esm/test_fixed_string.cpp index f3d4071c28..76ed346daa 100644 --- a/apps/openmw_test_suite/esm/test_fixed_string.cpp +++ b/apps/openmw_test_suite/esm/test_fixed_string.cpp @@ -170,4 +170,19 @@ namespace const ESM::FixedString<5> value("abcd"); EXPECT_EQ(value, std::string("abcd")); } + + TEST(EsmFixedString, equality_operator_for_not_convertible_to_uint32_with_string_view) + { + const ESM::FixedString<5> value("abcd"); + const std::string other("abcd"); + EXPECT_EQ(value, std::string_view(other)); + } + + TEST(EsmFixedString, equality_operator_should_not_get_out_of_bounds) + { + ESM::FixedString<5> value; + const char other[5] = { 'a', 'b', 'c', 'd', 'e' }; + std::memcpy(value.mData, other, sizeof(other)); + EXPECT_EQ(value, static_cast(other)); + } } diff --git a/components/esm/esmcommon.hpp b/components/esm/esmcommon.hpp index f272fa553e..e12a4703ce 100644 --- a/components/esm/esmcommon.hpp +++ b/components/esm/esmcommon.hpp @@ -121,23 +121,23 @@ namespace ESM } }; - template >> - inline bool operator==(const FixedString& lhs, const T* const& rhs) noexcept + template + inline bool operator==(const FixedString& lhs, std::string_view rhs) noexcept { - for (std::size_t i = 0; i < capacity; ++i) + for (std::size_t i = 0, n = std::min(rhs.size(), capacity); i < n; ++i) { if (lhs.mData[i] != rhs[i]) return false; if (lhs.mData[i] == '\0') return true; } - return rhs[capacity] == '\0'; + return rhs.size() <= capacity || rhs[capacity] == '\0'; } - template - inline bool operator==(const FixedString& lhs, const std::string& rhs) noexcept + template >> + inline bool operator==(const FixedString& lhs, const T* const& rhs) noexcept { - return lhs == rhs.c_str(); + return lhs == std::string_view(rhs, capacity); } template @@ -156,6 +156,12 @@ namespace ESM return lhs.toInt() == rhs.toInt(); } + template >> + inline bool operator==(const FixedString<4>& lhs, const T* const& rhs) noexcept + { + return lhs == std::string_view(rhs, 5); + } + template inline bool operator!=(const FixedString& lhs, const Rhs& rhs) noexcept {