Merge branch 'null_terminated' into 'master'

Improve format workaround and add unit tests

See merge request OpenMW/openmw!2338

apps/openmw_test_suite/misc/test_stringops.cpp

@@ -182,4 +182,26 @@ namespace
     {
         EXPECT_FALSE(ciStartsWith("foo", "foo bar"));
     }
+
+    TEST(MiscStringsFormat, string_format)
+    {
+        std::string f = "1%s2";
+        EXPECT_EQ(Misc::StringUtils::format(f, ""), "12");
+    }
+
+    TEST(MiscStringsFormat, string_format_arg)
+    {
+        std::string arg = "12";
+        EXPECT_EQ(Misc::StringUtils::format("1%s2", arg), "1122");
+    }
+
+    TEST(MiscStringsFormat, string_view_format_arg)
+    {
+        std::string f = "1%s2";
+        std::string_view view = "12";
+        EXPECT_EQ(Misc::StringUtils::format(f, view), "1122");
+        EXPECT_EQ(Misc::StringUtils::format(f, view.substr(0, 1)), "112");
+        EXPECT_EQ(Misc::StringUtils::format(f, view.substr(1, 1)), "122");
+        EXPECT_EQ(Misc::StringUtils::format(f, view.substr(2)), "12");
+    }
 }

components/misc/strings/format.hpp

@@ -11,31 +11,35 @@
 
 namespace Misc::StringUtils
 {
-    struct Details
+    namespace Details
     {
-        std::vector<std::string> mMemorySafety;
-
         // Allow to convert complex arguments to C-style strings for format() function
         template <typename T>
         T argument(T value) noexcept
         {
+            static_assert(!std::is_same_v<T, std::string_view>, "std::string_view is not supported");
             return value;
         }
 
-        template <typename T>
-        T const * argument(std::basic_string_view<T> const & value) noexcept
-        {
-            // TODO: switch to a format function that doesn't require null termination
-            auto& inserted = mMemorySafety.emplace_back(value);
-            return inserted.c_str();
-        }
-
         template <typename T>
         T const * argument(std::basic_string<T> const & value) noexcept
         {
             return value.c_str();
         }
-    };
+
+        template<class T>
+        T nullTerminated(T value) noexcept
+        {
+            return value;
+        }
+
+        template<class T>
+        std::basic_string<T> nullTerminated(const std::basic_string_view<T>& value) noexcept
+        {
+            // Ensure string_view arguments are null-terminated by creating a string
+            // TODO: Use a format function that doesn't require this workaround
+            return std::string{value};
+        }
 
         // Requires some C++11 features:
         // 1. std::string needs to be contiguous
@@ -44,22 +48,28 @@ namespace Misc::StringUtils
         template <typename ... Args>
         std::string format(const char* fmt, Args const & ... args)
         {
-        Details details;
+            const int size = std::snprintf(nullptr, 0, fmt, argument(args) ...);
-        const int size = std::snprintf(nullptr, 0, fmt, details.argument(args) ...);
             if (size < 0)
                 throw std::runtime_error(std::string("Failed to compute resulting string size: ") + std::strerror(errno));
             // Note: sprintf also writes a trailing null character. We should remove it.
             std::string ret(static_cast<std::size_t>(size) + 1, '\0');
-        if (std::sprintf(ret.data(), fmt, details.argument(args) ...) < 0)
+            if (std::sprintf(ret.data(), fmt, argument(args) ...) < 0)
                 throw std::runtime_error(std::string("Failed to format string: ") + std::strerror(errno));
             ret.erase(static_cast<std::size_t>(size));
             return ret;
         }
+    }
+
+    template <typename ... Args>
+    std::string format(const char* fmt, Args const & ... args)
+    {
+        return Details::format(fmt, Details::nullTerminated(args) ...);
+    }
 
     template <typename ... Args>
     std::string format(const std::string& fmt, Args const & ... args)
     {
-        return format(fmt.c_str(), args ...);
+        return Details::format(fmt.c_str(), Details::nullTerminated(args) ...);
     }
 }