fix: Move buffer initialization to set config

When calling set config message buffers were not affected because the creation was handled on init. Closes #267
espressif · Oct 25, 2023 · ea0df31 · ea0df31
1 parent abd8b6c
commit ea0df31
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 22 deletions.
diff --git a/include/mqtt_client.h b/include/mqtt_client.h
@@ -597,6 +597,9 @@ esp_err_t esp_mqtt_client_destroy(esp_mqtt_client_handle_t client);
  * @brief Set configuration structure, typically used when updating the config
  * (i.e. on "before_connect" event
  *
+ * Notes:
+ * - When calling this function make sure to have all the intendend configurations
+ *   set, otherwise default values are set.
  * @param client    *MQTT* client handle
  *
  * @param config    *MQTT* configuration structure

diff --git a/lib/include/mqtt_config.h b/lib/include/mqtt_config.h
@@ -100,6 +100,7 @@
 #define MQTT_ENABLE_SSL             CONFIG_MQTT_TRANSPORT_SSL
 #define MQTT_ENABLE_WS              CONFIG_MQTT_TRANSPORT_WEBSOCKET
 #define MQTT_ENABLE_WSS             CONFIG_MQTT_TRANSPORT_WEBSOCKET_SECURE
+#define MQTT_DEFAULT_RETRANSMIT_TIMEOUT_MS 1000
 
 #ifdef CONFIG_MQTT_EVENT_QUEUE_SIZE
 #define MQTT_EVENT_QUEUE_SIZE       CONFIG_MQTT_EVENT_QUEUE_SIZE

diff --git a/lib/mqtt_msg.c b/lib/mqtt_msg.c
@@ -618,7 +618,7 @@ int mqtt_has_valid_msg_hdr(uint8_t *buffer, size_t length)
 
 esp_err_t mqtt_msg_buffer_init(mqtt_connection_t *connection, int buffer_size)
 {
-    memset(connection, 0, sizeof(mqtt_connection_t));
+    memset(&connection->outbound_message, 0, sizeof(mqtt_message_t));
     connection->buffer = (uint8_t *)calloc(buffer_size, sizeof(uint8_t));
     if (!connection->buffer) {
         return ESP_ERR_NO_MEM;

diff --git a/mqtt_client.c b/mqtt_client.c
@@ -383,9 +383,26 @@ esp_err_t esp_mqtt_set_config(esp_mqtt_client_handle_t client, const esp_mqtt_cl
         });
     }
 
+    mqtt_msg_buffer_destroy(&client->mqtt_state.connection);
+    int buffer_size = config->buffer.size;
+    if (buffer_size <= 0) {
+        buffer_size = MQTT_BUFFER_SIZE_BYTE;
+    }
+
+    // use separate value for output buffer size if configured
+    int out_buffer_size = config->buffer.out_size > 0 ? config->buffer.out_size : buffer_size;
+    if (mqtt_msg_buffer_init(&client->mqtt_state.connection, out_buffer_size) != ESP_OK) {
+        goto _mqtt_set_config_failed;
+    }
+
+    free(client->mqtt_state.in_buffer);
+    client->mqtt_state.in_buffer = (uint8_t *)malloc(buffer_size);
+    ESP_MEM_CHECK(TAG, client->mqtt_state.in_buffer, goto _mqtt_set_config_failed);
+    client->mqtt_state.in_buffer_length = buffer_size;
+
     client->config->message_retransmit_timeout = config->session.message_retransmit_timeout;
     if (config->session.message_retransmit_timeout <= 0) {
-        client->config->message_retransmit_timeout = 1000;
+        client->config->message_retransmit_timeout = MQTT_DEFAULT_RETRANSMIT_TIMEOUT_MS;
     }
 
     client->config->task_prio = config->task.priority;
@@ -599,6 +616,8 @@ void esp_mqtt_destroy_config(esp_mqtt_client_handle_t client)
     if (client->config == NULL) {
         return;
     }
+    free(client->mqtt_state.in_buffer);
+    mqtt_msg_buffer_destroy(&client->mqtt_state.connection);
     free(client->config->host);
     free(client->config->uri);
     free(client->config->path);
@@ -807,6 +826,11 @@ static bool create_client_data(esp_mqtt_client_handle_t client)
     client->api_lock = xSemaphoreCreateRecursiveMutex();
     ESP_MEM_CHECK(TAG, client->api_lock, return false);
 
+    client->outbox = outbox_init();
+    ESP_MEM_CHECK(TAG, client->outbox, return false);
+    client->status_bits = xEventGroupCreate();
+    ESP_MEM_CHECK(TAG, client->status_bits, return false);
+
     return true;
 }
 
@@ -824,24 +848,6 @@ esp_mqtt_client_handle_t esp_mqtt_client_init(const esp_mqtt_client_config_t *co
     if (!create_client_data(client)) {
         goto _mqtt_init_failed;
     }
-    int buffer_size = config->buffer.size;
-    if (buffer_size <= 0) {
-        buffer_size = MQTT_BUFFER_SIZE_BYTE;
-    }
-
-    // use separate value for output buffer size if configured
-    int out_buffer_size = config->buffer.out_size > 0 ? config->buffer.out_size : buffer_size;
-    if (mqtt_msg_buffer_init(&client->mqtt_state.connection, out_buffer_size) != ESP_OK) {
-        goto _mqtt_init_failed;
-    }
-
-    client->mqtt_state.in_buffer = (uint8_t *)malloc(buffer_size);
-    ESP_MEM_CHECK(TAG, client->mqtt_state.in_buffer, goto _mqtt_init_failed);
-    client->mqtt_state.in_buffer_length = buffer_size;
-    client->outbox = outbox_init();
-    ESP_MEM_CHECK(TAG, client->outbox, goto _mqtt_init_failed);
-    client->status_bits = xEventGroupCreate();
-    ESP_MEM_CHECK(TAG, client->status_bits, goto _mqtt_init_failed);
 
     if (esp_mqtt_set_config(client, config) != ESP_OK) {
         goto _mqtt_init_failed;
@@ -890,8 +896,6 @@ esp_err_t esp_mqtt_client_destroy(esp_mqtt_client_handle_t client)
     if (client->status_bits) {
         vEventGroupDelete(client->status_bits);
     }
-    free(client->mqtt_state.in_buffer);
-    mqtt_msg_buffer_destroy(&client->mqtt_state.connection);
     if (client->api_lock) {
         vSemaphoreDelete(client->api_lock);
     }