From 52690ec7e799145401444319bdea4237410399fb Mon Sep 17 00:00:00 2001
From: hsaturn <hsaturn@gmail.com>
Date: Mon, 22 Mar 2021 00:27:23 +0100
Subject: [PATCH] Fix some rare case crashes

---
 src/TinyMqtt.cpp | 16 ++++++++++++----
 src/TinyMqtt.h   | 10 ++++++++--
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/TinyMqtt.cpp b/src/TinyMqtt.cpp
index e9d7ca3..c20b794 100644
--- a/src/TinyMqtt.cpp
+++ b/src/TinyMqtt.cpp
@@ -20,6 +20,14 @@ MqttBroker::MqttBroker(uint16_t port) : server(port)
 {
 }
 
+MqttBroker::~MqttBroker()
+{
+	while(clients.size())
+	{
+		delete clients[0];
+	}
+}
+
 MqttClient::MqttClient(MqttBroker* parent, WiFiClient& new_client)
 	: parent(parent)
 {
@@ -32,7 +40,7 @@ MqttClient::MqttClient(MqttBroker* parent)
 {
 		client = nullptr;
 
-		parent->addClient(this);
+		if (parent) parent->addClient(this);
 }
 
 MqttClient::~MqttClient()
@@ -61,9 +69,9 @@ void MqttClient::connect(std::string broker, uint16_t port)
 {
 	debug("cnx: closing");
 	close();
-	debug("cnx: closed");
 	if (client) delete client;
 	client = new WiFiClient;
+	debug("Trying to connect to " << broker.c_str() << ':' << port);
 	if (client->connect(broker.c_str(), port))
 	{
 	  debug("cnx: connecting");
@@ -467,9 +475,9 @@ void MqttMessage::incoming(char in_byte)
 			reset();
 			break;
 	}
-	if (buffer.length() > 256)	// TODO magic 256 ?
+	if (buffer.length() > MaxBufferLength)	// TODO magic 256 ?
 	{
-		debug("Too long");
+		debug("Too long " << state);
 		reset();
 	}
 }
diff --git a/src/TinyMqtt.h b/src/TinyMqtt.h
index 0d927cb..33195e7 100644
--- a/src/TinyMqtt.h
+++ b/src/TinyMqtt.h
@@ -4,7 +4,6 @@
 #include <string>
 #include "StringIndexer.h"
 
-#define MaxBufferLength 255
 
 class Topic : public IndexedString
 {
@@ -21,6 +20,7 @@ class Topic : public IndexedString
 class MqttClient;
 class MqttMessage
 {
+	const uint16_t MaxBufferLength = 255;
 	public:
 		enum Type
 		{
@@ -161,7 +161,12 @@ class MqttClient
 		uint32_t keep_alive;
 		uint32_t alive;
 		MqttMessage message;
+
+		// TODO	having a pointer on MqttBroker may produce larger binaries
+		// due to unecessary function linked if ever parent is not used
+		// (this is the case when MqttBroker isn't used except here)
 		MqttBroker* parent=nullptr;		// connection to local broker
+
 		WiFiClient* client=nullptr;		// connection to mqtt client or to remote broker
 		std::set<Topic>	subscriptions;
 		std::string clientId;
@@ -179,6 +184,7 @@ class MqttBroker
 	public:
 	  // TODO limit max number of clients
 		MqttBroker(uint16_t port);
+		~MqttBroker();
 
 		void begin() { server.begin(); }
 		void loop();
@@ -190,7 +196,7 @@ class MqttBroker
 
 		void dump()
 		{
-			Serial << "broker: " << clients.size() << " client/s" << endl;
+			Serial << clients.size() << " client/s" << endl;
 			for(auto client: clients)
 			{
 				Serial << "  ";