[Libreoffice-commits] online.git: 2 commits - common/Log.cpp common/Log.hpp common/Session.cpp common/Util.cpp common/Util.hpp Makefile.am net/ServerSocket.hpp net/Socket.hpp net/SslSocket.hpp test/Makefile.am wsd/DocumentBroker.cpp wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp wsd/README wsd/SenderQueue.hpp wsd/Storage.cpp wsd/TileCache.cpp

Michael Meeks michael.meeks at collabora.com
Thu Mar 30 17:21:29 UTC 2017


 Makefile.am            |    3 ++-
 common/Log.cpp         |   21 ++++++++++++++++-----
 common/Log.hpp         |    4 ++--
 common/Session.cpp     |    4 ++--
 common/Util.cpp        |   21 +++++++++++++++++++++
 common/Util.hpp        |    4 ++++
 net/ServerSocket.hpp   |    6 +++---
 net/Socket.hpp         |    6 +++---
 net/SslSocket.hpp      |    8 ++------
 test/Makefile.am       |    1 +
 wsd/DocumentBroker.cpp |    4 ++--
 wsd/LOOLWSD.cpp        |    5 +----
 wsd/LOOLWSD.hpp        |    1 +
 wsd/README             |    3 +++
 wsd/SenderQueue.hpp    |    8 ++++----
 wsd/Storage.cpp        |    6 ++++--
 wsd/TileCache.cpp      |    6 +++---
 17 files changed, 74 insertions(+), 37 deletions(-)

New commits:
commit ae0dba10889a745e04d7be350e89795dd782f030
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Thu Mar 30 18:14:40 2017 +0100

    Cleanup prctl / gettid system-call thrash on logging.
    
    Makes the strace look much prettier.

diff --git a/Makefile.am b/Makefile.am
index a5536b3b..0d9116b5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -131,7 +131,8 @@ looltool_SOURCES = tools/Tool.cpp
 loolstress_CPPFLAGS = -DTDOC=\"$(abs_top_srcdir)/test/data\" ${include_paths}
 loolstress_SOURCES = tools/Stress.cpp \
                      common/Protocol.cpp \
-                     common/Log.cpp
+                     common/Log.cpp \
+		     common/Util.cpp
 
 wsd_headers = wsd/Admin.hpp \
               wsd/AdminModel.hpp \
diff --git a/common/Log.cpp b/common/Log.cpp
index eaf101f1..71b5a5b6 100644
--- a/common/Log.cpp
+++ b/common/Log.cpp
@@ -31,6 +31,7 @@
 #include <Poco/Timestamp.h>
 
 #include "Log.hpp"
+#include "Util.hpp"
 
 static char LogPrefix[256] = { '\0' };
 
@@ -77,12 +78,22 @@ namespace Log
         }
     }
 
-    char* prefix(char* buffer, const char* level, const long osTid)
+    char* prefix(char* buffer, const char* level, bool sigSafe)
     {
+        long osTid;
         char procName[32];
-        if (prctl(PR_GET_NAME, reinterpret_cast<unsigned long>(procName), 0, 0, 0) != 0)
+        const char *threadName = procName;
+        if (sigSafe)
         {
-            strncpy(procName, "<noid>", sizeof(procName) - 1);
+            osTid = syscall(SYS_gettid);
+
+            if (prctl(PR_GET_NAME, reinterpret_cast<unsigned long>(procName), 0, 0, 0) != 0)
+                strncpy(procName, "<noid>", sizeof(procName) - 1);
+        }
+        else
+        {
+            osTid = Util::getThreadId();
+            threadName = Util::getThreadName();
         }
 
         Poco::DateTime time;
@@ -91,14 +102,14 @@ namespace Log
                     osTid,
                     time.hour(), time.minute(), time.second(),
                     time.millisecond() * 1000 + time.microsecond(),
-                    procName, level);
+                    threadName, level);
         return buffer;
     }
 
     void signalLogPrefix()
     {
         char buffer[1024];
-        prefix(buffer, "SIG", syscall(SYS_gettid));
+        prefix(buffer, "SIG", true);
         signalLog(buffer);
     }
 
diff --git a/common/Log.hpp b/common/Log.hpp
index 2ad4e5dd..3f64303a 100644
--- a/common/Log.hpp
+++ b/common/Log.hpp
@@ -28,7 +28,7 @@ namespace Log
                     std::map<std::string, std::string> config);
     Poco::Logger& logger();
 
-    char* prefix(char* buffer, const char* level, const long osTid);
+    char* prefix(char* buffer, const char* level, bool sigSafe);
 
     void trace(const std::string& msg);
     void debug(const std::string& msg);
@@ -175,7 +175,7 @@ namespace Log
     }
 }
 
-#define LOG_BODY_(PRIO, LVL, X) Poco::Message m_(l_.name(), "", Poco::Message::PRIO_##PRIO); char b_[1024]; std::ostringstream oss_(Log::prefix(b_, LVL, syscall(SYS_gettid)), std::ostringstream::ate); oss_ << std::boolalpha << X << "| " << __FILE__ << ':' << __LINE__; m_.setText(oss_.str()); l_.log(m_);
+#define LOG_BODY_(PRIO, LVL, X) Poco::Message m_(l_.name(), "", Poco::Message::PRIO_##PRIO); char b_[1024]; std::ostringstream oss_(Log::prefix(b_, LVL, false), std::ostringstream::ate); oss_ << std::boolalpha << X << "| " << __FILE__ << ':' << __LINE__; m_.setText(oss_.str()); l_.log(m_);
 #define LOG_TRC(X) do { auto& l_ = Log::logger(); if (l_.trace()) { LOG_BODY_(TRACE, "TRC", X); } } while (false)
 #define LOG_DBG(X) do { auto& l_ = Log::logger(); if (l_.debug()) { LOG_BODY_(DEBUG, "DBG", X); } } while (false)
 #define LOG_INF(X) do { auto& l_ = Log::logger(); if (l_.information()) { LOG_BODY_(INFORMATION, "INF", X); } } while (false)
diff --git a/common/Util.cpp b/common/Util.cpp
index 16335fe3..70987c3b 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -258,8 +258,12 @@ namespace Util
         return replace(r, "\n", " / ");
     }
 
+    static __thread char ThreadName[32];
+
     void setThreadName(const std::string& s)
     {
+        strncpy(ThreadName, s.c_str(), 31);
+        ThreadName[31] = '\0';
         if (prctl(PR_SET_NAME, reinterpret_cast<unsigned long>(s.c_str()), 0, 0, 0) != 0)
         {
             LOG_SYS("Cannot set thread name to " << s << ".");
@@ -270,6 +274,23 @@ namespace Util
         }
     }
 
+    const char *getThreadName()
+    {
+        // Avoid so many redundant system calls
+        return ThreadName;
+    }
+
+    static __thread pid_t ThreadTid;
+
+    pid_t getThreadId()
+    {
+        // Avoid so many redundant system calls
+        if (!ThreadTid)
+            ThreadTid = syscall(SYS_gettid);
+        return ThreadTid;
+    }
+
+
     void getVersionInfo(std::string& version, std::string& hash)
     {
         version = std::string(LOOLWSD_VERSION);
diff --git a/common/Util.hpp b/common/Util.hpp
index 57dc6237..b0e29024 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -101,6 +101,10 @@ namespace Util
 
     void setThreadName(const std::string& s);
 
+    const char *getThreadName();
+
+    pid_t getThreadId();
+
     /// Get version information
     void getVersionInfo(std::string& version, std::string& hash);
 
diff --git a/test/Makefile.am b/test/Makefile.am
index bb9377e3..0089e657 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -35,6 +35,7 @@ wsd_sources = \
             ../common/Log.cpp \
             ../common/Protocol.cpp \
             ../common/Session.cpp \
+            ../common/Util.cpp \
             ../common/MessageQueue.cpp \
             ../kit/Kit.cpp \
             ../wsd/TileCache.cpp \
commit 913c469aa8b0d2140054583bcb9ecfec23ac92ef
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Thu Mar 30 10:15:28 2017 +0100

    Cleanup whitespace, return is not a function.

diff --git a/common/Session.cpp b/common/Session.cpp
index 4c30f30f..d6a4eb8a 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -64,13 +64,13 @@ Session::~Session()
 bool Session::sendTextFrame(const char* buffer, const int length)
 {
     LOG_TRC(getName() << ": Send: " << getAbbreviatedMessage(buffer, length));
-    return (sendMessage(buffer, length, WSOpCode::Text) >= length);
+    return sendMessage(buffer, length, WSOpCode::Text) >= length;
 }
 
 bool Session::sendBinaryFrame(const char *buffer, int length)
 {
     LOG_TRC(getName() << ": Send: " << std::to_string(length) << " bytes.");
-    return (sendMessage(buffer, length, WSOpCode::Binary) >= length);
+    return sendMessage(buffer, length, WSOpCode::Binary) >= length;
 }
 
 void Session::parseDocOptions(const std::vector<std::string>& tokens, int& part, std::string& timestamp)
diff --git a/net/ServerSocket.hpp b/net/ServerSocket.hpp
index a0604e0b..805430ea 100644
--- a/net/ServerSocket.hpp
+++ b/net/ServerSocket.hpp
@@ -46,7 +46,7 @@ public:
         ::setsockopt(getFD(), SOL_SOCKET, SO_REUSEADDR, &reuseAddress, len);
 
         const int rc = ::bind(getFD(), address.addr(), address.length());
-        return (rc == 0);
+        return rc == 0;
     }
 
     /// Listen to incoming connections (Servers only).
@@ -55,7 +55,7 @@ public:
     bool listen(const int backlog = 64)
     {
         const int rc = ::listen(getFD(), backlog);
-        return (rc == 0);
+        return rc == 0;
     }
 
     /// Accepts an incoming connection (Servers only).
@@ -70,7 +70,7 @@ public:
         try
         {
             // Create a socket object using the factory.
-            return (rc != -1 ? _sockFactory->create(rc) : std::shared_ptr<Socket>(nullptr));
+            return rc != -1 ? _sockFactory->create(rc) : std::shared_ptr<Socket>(nullptr);
         }
         catch (const std::exception& ex)
         {
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 4767d26d..0b765731 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -129,7 +129,7 @@ public:
         int size;
         unsigned int len = sizeof(size);
         const int rc = ::getsockopt(_fd, SOL_SOCKET, SO_SNDBUF, &size, &len);
-        return (rc == 0 ? size : -1);
+        return rc == 0 ? size : -1;
     }
 
     /// Gets our fast cache of the socket buffer size
@@ -149,7 +149,7 @@ public:
     {
         constexpr unsigned int len = sizeof(size);
         const int rc = ::setsockopt(_fd, SOL_SOCKET, SO_RCVBUF, &size, len);
-        return (rc == 0);
+        return rc == 0;
     }
 
     /// Gets the actual receive buffer size in bytes, -1 on error.
@@ -158,7 +158,7 @@ public:
         int size;
         unsigned int len = sizeof(size);
         const int rc = ::getsockopt(_fd, SOL_SOCKET, SO_RCVBUF, &size, &len);
-        return (rc == 0 ? size : -1);
+        return rc == 0 ? size : -1;
     }
 
     /// Gets the error code.
diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp
index ec732c1a..52292c58 100644
--- a/net/SslSocket.hpp
+++ b/net/SslSocket.hpp
@@ -81,9 +81,7 @@ public:
 
         const int rc = doHandshake();
         if (rc <= 0)
-        {
-            return (rc != 0);
-        }
+            return rc != 0;
 
         // Default implementation.
         return StreamSocket::readIncomingData();
@@ -168,9 +166,7 @@ private:
             {
                 rc = handleSslState(rc);
                 if (rc <= 0)
-                {
-                    return (rc != 0);
-                }
+                    return rc != 0;
             }
 
             _doHandshake = false;
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 6d87fb7b..7894805c 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -58,8 +58,8 @@ std::string getCachePath(const std::string& uri)
 
     digestEngine.update(uri.c_str(), uri.size());
 
-    return (LOOLWSD::Cache + '/' +
-            Poco::DigestEngine::digestToHex(digestEngine.digest()).insert(3, "/").insert(2, "/").insert(1, "/"));
+    return LOOLWSD::Cache + '/' +
+        Poco::DigestEngine::digestToHex(digestEngine.digest()).insert(3, "/").insert(2, "/").insert(1, "/");
 }
 }
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index b5491e51..877d7ccc 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1129,15 +1129,12 @@ void PrisonerPoll::wakeupHook()
 
     std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex, std::defer_lock);
     if (docBrokersLock.try_lock())
-    {
         cleanupDocBrokers();
-    }
 }
 
 void LOOLWSD::triggerChildAndDocHousekeeping()
 {
     PrisonerPoll.wakeup();
-
 }
 
 bool LOOLWSD::createForKit()
@@ -1205,7 +1202,7 @@ bool LOOLWSD::createForKit()
     // Init the Admin manager
     Admin::instance().setForKitPid(ForKitProcId);
 
-    return (ForKitProcId != -1);
+    return ForKitProcId != -1;
 #endif
 }
 
diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp
index d20314c1..b86b23ac 100644
--- a/wsd/LOOLWSD.hpp
+++ b/wsd/LOOLWSD.hpp
@@ -23,6 +23,7 @@
 
 class ChildProcess;
 class TraceFileWriter;
+class DocumentBroker;
 
 std::shared_ptr<ChildProcess> getNewChild_Blocks();
 
diff --git a/wsd/README b/wsd/README
index ab8ea060..2768d859 100644
--- a/wsd/README
+++ b/wsd/README
@@ -387,3 +387,6 @@ The style is roughly as follows, in rough order of importance:
   CamelCaseWithInitialUpperCase.
 
 - [ No kind of Hungarian prefixes. ]
+
+- return - is not a function; but a statement - it doesn't need extra ()
+
diff --git a/wsd/SenderQueue.hpp b/wsd/SenderQueue.hpp
index 2b48d70b..77f09770 100644
--- a/wsd/SenderQueue.hpp
+++ b/wsd/SenderQueue.hpp
@@ -102,8 +102,8 @@ private:
             const auto& pos = std::find_if(_queue.begin(), _queue.end(),
                 [&newTile](const queue_item_t& cur)
                 {
-                    return (cur->firstToken() == "tile:" &&
-                            newTile == TileDesc::parse(cur->firstLine()));
+                    return cur->firstToken() == "tile:" &&
+                           newTile == TileDesc::parse(cur->firstLine());
                 });
 
             if (pos != _queue.end())
@@ -119,7 +119,7 @@ private:
             const auto& pos = std::find_if(_queue.begin(), _queue.end(),
                 [&command](const queue_item_t& cur)
                 {
-                    return (cur->firstToken() == command);
+                    return cur->firstToken() == command;
                 });
 
             if (pos != _queue.end())
@@ -145,7 +145,7 @@ private:
                         Poco::JSON::Parser parser;
                         const auto result = parser.parse(msg);
                         const auto& json = result.extract<Poco::JSON::Object::Ptr>();
-                        return (viewId == json->get("viewId").toString());
+                        return viewId == json->get("viewId").toString();
                     }
 
                     return false;
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 639d057d..94a01c93 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -280,8 +280,10 @@ namespace {
 inline
 Poco::Net::HTTPClientSession* getHTTPClientSession(const Poco::URI& uri)
 {
-    return (LOOLWSD::isSSLEnabled() || LOOLWSD::isSSLTermination()) ? new Poco::Net::HTTPSClientSession(uri.getHost(), uri.getPort(), Poco::Net::SSLManager::instance().defaultClientContext())
-                       : new Poco::Net::HTTPClientSession(uri.getHost(), uri.getPort());
+    return (LOOLWSD::isSSLEnabled() || LOOLWSD::isSSLTermination())
+        ? new Poco::Net::HTTPSClientSession(uri.getHost(), uri.getPort(),
+                                            Poco::Net::SSLManager::instance().defaultClientContext())
+        : new Poco::Net::HTTPClientSession(uri.getHost(), uri.getPort());
 }
 
 int getLevenshteinDist(const std::string& string1, const std::string& string2) {
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index 54ff8e75..4c1d7572 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -111,7 +111,7 @@ std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(c
     Util::assertIsLocked(_tilesBeingRenderedMutex);
 
     const auto tile = _tilesBeingRendered.find(cachedName);
-    return (tile != _tilesBeingRendered.end() ? tile->second : nullptr);
+    return tile != _tilesBeingRendered.end() ? tile->second : nullptr;
 }
 
 void TileCache::forgetTileBeingRendered(const TileDesc& tile)
@@ -389,7 +389,7 @@ std::string TileCache::cacheFileName(const TileDesc& tile)
 
 bool TileCache::parseCacheFileName(const std::string& fileName, int& part, int& width, int& height, int& tilePosX, int& tilePosY, int& tileWidth, int& tileHeight)
 {
-    return (std::sscanf(fileName.c_str(), "%d_%dx%d.%d,%d.%dx%d.png", &part, &width, &height, &tilePosX, &tilePosY, &tileWidth, &tileHeight) == 7);
+    return std::sscanf(fileName.c_str(), "%d_%dx%d.%d,%d.%dx%d.png", &part, &width, &height, &tilePosX, &tilePosY, &tileWidth, &tileHeight) == 7;
 }
 
 bool TileCache::intersectsTile(const std::string& fileName, int part, int x, int y, int width, int height)
@@ -525,7 +525,7 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri
     }
 
     const auto canceltiles = oss.str();
-    return (canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles);
+    return canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles;
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */


More information about the Libreoffice-commits mailing list