Ticket #3011: 0007-CLogger-Use-cppformat-instead-of-sys_vswprintf.patch

build/premake/premake4.lua

@@ -741 +741 @@
         "maths",
         "maths/scripting",
         "i18n",
-        "i18n/scripting"
+        "i18n/scripting",
+        "third_party/cppformat",
     }
     extern_libs = {
         "spidermonkey",

source/graphics/TextRenderer.cpp

@@ -144 +144 @@
     PutString(x, y, new std::wstring(buf), true);
 }
 
+void CTextRenderer::Put(float x, float y, const char* buf)
+{
+    if (buf[0] == 0)
+        return; // empty string; don't bother storing
+
+    PutString(x, y, new std::wstring(wstring_from_utf8(buf)), true);
+}
+
 void CTextRenderer::Put(float x, float y, const std::wstring* buf)
 {
     if (buf->empty())

source/graphics/TextRenderer.h

@@ -91 +91 @@
     /**
      * Print text at (x,y) under the current transform.
      * Does not alter the current transform.
+     * @p buf must be a UTF-8 string.
+     */
+    void Put(float x, float y, const char* buf);
+
+    /**
+     * Print text at (x,y) under the current transform.
+     * Does not alter the current transform.
      * @p buf must remain valid until Render() is called.
      * (This should be used to minimise memory copies when possible.)
      */

source/graphics/tests/test_MeshManager.h

@@ -190 +190 @@
 
         CModelDefPtr modeldef = meshManager->GetMesh(testDAE);
         TS_ASSERT(! modeldef);
-        TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"parser error");
+        TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "parser error");
     }
 
     void test_invalid_dae()
@@ -205 +205 @@
 
         CModelDefPtr modeldef = meshManager->GetMesh(testDAE);
         TS_ASSERT(! modeldef);
-        TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"parser error");
+        TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "parser error");
     }
 
     void test_load_nonexistent_pmd()

source/lib/self_test.h

@@ -269 +269 @@
 #define TS_ASSERT_PATH_EQUALS(path1, path2) TS_ASSERT_EQUALS((path1).string(), (path2).string())
 #define TSM_ASSERT_PATH_EQUALS(m, path1, path2) TSM_ASSERT_EQUALS(m, (path1).string(), (path2).string())
 
+bool ts_str_contains(const std::string& str1, const std::string& str2); // defined in test_setup.cpp
 bool ts_str_contains(const std::wstring& str1, const std::wstring& str2); // defined in test_setup.cpp
+#define TS_ASSERT_STR_CONTAINS(str1, str2) TSM_ASSERT(str1, ts_str_contains(str1, str2))
+#define TS_ASSERT_STR_NOT_CONTAINS(str1, str2) TSM_ASSERT(str1, !ts_str_contains(str1, str2))
 #define TS_ASSERT_WSTR_CONTAINS(str1, str2) TSM_ASSERT(str1, ts_str_contains(str1, str2))
 #define TS_ASSERT_WSTR_NOT_CONTAINS(str1, str2) TSM_ASSERT(str1, !ts_str_contains(str1, str2))
 

source/ps/CLogger.cpp

@@ -1 @@
-/* Copyright (C) 2012 Wildfire Games.
+/* Copyright (C) 2015 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -140 +140 @@
     }
 }
 
-static std::string ToHTML(const wchar_t* message)
+static std::string ToHTML(const char* message)
 {
-    Status err;
-    std::string cmessage = utf8_from_wstring(message, &err);
+    std::string cmessage = message;
     boost::algorithm::replace_all(cmessage, "&", "&");
     boost::algorithm::replace_all(cmessage, "<", "&lt;");
     return cmessage;
 }
 
-void CLogger::WriteMessage(const wchar_t* message, bool doRender = false)
+void CLogger::WriteMessage(const char* message, bool doRender = false)
 {
     std::string cmessage = ToHTML(message);
 
@@ -157 +156 @@
 
     ++m_NumberOfMessages;
 //  if (m_UseDebugPrintf)
-//      debug_printf(L"MESSAGE: %ls\n", message);
+//      debug_printf(L"MESSAGE: %hs\n", message);
 
     *m_MainLog << "<p>" << cmessage << "</p>\n";
     m_MainLog->flush();
     
     if (doRender)
     {
-        if (g_Console) g_Console->InsertMessage(L"INFO: %ls", message);
+        if (g_Console) g_Console->InsertMessage(L"INFO: %hs", message);
 
         PushRenderMessage(Normal, message);
     }
 }
 
-void CLogger::WriteError(const wchar_t* message)
+void CLogger::WriteError(const char* message)
 {
     std::string cmessage = ToHTML(message);
 
@@ -178 +177 @@
 
     ++m_NumberOfErrors;
     if (m_UseDebugPrintf)
-        debug_printf(L"ERROR: %ls\n", message);
+        debug_printf(L"ERROR: %hs\n", message);
 
-    if (g_Console) g_Console->InsertMessage(L"ERROR: %ls", message);
+    if (g_Console) g_Console->InsertMessage(L"ERROR: %hs", message);
     *m_InterestingLog << "<p class=\"error\">ERROR: " << cmessage << "</p>\n";
     m_InterestingLog->flush();
 
@@ -190 +189 @@
     PushRenderMessage(Error, message);
 }
 
-void CLogger::WriteWarning(const wchar_t* message)
+void CLogger::WriteWarning(const char* message)
 {
     std::string cmessage = ToHTML(message);
 
@@ -198 +197 @@
 
     ++m_NumberOfWarnings;
     if (m_UseDebugPrintf)
-        debug_printf(L"WARNING: %ls\n", message);
+        debug_printf(L"WARNING: %hs\n", message);
 
-    if (g_Console) g_Console->InsertMessage(L"WARNING: %ls", message);
+    if (g_Console) g_Console->InsertMessage(L"WARNING: %hs", message);
     *m_InterestingLog << "<p class=\"warning\">WARNING: " << cmessage << "</p>\n";
     m_InterestingLog->flush();
 
@@ -210 +209 @@
     PushRenderMessage(Warning, message);
 }
 
-void CLogger::LogMessage(const wchar_t* fmt, ...)
-{
-    va_list argp;
-    wchar_t buffer[BUFFER_SIZE] = {0};
-    
-    va_start(argp, fmt);
-    if (sys_vswprintf(buffer, ARRAY_SIZE(buffer), fmt, argp) == -1)
-    {
-        // Buffer too small - ensure the string is nicely terminated
-        wcscpy_s(buffer+ARRAY_SIZE(buffer)-4, 4, L"...");
-    }
-    va_end(argp);
-
-    WriteMessage(buffer);
-}
-
-void CLogger::LogMessageRender(const wchar_t* fmt, ...)
-{
-    va_list argp;
-    wchar_t buffer[BUFFER_SIZE] = {0};
-    
-    va_start(argp, fmt);
-    if (sys_vswprintf(buffer, ARRAY_SIZE(buffer), fmt, argp) == -1)
-    {
-        // Buffer too small - ensure the string is nicely terminated
-        wcscpy_s(buffer+ARRAY_SIZE(buffer)-4, 4, L"...");
-    }
-    va_end(argp);
-
-    WriteMessage(buffer, true);
-}
-
-void CLogger::LogWarning(const wchar_t* fmt, ...)
-{
-    va_list argp;
-    wchar_t buffer[BUFFER_SIZE] = {0};
-    
-    va_start(argp, fmt);
-    if (sys_vswprintf(buffer, ARRAY_SIZE(buffer), fmt, argp) == -1)
-    {
-        // Buffer too small - ensure the string is nicely terminated
-        wcscpy_s(buffer+ARRAY_SIZE(buffer)-4, 4, L"...");
-    }
-    va_end(argp);
-
-    WriteWarning(buffer);
-}
-
-void CLogger::LogError(const wchar_t* fmt, ...)
-{
-    va_list argp;
-    wchar_t buffer[BUFFER_SIZE] = {0};
-    
-    va_start(argp, fmt);
-    if (sys_vswprintf(buffer, ARRAY_SIZE(buffer), fmt, argp) == -1)
-    {
-        // Buffer too small - ensure the string is nicely terminated
-        wcscpy_s(buffer+ARRAY_SIZE(buffer)-4, 4, L"...");
-    }
-    va_end(argp);
-
-    WriteError(buffer);
-}
-
 void CLogger::Render()
 {
     PROFILE3_GPU("logger");
@@ -300 +235 @@
 
     for (std::deque<RenderedMessage>::iterator it = m_RenderMessages.begin(); it != m_RenderMessages.end(); ++it)
     {
-        const wchar_t* type;
+        const char* type;
         if (it->method == Normal)
         {
-            type = L"info";
+            type = "info";
             textRenderer.Color(0.0f, 0.8f, 0.0f);
         }
         else if (it->method == Warning)
         {
-            type = L"warning";
+            type = "warning";
             textRenderer.Color(1.0f, 1.0f, 0.0f);
         }
         else
         {
-            type = L"error";
+            type = "error";
             textRenderer.Color(1.0f, 0.0f, 0.0f);
         }
 
         CMatrix3D savedTransform = textRenderer.GetTransform();
 
-        textRenderer.PrintfAdvance(L"[%8.3f] %ls: ", it->time, type);
+        textRenderer.PrintfAdvance(L"[%8.3f] %hs: ", it->time, type);
         // Display the actual message in white so it's more readable
         textRenderer.Color(1.0f, 1.0f, 1.0f);
         textRenderer.Put(0.0f, 0.0f, it->message.c_str());
@@ -334 +269 @@
     textTech->EndPass();
 }
 
-void CLogger::PushRenderMessage(ELogMethod method, const wchar_t* message)
+void CLogger::PushRenderMessage(ELogMethod method, const char* message)
 {
     double now = timer_Time();
 
     // Add each message line separately
-    const wchar_t* pos = message;
-    const wchar_t* eol;
-    while ((eol = wcschr(pos, L'\n')) != NULL)
+    const char* pos = message;
+    const char* eol;
+    while ((eol = strchr(pos, '\n')) != NULL)
     {
         if (eol != pos)
         {
-            RenderedMessage r = { method, now, std::wstring(pos, eol) };
+            RenderedMessage r = { method, now, std::string(pos, eol) };
             m_RenderMessages.push_back(r);
         }
         pos = eol + 1;
     }
     // Add the last line, if we didn't end on a \n
-    if (*pos != L'\0')
+    if (*pos != '\0')
     {
-        RenderedMessage r = { method, now, std::wstring(pos) };
+        RenderedMessage r = { method, now, std::string(pos) };
         m_RenderMessages.push_back(r);
     }
 }
@@ -398 +333 @@
     g_Logger = m_OldLogger;
 }
 
-std::wstring TestLogger::GetOutput()
+std::string TestLogger::GetOutput()
 {
-    Status err;
-    return wstring_from_utf8(m_Stream.str(), &err);
+    return m_Stream.str();
 }
 
 TestStdoutLogger::TestStdoutLogger()

source/ps/CLogger.h

@@ -1 @@
-/* Copyright (C) 2010 Wildfire Games.
+/* Copyright (C) 2015 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -24 +24 @@
 #include <sstream>
 
 #include "ps/ThreadUtil.h"
+#include "third_party/cppformat/format.h"
 
 class CLogger;
 extern CLogger* g_Logger;
 
 
-#define LOGMESSAGE g_Logger->LogMessage
-#define LOGMESSAGERENDER g_Logger->LogMessageRender
-#define LOGWARNING g_Logger->LogWarning
-#define LOGERROR g_Logger->LogError
+#define LOGMESSAGE(...)       g_Logger->WriteMessage(fmt::sprintf(__VA_ARGS__).c_str(), false)
+#define LOGMESSAGERENDER(...) g_Logger->WriteMessage(fmt::sprintf(__VA_ARGS__).c_str(), true)
+#define LOGWARNING(...)       g_Logger->WriteWarning(fmt::sprintf(__VA_ARGS__).c_str())
+#define LOGERROR(...)         g_Logger->WriteError  (fmt::sprintf(__VA_ARGS__).c_str())
 
 /**
  * Error/warning/message logging class.
@@ -64 +65 @@
 
     // Functions to write different message types (Errors and warnings are placed 
     // both in mainLog and intrestingLog.)
-    void WriteMessage(const wchar_t* message, bool doRender);
-    void WriteError  (const wchar_t* message);
-    void WriteWarning(const wchar_t* message);
+    void WriteMessage(const char* message, bool doRender);
+    void WriteError  (const char* message);
+    void WriteWarning(const char* message);
     
-    // Functions to write a message, warning or error to file.
-    void LogMessage(const wchar_t* fmt, ...) WPRINTF_ARGS(2);
-    void LogMessageRender(const wchar_t* fmt, ...) WPRINTF_ARGS(2);
-    void LogWarning(const wchar_t* fmt, ...) WPRINTF_ARGS(2);
-    void LogError(const wchar_t* fmt, ...) WPRINTF_ARGS(2);
-
     // Render recent log messages onto the screen
     void Render();
     
 private:
     void Init();
 
-    void PushRenderMessage(ELogMethod method, const wchar_t* message);
+    void PushRenderMessage(ELogMethod method, const char* message);
 
     // Delete old timed-out entries from the list of text to render
     void CleanupRenderQueue();
@@ -104 +99 @@
     {
         ELogMethod method;
         double time;
-        std::wstring message;
+        std::string message;
     };
     std::deque<RenderedMessage> m_RenderMessages;
     double m_RenderLastEraseTime;
@@ -123 +118 @@
 public:
     TestLogger();
     ~TestLogger();
-    std::wstring GetOutput();
+    std::string GetOutput();
 private:
     CLogger* m_OldLogger;
     std::stringstream m_Stream;

source/ps/XML/tests/test_RelaxNG.h

@@ -44 +44 @@
         {
             TestLogger logger;
             TS_ASSERT(!v.Validate(L"doc", L"<bogus/>"));
-            TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"Parse error: doc:1: Expecting element test, got bogus");
+            TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "Parse error: doc:1: Expecting element test, got bogus");
         }
 
         {
             TestLogger logger;
             TS_ASSERT(!v.Validate(L"doc", L"bogus"));
-            TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"RelaxNGValidator: Failed to parse document");
+            TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "RelaxNGValidator: Failed to parse document");
         }
 
         TS_ASSERT(v.Validate(L"doc", L"<test/>"));
@@ -82 +82 @@
 
         TestLogger logger;
         TS_ASSERT(!v.LoadGrammar("whoops"));
-        TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"RelaxNGValidator: Failed to compile schema");
+        TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "RelaxNGValidator: Failed to compile schema");
 
         TS_ASSERT(!v.Validate(L"doc", L"<test/>"));
     }

source/ps/tests/test_CLogger.h

@@ -24 +24 @@
 public:
     void test_basic()
     {
-        logger->LogMessage(L"Test 1");
-        logger->LogMessage(L"Test 2");
+        logger->WriteMessage("Test 1", false);
+        logger->WriteMessage("Test 2", false);
 
         ParseOutput();
 
@@ -34 +34 @@
         TS_ASSERT_EQUALS(lines[1], "Test 2");
     }
 
-    void test_overflow()
-    {
-        const int buflen = 1024;
-
-        std::string msg0 (buflen-2, '*');
-        std::string msg1 (buflen-1, '*');
-        std::string msg2 (buflen,   '*');
-        std::string msg3 (buflen+1, '*');
-
-        std::string clipped (buflen-4, '*');
-        clipped += "...";
-
-        logger->LogMessage(L"%hs", msg0.c_str());
-        logger->LogMessage(L"%hs", msg1.c_str());
-        logger->LogMessage(L"%hs", msg2.c_str());
-        logger->LogMessage(L"%hs", msg3.c_str());
-
-        logger->LogMessageRender(L"%hs", msg0.c_str());
-        logger->LogMessageRender(L"%hs", msg1.c_str());
-        logger->LogMessageRender(L"%hs", msg2.c_str());
-        logger->LogMessageRender(L"%hs", msg3.c_str());
-
-        logger->LogWarning(L"%hs", msg0.c_str());
-        logger->LogWarning(L"%hs", msg1.c_str());
-        logger->LogWarning(L"%hs", msg2.c_str());
-        logger->LogWarning(L"%hs", msg3.c_str());
-
-        logger->LogError(L"%hs", msg0.c_str());
-        logger->LogError(L"%hs", msg1.c_str());
-        logger->LogError(L"%hs", msg2.c_str());
-        logger->LogError(L"%hs", msg3.c_str());
-
-        ParseOutput();
-
-        TS_ASSERT_EQUALS((int)lines.size(), 4*4);
-        TS_ASSERT_EQUALS(lines[0], msg0);
-        TS_ASSERT_EQUALS(lines[1], msg1);
-        TS_ASSERT_EQUALS(lines[2], clipped);
-        TS_ASSERT_EQUALS(lines[3], clipped);
-
-        TS_ASSERT_EQUALS(lines[4], msg0);
-        TS_ASSERT_EQUALS(lines[5], msg1);
-        TS_ASSERT_EQUALS(lines[6], clipped);
-        TS_ASSERT_EQUALS(lines[7], clipped);
-
-        TS_ASSERT_EQUALS(lines[8], "WARNING: "+msg0);
-        TS_ASSERT_EQUALS(lines[9], "WARNING: "+msg1);
-        TS_ASSERT_EQUALS(lines[10], "WARNING: "+clipped);
-        TS_ASSERT_EQUALS(lines[11], "WARNING: "+clipped);
-
-        TS_ASSERT_EQUALS(lines[12], "ERROR: "+msg0);
-        TS_ASSERT_EQUALS(lines[13], "ERROR: "+msg1);
-        TS_ASSERT_EQUALS(lines[14], "ERROR: "+clipped);
-        TS_ASSERT_EQUALS(lines[15], "ERROR: "+clipped);
-    }
-
     void test_unicode()
     {
-        logger->LogMessage(L"%lc %lc", 226, 295);
+        wchar_t str[] = { 226, 32, 295, 0 };
+        logger->WriteMessage(utf8_from_wstring(str).c_str(), false);
 
         ParseOutput();
 
@@ -102 +47 @@
 
     void test_html()
     {
-        logger->LogMessage(L"Test<a&b>c<d&e>");
+        logger->WriteMessage("Test<a&b>c<d&e>", false);
 
         ParseOutput();
 
@@ -112 +57 @@
 
     void test_null()
     {
-        logger->LogMessage(L"a %hs b", (const char *)NULL);
-        logger->LogMessage(L"c %ls d", (const wchar_t *)NULL);
+        logger->WriteMessage(fmt::sprintf("a %s b", (const char *)NULL).c_str(), false);
 
         ParseOutput();
 
-        TS_ASSERT_EQUALS((int)lines.size(), 2);
+        TS_ASSERT_EQUALS((int)lines.size(), 1);
         TS_ASSERT_EQUALS(lines[0], "a (null) b");
-        TS_ASSERT_EQUALS(lines[1], "c (null) d");
     }
 
     //////////////////////////////////////////////////////////////////////////

source/scriptinterface/tests/test_ScriptInterface.h

@@ -31 +31 @@
         ScriptInterface script("Test", "Test", g_ScriptRuntime);
         TestLogger logger;
         TS_ASSERT(script.LoadScript(L"test.js", "var x = 1+1;"));
-        TS_ASSERT_WSTR_NOT_CONTAINS(logger.GetOutput(), L"JavaScript error");
-        TS_ASSERT_WSTR_NOT_CONTAINS(logger.GetOutput(), L"JavaScript warning");
+        TS_ASSERT_STR_NOT_CONTAINS(logger.GetOutput(), "JavaScript error");
+        TS_ASSERT_STR_NOT_CONTAINS(logger.GetOutput(), "JavaScript warning");
     }
 
     void test_loadscript_error()
@@ -40 +40 @@
         ScriptInterface script("Test", "Test", g_ScriptRuntime);
         TestLogger logger;
         TS_ASSERT(!script.LoadScript(L"test.js", "1+"));
-        TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"JavaScript error: test.js line 1\nSyntaxError: syntax error");
+        TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "JavaScript error: test.js line 1\nSyntaxError: syntax error");
     }
 
     void test_loadscript_strict_warning()
@@ -49 +49 @@
         TestLogger logger;
         // in strict mode, this inside a function doesn't point to the global object
         TS_ASSERT(script.LoadScript(L"test.js", "var isStrict = (function() { return !this; })();warn('isStrict is '+isStrict);"));
-        TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"WARNING: isStrict is true");
+        TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "WARNING: isStrict is true");
     }
 
     void test_loadscript_strict_error()
@@ -57 +57 @@
         ScriptInterface script("Test", "Test", g_ScriptRuntime);
         TestLogger logger;
         TS_ASSERT(!script.LoadScript(L"test.js", "with(1){}"));
-        TS_ASSERT_WSTR_CONTAINS(logger.GetOutput(), L"JavaScript error: test.js line 1\nSyntaxError: strict mode code may not contain \'with\' statements");
+        TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "JavaScript error: test.js line 1\nSyntaxError: strict mode code may not contain \'with\' statements");
     }
 
     void test_clone_basic()

source/simulation2/tests/test_ComponentManager.h

@@ -109 +109 @@
         {
             TestLogger log;
             TS_ASSERT(! man.AddComponent(hnd1, 12345, noParam));
-            TS_ASSERT_WSTR_CONTAINS(log.GetOutput(), L"ERROR: Invalid component id 12345");
+            TS_ASSERT_STR_CONTAINS(log.GetOutput(), "ERROR: Invalid component id 12345");
         }
 
         {
             TestLogger log;
             TS_ASSERT(! man.AddComponent(hnd1, CID_Test1B, noParam));
-            TS_ASSERT_WSTR_CONTAINS(log.GetOutput(), L"ERROR: Multiple components for interface ");
+            TS_ASSERT_STR_CONTAINS(log.GetOutput(), "ERROR: Multiple components for interface ");
         }
     }
 
@@ -347 +347 @@
             // In SpiderMonkey 1.6, JS_ReportError calls the error reporter even if it's inside
             // a try{} in the script; in recent versions (not sure when it changed) it doesn't
             // so the error here won't get reported.
-            TS_ASSERT_WSTR_NOT_CONTAINS(log.GetOutput(), L"ERROR: JavaScript error: simulation/components/error.js line 4\nInvalid interface id");
+            TS_ASSERT_STR_NOT_CONTAINS(log.GetOutput(), "ERROR: JavaScript error: simulation/components/error.js line 4\nInvalid interface id");
         }
     }
 

source/test_setup.cpp

@@ -100 +100 @@
 
 // Definition of functions from lib/self_test.h
 
+bool ts_str_contains(const std::string& str1, const std::string& str2)
+{
+    return str1.find(str2) != str1.npos;
+}
+
 bool ts_str_contains(const std::wstring& str1, const std::wstring& str2)
 {
     return str1.find(str2) != str1.npos;