[Libreoffice-commits] online.git: loolwsd/Connect.cpp loolwsd/LOOLWebSocket.hpp loolwsd/LOOLWSD.cpp loolwsd/test

Ashod Nakashian ashod.nakashian at collabora.co.uk
Fri Nov 25 03:50:28 UTC 2016


 loolwsd/Connect.cpp            |   11 ++++++-----
 loolwsd/LOOLWSD.cpp            |    5 +++--
 loolwsd/LOOLWebSocket.hpp      |   33 ---------------------------------
 loolwsd/test/UnitFonts.cpp     |   13 ++++++-------
 loolwsd/test/helpers.hpp       |   12 +++++++-----
 loolwsd/test/httpcrashtest.cpp |    9 +++++----
 loolwsd/test/httpwstest.cpp    |   40 +++++++++++++++++++++++-----------------
 7 files changed, 50 insertions(+), 73 deletions(-)

New commits:
commit 45c1856c6ad1753f8a90d3bb90711ab0338d623c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Thu Nov 24 19:18:15 2016 -0500

    loolwsd: kill receiveFrame with char* and cleanup usage cases
    
    Change-Id: I64585f8992407e0a3ff26fba1dccd327de13b7ff
    Reviewed-on: https://gerrit.libreoffice.org/31185
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/Connect.cpp b/loolwsd/Connect.cpp
index 5b550dd..aad78fa 100644
--- a/loolwsd/Connect.cpp
+++ b/loolwsd/Connect.cpp
@@ -89,23 +89,24 @@ public:
         {
             do
             {
-                char buffer[100000];
-                n = _ws.receiveFrame(buffer, sizeof(buffer), flags);
+                Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
+                buffer.resize(0);
+                n = _ws.receiveFrame(buffer, flags);
                 if (n > 0 && (flags & WebSocket::FRAME_OP_BITMASK) != WebSocket::FRAME_OP_CLOSE)
                 {
                     {
                         std::unique_lock<std::mutex> lock(coutMutex);
-                        std::cout << "Got " << getAbbreviatedFrameDump(buffer, n, flags) << std::endl;
+                        std::cout << "Got " << getAbbreviatedFrameDump(buffer.begin(), n, flags) << std::endl;
                     }
 
-                    std::string firstLine = getFirstLine(buffer, n);
+                    const std::string firstLine = getFirstLine(buffer.begin(), n);
                     StringTokenizer tokens(firstLine, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
 
                     if (std::getenv("DISPLAY") != nullptr && tokens[0] == "tile:")
                     {
                         TemporaryFile pngFile;
                         std::ofstream pngStream(pngFile.path(), std::ios::binary);
-                        pngStream.write(buffer + firstLine.size() + 1, n - firstLine.size() - 1);
+                        pngStream.write(buffer.begin() + firstLine.size() + 1, n - firstLine.size() - 1);
                         pngStream.close();
                         if (std::system((std::string("display ") + pngFile.path()).c_str()) == -1)
                         {
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 5ca3fce..b7ad287 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -196,7 +196,7 @@ void shutdownLimitReached(LOOLWebSocket& ws)
     {
         int flags = 0;
         int retries = 7;
-        std::vector<char> buffer(READ_BUFFER_SIZE * 100);
+        Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
 
         const Poco::Timespan waitTime(POLL_TIMEOUT_MS * 1000);
         do
@@ -214,7 +214,8 @@ void shutdownLimitReached(LOOLWebSocket& ws)
             // Ignore incoming messages.
             if (ws.poll(waitTime, Poco::Net::Socket::SELECT_READ))
             {
-                ws.receiveFrame(buffer.data(), buffer.capacity(), flags);
+                buffer.resize(0);
+                ws.receiveFrame(buffer, flags);
             }
 
             // Shutdown.
diff --git a/loolwsd/LOOLWebSocket.hpp b/loolwsd/LOOLWebSocket.hpp
index fb1d88b..827c324 100644
--- a/loolwsd/LOOLWebSocket.hpp
+++ b/loolwsd/LOOLWebSocket.hpp
@@ -79,39 +79,6 @@ public:
     /// Wrapper for Poco::Net::WebSocket::receiveFrame() that handles PING frames
     /// (by replying with a PONG frame) and PONG frames. PONG frames are ignored.
     /// Should we also factor out the handling of non-final and continuation frames into this?
-    int receiveFrame(char* buffer, const int length, int& flags)
-    {
-#ifdef ENABLE_DEBUG
-        // Delay receiving the frame
-        std::this_thread::sleep_for(getWebSocketDelay());
-#endif
-        // Timeout given is in microseconds.
-        static const Poco::Timespan waitTime(POLL_TIMEOUT_MS * 1000);
-
-        while (poll(waitTime, Poco::Net::Socket::SELECT_READ))
-        {
-            const int n = Poco::Net::WebSocket::receiveFrame(buffer, length, flags);
-            if ((flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_PING)
-            {
-                sendFrame(buffer, n, WebSocket::FRAME_FLAG_FIN | WebSocket::FRAME_OP_PONG);
-            }
-            else if ((flags & WebSocket::FRAME_OP_BITMASK) == WebSocket::FRAME_OP_PONG)
-            {
-                // In case we do send pongs in the future.
-            }
-            else
-            {
-                return n;
-            }
-        }
-
-        return -1;
-    }
-
-
-    /// Wrapper for Poco::Net::WebSocket::receiveFrame() that handles PING frames
-    /// (by replying with a PONG frame) and PONG frames. PONG frames are ignored.
-    /// Should we also factor out the handling of non-final and continuation frames into this?
     int receiveFrame(Poco::Buffer<char>& buffer, int& flags)
     {
 #ifdef ENABLE_DEBUG
diff --git a/loolwsd/test/UnitFonts.cpp b/loolwsd/test/UnitFonts.cpp
index 60312d4..232ecb2 100644
--- a/loolwsd/test/UnitFonts.cpp
+++ b/loolwsd/test/UnitFonts.cpp
@@ -44,17 +44,16 @@ namespace {
     std::string readFontList(const std::shared_ptr<LOOLWebSocket> &socket)
     {
         int flags;
-        char buffer[100 * 1000];
+        Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
 
-        int length = socket->receiveFrame(buffer, sizeof (buffer), flags);
+        buffer.resize(0);
+        const int length = socket->receiveFrame(buffer, flags);
         if (length > 0)
         {
-            assert(length<(int)sizeof(buffer));
-            buffer[length] = '\0';
-            return std::string(buffer);
+            return std::string(buffer.begin(), length);
         }
-        else
-            return std::string("read failure");
+
+        return std::string("read failure");
     }
 }
 
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 7e5f039..c6c730a 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -173,7 +173,8 @@ int getErrorCode(LOOLWebSocket& ws, std::string& message)
     ws.setReceiveTimeout(timeout);
     do
     {
-        bytes = ws.receiveFrame(buffer.begin(), READ_BUFFER_SIZE, flags);
+        buffer.resize(0);
+        bytes = ws.receiveFrame(buffer, flags);
     }
     while ((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
 
@@ -409,7 +410,7 @@ void SocketProcessor(const std::string& name,
     const Poco::Timespan waitTime(timeoutMs * 1000);
     int flags = 0;
     int n = 0;
-    char buffer[READ_BUFFER_SIZE];
+    Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
     do
     {
         if (!socket->poll(waitTime, Poco::Net::Socket::SELECT_READ))
@@ -418,11 +419,12 @@ void SocketProcessor(const std::string& name,
             break;
         }
 
-        n = socket->receiveFrame(buffer, sizeof(buffer), flags);
-        std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(buffer, n, flags) << std::endl;
+        buffer.resize(0);
+        n = socket->receiveFrame(buffer, flags);
+        std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(buffer.begin(), n, flags) << std::endl;
         if (n > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
         {
-            if (!handler(std::string(buffer, n)))
+            if (!handler(std::string(buffer.begin(), n)))
             {
                 break;
             }
diff --git a/loolwsd/test/httpcrashtest.cpp b/loolwsd/test/httpcrashtest.cpp
index 1bfd4f2..b9a297c 100644
--- a/loolwsd/test/httpcrashtest.cpp
+++ b/loolwsd/test/httpcrashtest.cpp
@@ -164,11 +164,12 @@ void HTTPCrashTest::testCrashKit()
         // receive close frame handshake
         int bytes;
         int flags;
-        char buffer[READ_BUFFER_SIZE];
+        Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
         do
         {
-            bytes = socket->receiveFrame(buffer, sizeof(buffer), flags);
-            std::cerr << testname << "Got " << LOOLProtocol::getAbbreviatedFrameDump(buffer, bytes, flags) << std::endl;
+            buffer.resize(0);
+            bytes = socket->receiveFrame(buffer, flags);
+            std::cerr << testname << "Got " << LOOLProtocol::getAbbreviatedFrameDump(buffer.begin(), bytes, flags) << std::endl;
         }
         while ((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
 
@@ -176,7 +177,7 @@ void HTTPCrashTest::testCrashKit()
         socket->shutdown();
 
         // no more messages is received.
-        bytes = socket->receiveFrame(buffer, sizeof(buffer), flags);
+        bytes = socket->receiveFrame(buffer, flags);
         CPPUNIT_ASSERT_MESSAGE("Expected no more data", bytes <= 2); // The 2-byte marker is ok.
         CPPUNIT_ASSERT_EQUAL(0x88, flags);
     }
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index 1e65fc2..eeff3f4 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -253,37 +253,42 @@ void HTTPWSTest::testHandShake()
         socket.setReceiveTimeout(0);
 
         int flags = 0;
-        char buffer[1024] = {0};
-        int bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
-        CPPUNIT_ASSERT_EQUAL(std::string("statusindicator: find"), std::string(buffer, bytes));
-
-        bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
-        if (bytes > 0 && !std::strstr(buffer, "error:"))
+        Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
+        buffer.resize(0);
+        int bytes = socket.receiveFrame(buffer, flags);
+        CPPUNIT_ASSERT_EQUAL(std::string("statusindicator: find"), std::string(buffer.begin(), bytes));
+
+        buffer.resize(0);
+        bytes = socket.receiveFrame(buffer, flags);
+        if (bytes > 0 && !std::strstr(buffer.begin(), "error:"))
         {
-            CPPUNIT_ASSERT_EQUAL(std::string("statusindicator: connect"), std::string(buffer, bytes));
+            CPPUNIT_ASSERT_EQUAL(std::string("statusindicator: connect"), std::string(buffer.begin(), bytes));
 
-            bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
-            if (!std::strstr(buffer, "error:"))
+            buffer.resize(0);
+            bytes = socket.receiveFrame(buffer, flags);
+            if (!std::strstr(buffer.begin(), "error:"))
             {
-                CPPUNIT_ASSERT_EQUAL(std::string("statusindicator: ready"), std::string(buffer, bytes));
+                CPPUNIT_ASSERT_EQUAL(std::string("statusindicator: ready"), std::string(buffer.begin(), bytes));
             }
             else
             {
                 // check error message
-                CPPUNIT_ASSERT(std::strstr(SERVICE_UNAVAILABLE_INTERNAL_ERROR, buffer) != nullptr);
+                CPPUNIT_ASSERT(std::strstr(SERVICE_UNAVAILABLE_INTERNAL_ERROR, buffer.begin()) != nullptr);
 
                 // close frame message
-                bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
+                buffer.resize(0);
+                bytes = socket.receiveFrame(buffer, flags);
                 CPPUNIT_ASSERT((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) == Poco::Net::WebSocket::FRAME_OP_CLOSE);
             }
         }
         else
         {
             // check error message
-            CPPUNIT_ASSERT(std::strstr(SERVICE_UNAVAILABLE_INTERNAL_ERROR, buffer) != nullptr);
+            CPPUNIT_ASSERT(std::strstr(SERVICE_UNAVAILABLE_INTERNAL_ERROR, buffer.begin()) != nullptr);
 
             // close frame message
-            bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
+            buffer.resize(0);
+            bytes = socket.receiveFrame(buffer, flags);
             CPPUNIT_ASSERT((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) == Poco::Net::WebSocket::FRAME_OP_CLOSE);
         }
     }
@@ -309,15 +314,16 @@ void HTTPWSTest::testCloseAfterClose()
         // receive close frame handshake
         int bytes;
         int flags;
-        char buffer[READ_BUFFER_SIZE];
+        Poco::Buffer<char> buffer(READ_BUFFER_SIZE);
         do
         {
-            bytes = socket->receiveFrame(buffer, sizeof(buffer), flags);
+            buffer.resize(0);
+            bytes = socket->receiveFrame(buffer, flags);
         }
         while (bytes && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
 
         // no more messages is received.
-        bytes = socket->receiveFrame(buffer, sizeof(buffer), flags);
+        bytes = socket->receiveFrame(buffer, flags);
         std::cerr << "Received " << bytes << " bytes, flags: "<< std::hex << flags << std::dec << std::endl;
         CPPUNIT_ASSERT_EQUAL(0, bytes);
         CPPUNIT_ASSERT_EQUAL(0, flags);


More information about the Libreoffice-commits mailing list