From b6e3fc1dc165e63745fe078c6704eedd3a6be9bd Mon Sep 17 00:00:00 2001 From: Fanda Vacek Date: Fri, 15 Nov 2024 21:05:20 +0100 Subject: [PATCH] Change FrameWriter::addFrame() to move its argument. This can prevent copy frame data since c++23 --- .../include/shv/chainpack/rpcdriver.h | 2 +- .../include/shv/chainpack/rpcmessage.h | 1 + .../include/shv/chainpack/socketrpcdriver.h | 2 +- libshvchainpack/src/rpcdriver.cpp | 10 ++++++---- libshvchainpack/src/rpcmessage.cpp | 10 ++++++++-- libshvchainpack/src/socketrpcdriver.cpp | 8 +++++--- .../include/shv/iotqt/rpc/serialportsocket.h | 3 ++- libshviotqt/include/shv/iotqt/rpc/socket.h | 10 ++++++---- .../include/shv/iotqt/rpc/socketrpcconnection.h | 2 +- libshviotqt/src/rpc/serialportsocket.cpp | 4 ++-- libshviotqt/src/rpc/socket.cpp | 17 ++++++++++++++--- libshviotqt/src/rpc/socketrpcconnection.cpp | 4 ++-- libshviotqt/tests/test_frame_reader.cpp | 4 ++-- 13 files changed, 51 insertions(+), 26 deletions(-) diff --git a/libshvchainpack/include/shv/chainpack/rpcdriver.h b/libshvchainpack/include/shv/chainpack/rpcdriver.h index 960fc8148..4b9817b8f 100644 --- a/libshvchainpack/include/shv/chainpack/rpcdriver.h +++ b/libshvchainpack/include/shv/chainpack/rpcdriver.h @@ -26,7 +26,7 @@ class SHVCHAINPACK_DECL_EXPORT RpcDriver protected: virtual bool isOpen() = 0; - virtual void writeFrameData(const std::string &frame_data) = 0; + virtual void writeFrame(RpcFrame &&frame) = 0; void processRpcFrame(RpcFrame &&frame); virtual void onRpcFrameReceived(RpcFrame &&frame); diff --git a/libshvchainpack/include/shv/chainpack/rpcmessage.h b/libshvchainpack/include/shv/chainpack/rpcmessage.h index 2937df771..79b8bc0bb 100644 --- a/libshvchainpack/include/shv/chainpack/rpcmessage.h +++ b/libshvchainpack/include/shv/chainpack/rpcmessage.h @@ -26,6 +26,7 @@ struct SHVCHAINPACK_DECL_EXPORT RpcFrame RpcFrame(Rpc::ProtocolType protocol, RpcValue::MetaData &&meta, std::string &&data) : protocol(protocol), meta(std::move(meta)), data(std::move(data)) {} bool isValid() const { return !meta.isEmpty() && !data.empty(); } RpcMessage toRpcMessage(std::string *errmsg = nullptr) const; + std::string toFrameHead() const; std::string toFrameData() const; static RpcFrame fromFrameData(const std::string &frame_data); }; diff --git a/libshvchainpack/include/shv/chainpack/socketrpcdriver.h b/libshvchainpack/include/shv/chainpack/socketrpcdriver.h index e77f1cdfa..48aaa2abd 100644 --- a/libshvchainpack/include/shv/chainpack/socketrpcdriver.h +++ b/libshvchainpack/include/shv/chainpack/socketrpcdriver.h @@ -20,7 +20,7 @@ class SHVCHAINPACK_DECL_EXPORT SocketRpcDriver : public RpcDriver void sendNotify(std::string &&method, const RpcValue &result); protected: bool isOpen() override; - void writeFrameData(const std::string &frame_data) override; + void writeFrame(RpcFrame &&frame) override; virtual void onFrameDataRead(const std::string &frame_data); virtual void idleTaskOnSelectTimeout(); diff --git a/libshvchainpack/src/rpcdriver.cpp b/libshvchainpack/src/rpcdriver.cpp index bee3d2211..d0c122e3e 100644 --- a/libshvchainpack/src/rpcdriver.cpp +++ b/libshvchainpack/src/rpcdriver.cpp @@ -46,11 +46,13 @@ void RpcDriver::sendRpcFrame(RpcFrame &&frame) if (!errmsg.empty()) { throw std::runtime_error("Cannot convert RPC frame to message: " + errmsg); } - frame = msg.toRpcFrame(m_clientProtocolType); + auto frame2 = msg.toRpcFrame(m_clientProtocolType); + writeFrame(std::move(frame2)); + } + else { + //logRpcData().nospace() << "FRAME DATA WRITE " << frame_data.size() << " bytes of data:\n" << shv::chainpack::utils::hexDump(frame_data); + writeFrame(std::move(frame)); } - auto frame_data = frame.toFrameData(); - //logRpcData().nospace() << "FRAME DATA WRITE " << frame_data.size() << " bytes of data:\n" << shv::chainpack::utils::hexDump(frame_data); - writeFrameData(frame_data); } catch (const std::exception &e) { nError() << "ERROR send frame:" << e.what(); diff --git a/libshvchainpack/src/rpcmessage.cpp b/libshvchainpack/src/rpcmessage.cpp index ffa2d88fd..27e44354b 100644 --- a/libshvchainpack/src/rpcmessage.cpp +++ b/libshvchainpack/src/rpcmessage.cpp @@ -96,7 +96,7 @@ RpcMessage RpcFrame::toRpcMessage(std::string *errmsg) const return {}; } -std::string RpcFrame::toFrameData() const +std::string RpcFrame::toFrameHead() const { switch (protocol) { case Rpc::ProtocolType::ChainPack: { @@ -118,7 +118,6 @@ std::string RpcFrame::toFrameData() const } auto ret = out.str(); ret = static_cast(protocol) + ret; - ret += data; return ret; } default: { @@ -127,6 +126,13 @@ std::string RpcFrame::toFrameData() const } } +std::string RpcFrame::toFrameData() const +{ + auto ret = toFrameHead(); + ret += data; + return ret; +} + RpcFrame RpcFrame::fromFrameData(const std::string &frame_data) { std::istringstream in(frame_data); diff --git a/libshvchainpack/src/socketrpcdriver.cpp b/libshvchainpack/src/socketrpcdriver.cpp index 6fec2cd31..a8302a916 100644 --- a/libshvchainpack/src/socketrpcdriver.cpp +++ b/libshvchainpack/src/socketrpcdriver.cpp @@ -56,14 +56,16 @@ void SocketRpcDriver::idleTaskOnSelectTimeout() { } -void SocketRpcDriver::writeFrameData(const std::string &frame_data) +void SocketRpcDriver::writeFrame(RpcFrame &&frame) { if(!isOpen()) { nInfo() << "Write to closed socket"; return; } - if (m_writeBuffer.size() + frame_data.size() < m_maxWriteBufferLength) { - m_writeBuffer += frame_data; + auto frame_head = frame.toFrameHead(); + if (m_writeBuffer.size() + frame_head.size() + frame.data.size() < m_maxWriteBufferLength) { + m_writeBuffer += frame_head; + m_writeBuffer += frame.data; flush(); } } diff --git a/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h b/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h index 332e8977b..4cfaf6e0c 100644 --- a/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h +++ b/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h @@ -43,8 +43,9 @@ class SHVIOTQT_DECL_EXPORT SerialFrameWriter : public FrameWriter SerialFrameWriter(CrcCheck crc); ~SerialFrameWriter() override = default; - void addFrame(const std::string &frame_data) override; void resetCommunication() override; +protected: + void addFrameData(std::string &&frame_data) override; private: bool m_withCrcCheck = true; }; diff --git a/libshviotqt/include/shv/iotqt/rpc/socket.h b/libshviotqt/include/shv/iotqt/rpc/socket.h index 6c023d530..cb75e4ef7 100644 --- a/libshviotqt/include/shv/iotqt/rpc/socket.h +++ b/libshviotqt/include/shv/iotqt/rpc/socket.h @@ -45,13 +45,15 @@ class SHVIOTQT_DECL_EXPORT FrameWriter { public: virtual ~FrameWriter() = default; - virtual void addFrame(const std::string &frame_data) = 0; + void addFrame(shv::chainpack::RpcFrame &&frame); virtual void resetCommunication() {} void flushToDevice(QIODevice *device); void clear(); #ifdef WITH_SHV_WEBSOCKETS void flushToWebSocket(QWebSocket *socket); #endif +protected: + virtual void addFrameData(std::string &&frame_data) = 0; protected: QList m_messageDataToWrite; }; @@ -70,8 +72,8 @@ class SHVIOTQT_DECL_EXPORT StreamFrameWriter : public FrameWriter { public: ~StreamFrameWriter() override = default; - - void addFrame(const std::string &frame_data) override; +protected: + void addFrameData(std::string &&frame_data) override; }; /// wrapper class for QTcpSocket and QWebSocket @@ -103,7 +105,7 @@ class SHVIOTQT_DECL_EXPORT Socket : public QObject virtual quint16 peerPort() const = 0; std::vector takeFrames(); - void writeFrameData(const std::string &frame_data); + void writeFrame(chainpack::RpcFrame &&frame); virtual void ignoreSslErrors() = 0; diff --git a/libshviotqt/include/shv/iotqt/rpc/socketrpcconnection.h b/libshviotqt/include/shv/iotqt/rpc/socketrpcconnection.h index 6938d921a..563cf20cb 100644 --- a/libshviotqt/include/shv/iotqt/rpc/socketrpcconnection.h +++ b/libshviotqt/include/shv/iotqt/rpc/socketrpcconnection.h @@ -50,7 +50,7 @@ class SHVIOTQT_DECL_EXPORT SocketRpcConnection : public QObject, public shv::cha protected: // RpcDriver interface bool isOpen() Q_DECL_OVERRIDE; - void writeFrameData(const std::string &frame_data) override; + void writeFrame(shv::chainpack::RpcFrame &&frame) override; Socket* socket(); void onReadyRead(); diff --git a/libshviotqt/src/rpc/serialportsocket.cpp b/libshviotqt/src/rpc/serialportsocket.cpp index 116f24dad..4c02f9e07 100644 --- a/libshviotqt/src/rpc/serialportsocket.cpp +++ b/libshviotqt/src/rpc/serialportsocket.cpp @@ -177,7 +177,7 @@ SerialFrameWriter::SerialFrameWriter(CrcCheck crc) { } -void SerialFrameWriter::addFrame(const std::string &frame_data) +void SerialFrameWriter::addFrameData(string &&frame_data) { QByteArray data_to_write; auto write_escaped = [&data_to_write](uint8_t b) { @@ -213,7 +213,7 @@ void SerialFrameWriter::resetCommunication() QByteArray data_to_write; data_to_write.append(static_cast(00)); - addFrame(data_to_write.toStdString()); + addFrameData(data_to_write.toStdString()); } //====================================================== diff --git a/libshviotqt/src/rpc/socket.cpp b/libshviotqt/src/rpc/socket.cpp index 130ae3802..9d21316f6 100644 --- a/libshviotqt/src/rpc/socket.cpp +++ b/libshviotqt/src/rpc/socket.cpp @@ -24,6 +24,17 @@ namespace shv::iotqt::rpc { //====================================================== // FrameWriter //====================================================== +void FrameWriter::addFrame(chainpack::RpcFrame &&frame) +{ + try { + auto frame_data = frame.toFrameHead(); + frame_data += frame.data; + addFrameData(std::move(frame_data)); + } catch (const std::runtime_error &e) { + shvWarning() << "Error converting frame to data:" << e.what(); + } +} + void FrameWriter::flushToDevice(QIODevice *device) { while (!m_messageDataToWrite.isEmpty()) { @@ -160,7 +171,7 @@ QList StreamFrameReader::addData(std::string_view data) //====================================================== // StreamFrameWriter //====================================================== -void StreamFrameWriter::addFrame(const std::string &frame_data) +void StreamFrameWriter::addFrameData(std::string &&frame_data) { using namespace shv::chainpack; std::ostringstream out; @@ -245,10 +256,10 @@ std::vector Socket::takeFrames() return m_frameReader->takeFrames(); } -void Socket::writeFrameData(const std::string &frame_data) +void Socket::writeFrame(shv::chainpack::RpcFrame &&frame) { Q_ASSERT(m_frameWriter); - m_frameWriter->addFrame(frame_data); + m_frameWriter->addFrame(std::move(frame)); flushWriteBuffer(); } diff --git a/libshviotqt/src/rpc/socketrpcconnection.cpp b/libshviotqt/src/rpc/socketrpcconnection.cpp index a873c03dc..0936468f2 100644 --- a/libshviotqt/src/rpc/socketrpcconnection.cpp +++ b/libshviotqt/src/rpc/socketrpcconnection.cpp @@ -111,9 +111,9 @@ bool SocketRpcConnection::isOpen() return isSocketConnected(); } -void SocketRpcConnection::writeFrameData(const std::string &frame_data) +void SocketRpcConnection::writeFrame(chainpack::RpcFrame &&frame) { - socket()->writeFrameData(frame_data); + socket()->writeFrame(std::move(frame)); } void SocketRpcConnection::sendRpcMessage(const shv::chainpack::RpcMessage &rpc_msg) diff --git a/libshviotqt/tests/test_frame_reader.cpp b/libshviotqt/tests/test_frame_reader.cpp index c098dfe4b..ea4b566fa 100644 --- a/libshviotqt/tests/test_frame_reader.cpp +++ b/libshviotqt/tests/test_frame_reader.cpp @@ -36,7 +36,7 @@ vector msg_to_raw_data(const vector &cpons) auto rv = RpcValue::fromCpon(cpon); auto msg = RpcMessage(rv); StreamFrameWriter wr; - wr.addFrame(msg.toRpcFrame().toFrameData()); + wr.addFrame(msg.toRpcFrame()); QByteArray ba; { QBuffer buffer(&ba); @@ -54,7 +54,7 @@ vector msg_to_raw_data_serial(const vector &cpons, SerialFrameWr auto rv = RpcValue::fromCpon(cpon); auto msg = RpcMessage(rv); SerialFrameWriter wr(crc_check); - wr.addFrame(msg.toRpcFrame().toFrameData()); + wr.addFrame(msg.toRpcFrame()); QByteArray ba; { QBuffer buffer(&ba);