Merge branch 'fix_esm_fixed_string_warning' into 'master'

Fix gcc warning: array subscript n is outside array bounds of ‘const char [n]’

See merge request OpenMW/openmw!2585

apps/openmw_test_suite/esm/test_fixed_string.cpp

@@ -2,8 +2,10 @@
 #include "components/esm/esmcommon.hpp"
 #include <gtest/gtest.h>
 
-TEST(EsmFixedString, operator__eq_ne)
+namespace
 {
+    TEST(EsmFixedString, operator__eq_ne)
+    {
         {
             SCOPED_TRACE("asdc == asdc");
             constexpr ESM::NAME name("asdc");
@@ -34,9 +36,10 @@ TEST(EsmFixedString, operator__eq_ne)
             EXPECT_TRUE(name == ss.c_str());
             EXPECT_TRUE(name == ss);
         }
-}
+    }
-TEST(EsmFixedString, operator__eq_ne_const)
+
-{
+    TEST(EsmFixedString, operator__eq_ne_const)
+    {
         {
             SCOPED_TRACE("asdc == asdc (const)");
             constexpr ESM::NAME name("asdc");
@@ -67,10 +70,10 @@ TEST(EsmFixedString, operator__eq_ne_const)
             EXPECT_TRUE(name == ss.c_str());
             EXPECT_TRUE(name == ss);
         }
-}
+    }
 
-TEST(EsmFixedString, empty_strings)
+    TEST(EsmFixedString, empty_strings)
-{
+    {
         {
             SCOPED_TRACE("4 bytes");
             ESM::NAME empty = ESM::NAME();
@@ -85,59 +88,101 @@ TEST(EsmFixedString, empty_strings)
             EXPECT_TRUE(empty == "");
             EXPECT_TRUE(empty != "some string");
         }
-}
+    }
 
-TEST(EsmFixedString, assign_should_zero_untouched_bytes_for_4)
+    TEST(EsmFixedString, assign_should_zero_untouched_bytes_for_4)
-{
+    {
         ESM::NAME value;
         value = static_cast<uint32_t>(0xFFFFFFFFu);
         value.assign(std::string(1, 'a'));
         EXPECT_EQ(value, static_cast<uint32_t>('a')) << value.toInt();
-}
+    }
 
-TEST(EsmFixedString, assign_should_only_truncate_for_4)
+    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)
+    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)
+    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)
+    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)
+    TEST(EsmFixedString, assignment_operator_is_supported_for_uint32)
-{
+    {
         ESM::NAME value;
         value = static_cast<uint32_t>(0xFEDCBA98u);
         EXPECT_EQ(value, static_cast<uint32_t>(0xFEDCBA98u)) << value.toInt();
-}
+    }
 
-TEST(EsmFixedString, construction_from_uint32_is_supported)
+    TEST(EsmFixedString, construction_from_uint32_is_supported)
-{
+    {
         constexpr ESM::NAME value(0xFEDCBA98u);
         EXPECT_EQ(value, static_cast<std::uint32_t>(0xFEDCBA98u)) << value.toInt();
-}
+    }
 
-TEST(EsmFixedString, construction_from_RecNameInts_is_supported)
+    TEST(EsmFixedString, construction_from_RecNameInts_is_supported)
-{
+    {
         constexpr ESM::NAME value(ESM::RecNameInts::REC_ACTI);
         EXPECT_EQ(value, static_cast<std::uint32_t>(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<const char*>(other));
+    }
+
+    TEST(EsmFixedString, equality_operator_for_not_convertible_to_uint32_with_string)
+    {
+        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<const char*>(other));
+    }
 }

components/esm/esmcommon.hpp

@@ -121,23 +121,23 @@ namespace ESM
         }
     };
 
-    template <std::size_t capacity, class T, typename = std::enable_if_t<std::is_same_v<T, char>>>
+    template <std::size_t capacity>
-    inline bool operator==(const FixedString<capacity>& lhs, const T* const& rhs) noexcept
+    inline bool operator==(const FixedString<capacity>& 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 <std::size_t capacity>
+    template <std::size_t capacity, class T, typename = std::enable_if_t<std::is_same_v<T, char>>>
-    inline bool operator==(const FixedString<capacity>& lhs, const std::string& rhs) noexcept
+    inline bool operator==(const FixedString<capacity>& lhs, const T* const& rhs) noexcept
     {
-        return lhs == rhs.c_str();
+        return lhs == std::string_view(rhs, capacity);
     }
 
     template <std::size_t capacity, std::size_t rhsSize>
@@ -156,6 +156,12 @@ namespace ESM
         return lhs.toInt() == rhs.toInt();
     }
 
+    template <class T, typename = std::enable_if_t<std::is_same_v<T, char>>>
+    inline bool operator==(const FixedString<4>& lhs, const T* const& rhs) noexcept
+    {
+        return lhs == std::string_view(rhs, 5);
+    }
+
     template <std::size_t capacity, class Rhs>
     inline bool operator!=(const FixedString<capacity>& lhs, const Rhs& rhs) noexcept
     {