From 15eb0798c6ca85b3ba97a32468fd6e3fa711d971 Mon Sep 17 00:00:00 2001
From: Wiebe Cazemier <wiebe@halfgaar.net>
Date: Sat, 19 Mar 2022 12:45:37 +0100
Subject: [PATCH] still need to avoid unneccessary copy of subtopics

---
 mqttpacket.cpp         |  9 ++-------
 mqttpacket.h           |  2 +-
 publishcopyfactory.cpp | 16 ++++++++--------
 types.cpp              |  1 +
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/mqttpacket.cpp b/mqttpacket.cpp
index 3db7c9e..9a58834 100644
--- a/mqttpacket.cpp
+++ b/mqttpacket.cpp
@@ -910,11 +910,6 @@ void MqttPacket::setDuplicate()
 /**
  * @brief MqttPacket::getPayloadCopy takes part of the vector of bytes and returns it as a string.
  * @return
- *
- * It's necessary sometimes, but it's against FlashMQ's concept of not parsing the payload. Normally, you can just write out
- * the whole byte array of an original packet to subscribers. No need to copy and such.
- *
- * But, as stated, sometimes it's necessary.
  */
 std::string MqttPacket::getPayloadCopy() const
 {
@@ -1128,12 +1123,12 @@ void MqttPacket::setRetain()
     }
 }
 
-Publish *MqttPacket::getPublishData()
+const Publish &MqttPacket::getPublishData()
 {
     if (payloadLen > 0 && publishData.payload.empty())
         publishData.payload = getPayloadCopy();
 
-    return &publishData;
+    return publishData;
 }
 
 void MqttPacket::readIntoBuf(CirBuf &buf) const
diff --git a/mqttpacket.h b/mqttpacket.h
index 2575564..ade9f89 100644
--- a/mqttpacket.h
+++ b/mqttpacket.h
@@ -121,7 +121,7 @@ public:
     std::string getPayloadCopy() const;
     bool getRetain() const;
     void setRetain();
-    Publish *getPublishData();
+    const Publish &getPublishData();
 };
 
 #endif // MQTTPACKET_H
diff --git a/publishcopyfactory.cpp b/publishcopyfactory.cpp
index d0bb77c..923f377 100644
--- a/publishcopyfactory.cpp
+++ b/publishcopyfactory.cpp
@@ -19,8 +19,6 @@ PublishCopyFactory::PublishCopyFactory(Publish *publish) :
 
 MqttPacket *PublishCopyFactory::getOptimumPacket(const char max_qos, const ProtocolVersion protocolVersion)
 {
-    // TODO: cache idea: a set of constructed packets per branch in this code, witn an int identifier.
-
     if (packet)
     {
         if (packet->getProtocolVersion() == protocolVersion && orgQos == max_qos)
@@ -34,12 +32,12 @@ MqttPacket *PublishCopyFactory::getOptimumPacket(const char max_qos, const Proto
 
         if (!cachedPack)
         {
-            Publish *orgPublish = packet->getPublishData();
-            orgPublish->splitTopic = false;
-            orgPublish->qos = max_qos;
+            Publish newPublish(packet->getPublishData());
+            newPublish.splitTopic = false;
+            newPublish.qos = max_qos;
             if (protocolVersion >= ProtocolVersion::Mqtt5)
-                orgPublish->setClientSpecificProperties();
-            cachedPack = std::make_unique<MqttPacket>(protocolVersion, *orgPublish);
+                newPublish.setClientSpecificProperties();
+            cachedPack = std::make_unique<MqttPacket>(protocolVersion, newPublish);
         }
 
         return cachedPack.get();
@@ -100,11 +98,13 @@ Publish PublishCopyFactory::getNewPublish() const
 
     if (packet)
     {
-        Publish p(*packet->getPublishData());
+        Publish p(packet->getPublishData());
+        p.qos = orgQos;
         return p;
     }
 
     Publish p(*publish);
+    p.qos = orgQos;
     return p;
 }
 
diff --git a/types.cpp b/types.cpp
index bb9d050..2b012ab 100644
--- a/types.cpp
+++ b/types.cpp
@@ -19,6 +19,7 @@ License along with FlashMQ. If not, see <https://www.gnu.org/licenses/>.
 
 #include "types.h"
 #include "mqtt5properties.h"
+#include "mqttpacket.h"
 
 ConnAck::ConnAck(const ProtocolVersion protVersion, ReasonCodes return_code, bool session_present) :
     protocol_version(protVersion),
--
libgit2 0.21.4