loolwsd/LOOLProtocol.hpp | 6 +++++-
loolwsd/test/helpers.hpp | 16 ++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
New commits:
commit 53b718844fd2c0b6366055cf6fc4d018cf6c339b
Author: Tor Lillqvist <t...@collabora.com>
Date: Tue Oct 18 16:23:33 2016 +0300
Add a FIXME for some sillyness
One thing works mainly by accident, another is inefficient.
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 9e3756e..ab2b1ba 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -188,6 +188,22 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket&
ws, const std::string
auto message = LOOLProtocol::getAbbreviatedMessage(response);
if (bytes > 0 && (flags &
Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
{
+ // FIXME: This is wrong in two ways:
+
+ // 1) The message variable is the return value from
getAbbreviatedMessage(),
+ // That is a string that is intended for human-readable
logging only. It is not
+ // intended to be used for actually decoding the protocol.
Sure, at the moment
+ // it happens to work, but is still wrong.
+
+ // 2) Using std::string::find() like this is silly. If
message does not start
+ // with prefix, the find() function will search through
the whole buffer. That
+ // is a waste of cycles. Use the LOOLProtocol functions to
manipulate and
+ // inspect LOOL protocol frames, that is what they are
there
+ // for. getFirstToken() should be quite efficient, and it
doesn't incur the
+ // (potential) overhead of setting up a StringTokenizer.
Or, if this is actually
+ // used to look for not just a first token, then introduce
a startsWith()
+ // function.
+
if (message.find(prefix) == 0)
{
std::cerr << name << "Got " << bytes << " bytes: " <<
message << std::endl;
commit b0c870cbeb77b69c5bf1046646b5253b411a546a
Author: Tor Lillqvist <t...@collabora.com>
Date: Tue Oct 18 15:51:29 2016 +0300
Expand documentation for getAbbreviatedMessage()
It has always been the intent that getAbbreviatedMessage() is for
producing human-readbale nice output for logging purposes only. But
this has not been mentioned, so (naturally) the function has been
misused. Sigh.
diff --git a/loolwsd/LOOLProtocol.hpp b/loolwsd/LOOLProtocol.hpp
index 6120e0c..3857a4e 100644
--- a/loolwsd/LOOLProtocol.hpp
+++ b/loolwsd/LOOLProtocol.hpp
@@ -133,7 +133,11 @@ namespace LOOLProtocol
return getFirstLine(message.data(), message.size());
}
- /// Returns an abbereviation of the message (the first line, indicating
truncation).
+ /// Returns an abbereviation of the message (the first line, indicating
truncation). We assume
+ /// that it adhers to the LOOL protocol, i.e. that there is always a first
(or only) line that
+ /// is in printable UTF-8. I.e. no encoding of binary bytes is done. The
format of the result is
+ /// not guaranteed to be stable. It is to be used for logging purposes
only, not for decoding
+ /// protocol frames.
inline
std::string getAbbreviatedMessage(const char *message, const int length)
{
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
