[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4' - common/Log.cpp common/Log.hpp

[Libreoffice Gerrit user]

 common/Log.cpp |   16 ----------------
 common/Log.hpp |   26 ++++++++++++--------------
 2 files changed, 12 insertions(+), 30 deletions(-)

New commits:
commit e1e22380f285329a0838d637cf94a2c7e6973493
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Sat Apr 27 22:12:57 2019 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Sat Apr 27 22:13:25 2019 +0100

    Revert attempts at re-using ostringstream
    
    Cleaning up the thread variable with the shared string stream is
    something of a nightmare, for a rather marginal gain.
    
    ==9296== Invalid write of size 1
    ...
    ==9296==    by 0x738C092: str (sstream:195)
    ==9296==    by 0x738C092: std::__cxx11::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >::str(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (sstream:649)
    ==9296==    by 0x65383A: Log::beginLog[abi:cxx11](char const*) (Log.cpp:141)
    ==9296==    by 0x551823: Admin::~Admin() (Admin.cpp:381)
    ==9296==    by 0x7D9ECF7: __run_exit_handlers (exit.c:83)
    ==9296==    by 0x7D9ED49: exit (exit.c:105)
    ==9296==    by 0x7D86F50: (below main) (libc-start.c:342)
    ==9296==  Address 0x8ba41c0 is 0 bytes inside a block of size 513 free'd
    ==9296==    at 0x4C2FA1D: operator delete(void*) (vg_replace_malloc.c:576)
    ...
    ==9296==    by 0x738784A: ~basic_stringbuf (sstream:65)
    ==9296==    by 0x738784A: std::__cxx11::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >::~basic_ostringstream() (sstream:591)
    ==9296==    by 0x7D9F27E: __call_tls_dtors (cxa_thread_atexit_impl.c:155)
    ==9296==    by 0x7D9EC0A: __run_exit_handlers (exit.c:41)
    ==9296==    by 0x7D9ED49: exit (exit.c:105)
    ==9296==    by 0x7D86F50: (below main) (libc-start.c:342)
    
    Good to log during shutdown / exit.
    
    This reverts commit bc67163a481d02d76266875372af5ff79a861d23.
    This reverts commit 242df1594dd9283ba0501b141e176617b03cf3d9.

diff --git a/common/Log.cpp b/common/Log.cpp
index f1ed70dcb..a5f101e93 100644
--- a/common/Log.cpp
+++ b/common/Log.cpp
@@ -131,22 +131,6 @@ namespace Log
         return buffer;
     }
 
-    // Reuse the same buffer to minimize memory fragmentation.
-    static thread_local std::ostringstream Oss;
-
-    std::ostringstream& beginLog(const char* level)
-    {
-        // Reset the oss.
-        Oss.clear();
-        Oss.str(std::string());
-        Oss.seekp(0);
-
-        // Output the prefix.
-        char buffer[1024];
-        Oss << Log::prefix(buffer, sizeof(buffer) - 1, level) << std::boolalpha;
-        return Oss;
-    }
-
     void signalLogPrefix()
     {
         char buffer[1024];
diff --git a/common/Log.hpp b/common/Log.hpp
index a17509317..bf05fc7ce 100644
--- a/common/Log.hpp
+++ b/common/Log.hpp
@@ -50,9 +50,6 @@ namespace Log
 
     char* prefix(char* buffer, std::size_t len, const char* level);
 
-    /// Starts logging by generating the prefix and returning an oss.
-    std::ostringstream& beginLog(const char* level);
-
     inline bool traceEnabled() { return logger().trace(); }
     inline bool debugEnabled() { return logger().debug(); }
     inline bool infoEnabled() { return logger().information(); }
@@ -236,19 +233,20 @@ namespace Log
 #define LOG_FILE_NAME(f) (strrchr(f, '/')+1)
 #endif
 
-#define LOG_END(LOG, FILEP)                                                                        \
-    do                                                                                             \
-    {                                                                                              \
-        if (FILEP)                                                                                 \
-            LOG << "| " << LOG_FILE_NAME(__FILE__) << ':' << __LINE__;                             \
+#define LOG_END(LOG, FILEP)                             \
+    do                                                  \
+    {                                                   \
+        if (FILEP)                                      \
+            LOG << "| " << LOG_FILE_NAME(__FILE__) << ':' << __LINE__; \
     } while (false)
 
-#define LOG_BODY_(LOG, PRIO, LVL, X, FILEP)                                                        \
-    Poco::Message m_(LOG.name(), "", Poco::Message::PRIO_##PRIO);                                  \
-    std::ostringstream& oss_ = Log::beginLog(LVL);                                                    \
-    oss_ << X;                                                                                     \
-    LOG_END(oss_, FILEP);                                                                          \
-    m_.setText(oss_.str());                                                                        \
+#define LOG_BODY_(LOG, PRIO, LVL, X, FILEP)                                                 \
+    Poco::Message m_(LOG.name(), "", Poco::Message::PRIO_##PRIO);                           \
+    char b_[1024];                                                                          \
+    std::ostringstream oss_(Log::prefix(b_, sizeof(b_) - 1, LVL), std::ostringstream::ate); \
+    oss_ << std::boolalpha << X;                                                            \
+    LOG_END(oss_, FILEP);                                                                   \
+    m_.setText(oss_.str());                                                                 \
     LOG.log(m_);
 
 #define LOG_TRC(X)                                  \