From 520c1cf5ac8c1ac01b59e46d88a36c6e166b8582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BD=95=E5=B0=91=E7=90=BC?= Date: Sat, 27 Jun 2026 01:01:24 +0800 Subject: [PATCH] perf(iot-client): reduce SDK heap churn; fix atop OOM leak Memory-management pass over iot-client (+ shared TLS/rtc-log). No public API change beyond the iot_client_t internal layout. Per-session (iot_client_t): inline https_url/mqtt_url as char[64] (was char*+strdup) and the DP context as opaque inline storage (was a lazy pal->malloc), guarded by a _Static_assert. Fewer per-session allocations, bounded URL storage. Per-op: stack the atop sign buffer, the MQTT subscribe topic and the publish topic (short fixed formats, no per-op topic malloc); combine the HTTP request-header + response into one allocation; DP report/state returns the cJSON string directly (cJSON hooks are the pal allocator) instead of strdup+copy; tai_pkt_log formats into a stack buffer instead of a per-log malloc/free (no heap churn on the log path) and drops a nested 260B base64 buffer. fix(atop): atop_activate_request stack-allocates its response like the other ATOP calls, which also fixes an OOM leak of the POST-body buffer on the response-malloc-failure path. mbedTLS global config (allocator, record sizes) is left to the integrator: removed the SDK-side MBEDTLS_USER_CONFIG_FILE wiring and documented ownership in common/tls.h (the host may share the same mbedTLS instance). Co-Authored-By: Claude Opus 4.8 (1M context) --- CMakeLists.txt | 5 ++ common/tls.h | 12 +++++ .../posix/dp-management/dp_management_demo.c | 2 +- modules/iot-client/include/iot_client.h | 12 +++-- modules/iot-client/src/atop.c | 28 +++++----- modules/iot-client/src/atop_base.c | 9 ++-- .../iot-client/src/http_client_interface.c | 49 ++++++----------- modules/iot-client/src/iot_client.c | 39 +++++++------- modules/iot-client/src/iot_client_message.c | 52 +++++++------------ modules/iot-client/src/iot_dp.c | 28 +++++----- .../iot-client/test/iot_client_message_test.c | 22 ++++---- modules/iot-client/test/iot_dp_test.c | 7 ++- modules/rtc-tcp-client/src/tai_client.c | 4 +- modules/rtc-tcp-client/src/tai_internal.h | 2 +- modules/rtc-tcp-client/src/tai_pkt_log.c | 33 ++++++------ pal/pal_freertos.c | 2 +- 16 files changed, 144 insertions(+), 162 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index aea6f96..45ae340 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,6 +47,11 @@ agentic_kit_require_dependency(cJSON) agentic_kit_require_dependency(coreHTTP) agentic_kit_require_dependency(coreMQTT) +# mbedTLS uses its vendored default config. The SDK deliberately does not set +# MBEDTLS_USER_CONFIG_FILE or alter mbedTLS's global config (allocator, record +# sizes): the host application may share the same mbedTLS, so those are the +# integrator's to own (see common/tls.h). + if(NOT TARGET MbedTLS::mbedtls) add_subdirectory("${mbedtls_source_dir}" "${CMAKE_BINARY_DIR}/third_party/mbedtls" EXCLUDE_FROM_ALL) endif() diff --git a/common/tls.h b/common/tls.h index 73b86cc..ec18b22 100644 --- a/common/tls.h +++ b/common/tls.h @@ -16,6 +16,18 @@ * recursive mutex, releasing it during tcp_poll waits, so a worker thread and a * sender thread may interleave freely (the rtc model). The handshake DRBG is a * process-wide, lazily-seeded singleton shared by all connections. + * + * mbedTLS configuration is the integrator's responsibility. This SDK does NOT + * touch mbedTLS's process-global configuration -- the allocator + * (mbedtls_platform_set_calloc_free), threading callbacks + * (mbedtls_threading_set_alt), and record-buffer sizes + * (MBEDTLS_SSL_{IN,OUT}_CONTENT_LEN) are owned by the application, because the + * host may share the same mbedTLS instance and last-writer-wins on those + * globals would corrupt its state. To route mbedTLS through your own heap + * (e.g. the same allocator the SDK's pal uses, to keep one coalescing heap), + * call mbedtls_platform_set_calloc_free() yourself at startup -- once, + * single-threaded, before any SDK init -- and build mbedTLS with + * MBEDTLS_PLATFORM_MEMORY. */ #ifndef COMMON_TLS_H diff --git a/examples/posix/dp-management/dp_management_demo.c b/examples/posix/dp-management/dp_management_demo.c index 42c80d0..ab47974 100644 --- a/examples/posix/dp-management/dp_management_demo.c +++ b/examples/posix/dp-management/dp_management_demo.c @@ -170,7 +170,7 @@ static void on_schema_update(const char *schema_id, const char *new_schema, void * Returns the allocated cert (assign to client->cacert; free at shutdown) or NULL. */ static char *ensure_mqtt_ca(iot_client_t *client) { - if (client->mqtt_disable_tls || client->cacert || !client->mqtt_url) + if (client->mqtt_disable_tls || client->cacert || client->mqtt_url[0] == '\0') return NULL; char scheme[8] = {0}; diff --git a/modules/iot-client/include/iot_client.h b/modules/iot-client/include/iot_client.h index f5e4085..eb50442 100644 --- a/modules/iot-client/include/iot_client.h +++ b/modules/iot-client/include/iot_client.h @@ -124,8 +124,11 @@ typedef struct { } iot_on_boarding_config_t; -/* Opaque DP-layer context; defined privately in src/iot_dp.c. */ +/* Opaque DP-layer context; defined privately in src/iot_dp.c. Its storage is + * inlined into iot_client_t (dp_storage below) to avoid a per-session heap + * allocation; iot_dp.c _Static_asserts that the real struct fits. */ struct iot_dp_context; +#define IOT_DP_CONTEXT_STORAGE 128 /** * @brief IoT client instance structure @@ -139,8 +142,8 @@ struct iot_dp_context; char local_key[32]; // Local encryption key for LAN communication char schema_id[64]; // Device schema ID (from activation; stable key for schema upgrade) - char *https_url; // HTTPS endpoint URL for ATOP requests (SDK-owned) - char *mqtt_url; // MQTT broker URL (SDK-owned, mqtt:// or mqtts://) + char https_url[64]; // HTTPS endpoint URL for ATOP (inline; "" = unresolved) + char mqtt_url[64]; // MQTT broker URL (inline, mqtt://|mqtts://; "" = unresolved) char *schema; // Device schema JSON (dynamically allocated) iot_region_t region; // Server region (CN, US, EU, IN) @@ -153,7 +156,8 @@ struct iot_dp_context; struct mqtt_client *mqtt; // Internal MQTT client handle iot_message_callback_t message_callback; // User callback for incoming messages - struct iot_dp_context *dp; // DP management layer state (SDK-owned, opaque; NULL when inactive) + struct iot_dp_context *dp; // DP layer state; points into dp_storage, NULL when inactive + void *dp_storage[IOT_DP_CONTEXT_STORAGE / sizeof(void *)]; // inline storage for *dp (no heap) } iot_client_t; /** diff --git a/modules/iot-client/src/atop.c b/modules/iot-client/src/atop.c index 8e0fb27..2a52fe4 100644 --- a/modules/iot-client/src/atop.c +++ b/modules/iot-client/src/atop.c @@ -231,23 +231,21 @@ int atop_activate_request(const pal_t *pal, const activite_request_t *request, a .cert_bundle_attach = request->cert_bundle_attach}; /* ATOP service request send */ - atop_base_response_t *atop_response = pal->malloc(sizeof(atop_base_response_t)); - if (atop_response == NULL) { - return OPRT_MALLOC_FAILED; - } - rt = atop_base_request(pal,&atop_request, atop_response); + /* Stack-allocated like the other ATOP service calls; atop_base_response_free + * still releases the detached cJSON `result` subtree. Heap-allocating it was + * the lone inconsistency here and leaked `buffer` if that malloc failed. */ + atop_base_response_t atop_response = {0}; + rt = atop_base_request(pal, &atop_request, &atop_response); pal->free(buffer); if (OPRT_OK != rt) { log_error("atop_base_request error:%d", rt); - pal->free(atop_response); return rt; } - cJSON *result = atop_response->result; + cJSON *result = atop_response.result; if (result == NULL) { log_error("activate response no result"); - atop_base_response_free(pal,atop_response); - pal->free(atop_response); + atop_base_response_free(pal, &atop_response); return OPRT_COMMUNICATION_ERROR; } @@ -266,13 +264,11 @@ int atop_activate_request(const pal_t *pal, const activite_request_t *request, a if (!response->devid || !response->secret_key || !response->local_key) { log_error("activate response missing required fields"); atop_activate_response_free(pal, response); - atop_base_response_free(pal,atop_response); - pal->free(atop_response); + atop_base_response_free(pal, &atop_response); return OPRT_COMMUNICATION_ERROR; } - atop_base_response_free(pal,atop_response); - pal->free(atop_response); + atop_base_response_free(pal, &atop_response); return rt; } @@ -568,7 +564,7 @@ int atop_ai_token_get(const pal_t *pal, const ai_token_request_t *request, ai_to cJSON_free(json_str); } } - atop_base_response_free(pal,&atop_response); + atop_base_response_free(pal, &atop_response); if (response->token == NULL) { return OPRT_MALLOC_FAILED; @@ -794,7 +790,7 @@ int atop_upgrade_get(const pal_t *pal, const ota_upgrade_request_t *request, ota /* success=true but result=null → cloud has no upgrade configured for this device */ log_debug("atop_upgrade_get: no upgrade available (success=%d)", atop_response.success); atop_base_response_free(pal, &atop_response); - return OPRT_OK; + return OPRT_OK; /* no-upgrade is success; response->has_upgrade stays false */ } /* The result object contains firmware upgrade info. @@ -812,7 +808,7 @@ int atop_upgrade_get(const pal_t *pal, const ota_upgrade_request_t *request, ota if (chosen_url == NULL) { log_debug("atop_upgrade_get: no cdnUrl/httpsUrl (no upgrade)"); atop_base_response_free(pal, &atop_response); - return OPRT_OK; + return OPRT_OK; /* no-upgrade is success; response->has_upgrade stays false */ } response->has_upgrade = true; diff --git a/modules/iot-client/src/atop_base.c b/modules/iot-client/src/atop_base.c index 3d607ef..eef6b50 100644 --- a/modules/iot-client/src/atop_base.c +++ b/modules/iot-client/src/atop_base.c @@ -40,7 +40,7 @@ #define DEFAULT_RESPONSE_BUFFER_LEN (1024) #define AES_GCM128_NONCE_LEN 12 #define AES_GCM128_TAG_LEN 16 - #define URL_PARAM_SIGN_BUFFER_LEN (512) + #define URL_PARAM_SIGN_BUFFER_LEN (256) // Error check macro @@ -62,21 +62,19 @@ size_t printlen = 0; int i = 0; uint8_t digest[MD5SUM_LENGTH]; + (void)pal; /* sign buffer is stack-allocated now; pal unused here */ - char *buffer = pal->malloc(URL_PARAM_SIGN_BUFFER_LEN); - TUYA_CHECK_NULL_RETURN(buffer, OPRT_MALLOC_FAILED); + char buffer[URL_PARAM_SIGN_BUFFER_LEN]; for (i = 0; i < param_num; ++i) { int ret = snprintf(buffer + printlen, URL_PARAM_SIGN_BUFFER_LEN - printlen, "%s=%s||", params[i].key, params[i].value); if (ret < 0 || (size_t)ret >= URL_PARAM_SIGN_BUFFER_LEN - printlen) { - pal->free(buffer); return OPRT_COMMUNICATION_ERROR; } printlen += (size_t)ret; } int ret = snprintf(buffer + printlen, URL_PARAM_SIGN_BUFFER_LEN - printlen, "%s", (char *)key); if (ret < 0 || (size_t)ret >= URL_PARAM_SIGN_BUFFER_LEN - printlen) { - pal->free(buffer); return OPRT_COMMUNICATION_ERROR; } printlen += (size_t)ret; @@ -88,7 +86,6 @@ if (md5_ret == 0) md5_ret = mbedtls_md5_update(&ctx, (const uint8_t *)buffer, printlen); if (md5_ret == 0) md5_ret = mbedtls_md5_finish(&ctx, digest); mbedtls_md5_free(&ctx); - pal->free(buffer); if (md5_ret != 0) { log_error("MD5 computation failed: -0x%04x", -md5_ret); return OPRT_COMMUNICATION_ERROR; diff --git a/modules/iot-client/src/http_client_interface.c b/modules/iot-client/src/http_client_interface.c index 0a66ad1..030cf13 100644 --- a/modules/iot-client/src/http_client_interface.c +++ b/modules/iot-client/src/http_client_interface.c @@ -183,12 +183,9 @@ http_client_status_t http_client_request(const http_client_request_t *request, use_tls = false; } - // Allocate network context - struct HTTPNetworkContext *network_ctx = (struct HTTPNetworkContext *)pal->malloc(sizeof(struct HTTPNetworkContext)); - if (!network_ctx) { - log_error("Failed to allocate network context"); - return HTTP_CLIENT_ERROR; - } + // Network context lives on the stack (call-local, never returned). + struct HTTPNetworkContext network_ctx_buf; + struct HTTPNetworkContext *network_ctx = &network_ctx_buf; network_ctx->tcp_handle = NULL; network_ctx->use_tls = false; @@ -211,7 +208,6 @@ http_client_status_t http_client_request(const http_client_request_t *request, if (connect_ret != 0) { log_error("Failed to connect to %s:%d", request->host, request->port); - pal->free(network_ctx); return (connect_ret == OPRT_TLS_HANDSHAKE_FAILED) ? HTTP_CLIENT_TLS_ERROR : HTTP_CLIENT_ERROR; } @@ -223,15 +219,18 @@ http_client_status_t http_client_request(const http_client_request_t *request, .pNetworkContext = (NetworkContext_t *)network_ctx }; - // Prepare request headers buffer (allocate on heap to avoid stack overflow) + // One block holds both the request-header and response buffers (both alive + // through HTTPClient_Send) -- one allocation instead of two. #define REQUEST_HEADER_BUFFER_SIZE 1024 - uint8_t *request_header_buffer = (uint8_t *)pal->malloc(REQUEST_HEADER_BUFFER_SIZE); - if (!request_header_buffer) { - log_error("Failed to allocate request header buffer"); + #define RESPONSE_BUFFER_SIZE 4096 + uint8_t *http_buf = (uint8_t *)pal->malloc(REQUEST_HEADER_BUFFER_SIZE + RESPONSE_BUFFER_SIZE); + if (!http_buf) { + log_error("Failed to allocate HTTP buffers"); disconnect(network_ctx); - pal->free(network_ctx); return HTTP_CLIENT_ERROR; } + uint8_t *request_header_buffer = http_buf; + uint8_t *response_buffer = http_buf + REQUEST_HEADER_BUFFER_SIZE; HTTPRequestHeaders_t request_headers = { .pBuffer = request_header_buffer, .bufferLen = REQUEST_HEADER_BUFFER_SIZE, @@ -253,9 +252,8 @@ http_client_status_t http_client_request(const http_client_request_t *request, HTTPStatus_t http_status = HTTPClient_InitializeRequestHeaders(&request_headers, &request_info); if (http_status != HTTPSuccess) { log_error("Failed to initialize request headers: %d", http_status); - pal->free(request_header_buffer); + pal->free(http_buf); disconnect(network_ctx); - pal->free(network_ctx); return HTTP_CLIENT_ERROR; } @@ -268,23 +266,12 @@ http_client_status_t http_client_request(const http_client_request_t *request, strlen(request->headers[i].value)); if (http_status != HTTPSuccess) { log_error("Failed to add header %s: %d", request->headers[i].key, http_status); - pal->free(request_header_buffer); + pal->free(http_buf); disconnect(network_ctx); - pal->free(network_ctx); return HTTP_CLIENT_ERROR; } } - // Prepare response buffer (allocate on heap to avoid stack overflow) - #define RESPONSE_BUFFER_SIZE 4096 - uint8_t *response_buffer = (uint8_t *)pal->malloc(RESPONSE_BUFFER_SIZE); - if (!response_buffer) { - log_error("Failed to allocate response buffer"); - pal->free(request_header_buffer); - disconnect(network_ctx); - pal->free(network_ctx); - return HTTP_CLIENT_ERROR; - } HTTPResponse_t http_response = { .pBuffer = response_buffer, .bufferLen = RESPONSE_BUFFER_SIZE, @@ -330,11 +317,9 @@ http_client_status_t http_client_request(const http_client_request_t *request, } else { log_error("Failed to allocate response body buffer"); ret_status = HTTP_CLIENT_ERROR; - disconnect(network_ctx); - pal->free(network_ctx); response->internal = NULL; - pal->free(request_header_buffer); - pal->free(response_buffer); + pal->free(http_buf); + disconnect(network_ctx); return ret_status; } } @@ -363,10 +348,8 @@ http_client_status_t http_client_request(const http_client_request_t *request, } } - pal->free(request_header_buffer); - pal->free(response_buffer); + pal->free(http_buf); disconnect(network_ctx); - pal->free(network_ctx); return ret_status; } diff --git a/modules/iot-client/src/iot_client.c b/modules/iot-client/src/iot_client.c index 4be2685..b91ae3a 100644 --- a/modules/iot-client/src/iot_client.c +++ b/modules/iot-client/src/iot_client.c @@ -90,7 +90,7 @@ void iot_client_resolve_atop_host(iot_client_t *client, char *host_out, size_t h *port_out = IOT_DEFAULT_PORT; if (host_len == 0) return; host_out[0] = '\0'; - if (client->https_url) { + if (client->https_url[0] != '\0') { parse_host_port(client->https_url, host_out, host_len, port_out); } else { const char *h = iot_region_to_host(client->region, client->env); @@ -130,20 +130,23 @@ static int iot_client_dns_resolve(iot_client_t *client) if (strcmp(dns_resp.endpoints[i].key, mqtt_dns_key) == 0) { const char *scheme = client->mqtt_disable_tls ? "mqtt" : "mqtts"; const char *addr = dns_resp.endpoints[i].addr; - size_t url_len = strlen(scheme) + 3 + strlen(addr) + 1; - char *mqtt_url = (char *)client->pal->malloc(url_len); - if (mqtt_url) { - int sn = snprintf(mqtt_url, url_len, "%s://%s", scheme, addr); - if (sn < 0 || (size_t)sn >= url_len) { - client->pal->free(mqtt_url); - } else { - client->mqtt_url = mqtt_url; - log_info("IoT DNS %s: %s", mqtt_dns_key, mqtt_url); - } + int sn = snprintf(client->mqtt_url, sizeof(client->mqtt_url), + "%s://%s", scheme, addr); + if (sn < 0 || (size_t)sn >= sizeof(client->mqtt_url)) { + client->mqtt_url[0] = '\0'; /* too long: leave unresolved */ + log_warn("IoT DNS %s url too long, ignored", mqtt_dns_key); + } else { + log_info("IoT DNS %s: %s", mqtt_dns_key, client->mqtt_url); } } else if (strcmp(dns_resp.endpoints[i].key, "httpsUrl") == 0) { - client->https_url = pal_strdup(client->pal, dns_resp.endpoints[i].addr); - log_info("IoT DNS httpsUrl: %s", dns_resp.endpoints[i].addr); + const char *addr = dns_resp.endpoints[i].addr; + int sn = snprintf(client->https_url, sizeof(client->https_url), "%s", addr); + if (sn < 0 || (size_t)sn >= sizeof(client->https_url)) { + client->https_url[0] = '\0'; + log_warn("IoT DNS httpsUrl too long, ignored"); + } else { + log_info("IoT DNS httpsUrl: %s", client->https_url); + } } } iot_dns_url_config_response_free(client->pal, &dns_resp); @@ -225,7 +228,7 @@ IOT_API iot_client_t *iot_client_init(const iot_client_config_t *config) iot_client_dns_resolve(client); } - if (client->mqtt_url && config->mqtt_auto_connect) { + if (client->mqtt_url[0] != '\0' && config->mqtt_auto_connect) { int ret = iot_client_message_connect(client); if (ret != OPRT_OK) { log_error("MQTT connect failed: %d", ret); @@ -239,7 +242,7 @@ IOT_API iot_client_t *iot_client_init(const iot_client_config_t *config) char meta_host[64] = {0}; uint16_t meta_port = IOT_DEFAULT_PORT; const char *host; - if (client->https_url) { + if (client->https_url[0] != '\0') { parse_host_port(client->https_url, meta_host, sizeof(meta_host), &meta_port); host = meta_host; } else { @@ -286,12 +289,6 @@ IOT_API void iot_client_deinit(iot_client_t *client) } iot_client_message_disconnect(client); iot_dp_deinit(client); /* after disconnect: no more dispatch on the process thread */ - if (client->https_url) { - client->pal->free(client->https_url); - } - if (client->mqtt_url) { - client->pal->free(client->mqtt_url); - } if (client->schema) { client->pal->free(client->schema); } diff --git a/modules/iot-client/src/iot_client_message.c b/modules/iot-client/src/iot_client_message.c index f5a1be4..ce1f4a6 100644 --- a/modules/iot-client/src/iot_client_message.c +++ b/modules/iot-client/src/iot_client_message.c @@ -44,24 +44,19 @@ static void mqtt_message_handler(const char *topic, size_t topic_len, static int iot_client_message_try_connect(iot_client_t *client) { - size_t devid_len = strlen(client->devid); - size_t topic_buf_len = 17 + devid_len + 1; - char *subscribe_topic = (char *)client->pal->malloc(topic_buf_len); - if (!subscribe_topic) - return OPRT_MALLOC_FAILED; - int sn_ret = snprintf(subscribe_topic, topic_buf_len, + /* Connect is rare (not a hot path); the subscribe topic lives on the stack + * for the call -- mqtt_client_create_with_config copies it (mqtt.c:327), so + * no malloc/free is needed. */ + char subscribe_topic[64]; + int sn_ret = snprintf(subscribe_topic, sizeof(subscribe_topic), "smart/device/in/%s", client->devid); - if (sn_ret < 0 || (size_t)sn_ret >= topic_buf_len) { - client->pal->free(subscribe_topic); + if (sn_ret < 0 || (size_t)sn_ret >= (int)sizeof(subscribe_topic)) return OPRT_COMMUNICATION_ERROR; - } char password[17] = {0}; int md5_ret = iot_md5_password(client->secret_key, password); - if (md5_ret != 0) { - client->pal->free(subscribe_topic); + if (md5_ret != 0) return OPRT_COMMUNICATION_ERROR; - } mqtt_tls_config_t tls_cfg = { .cacert = client->cacert, .cert_bundle_attach = client->cert_bundle_attach }; @@ -80,7 +75,6 @@ static int iot_client_message_try_connect(iot_client_t *client) client->mqtt = mqtt_client_create_with_config(&mqtt_cfg); if (!client->mqtt) { log_error("Failed to create MQTT client"); - client->pal->free(subscribe_topic); return OPRT_COMMUNICATION_ERROR; } @@ -89,7 +83,6 @@ static int iot_client_message_try_connect(iot_client_t *client) log_error("Failed to connect to MQTT broker: %d", ret); mqtt_client_destroy(client->mqtt); client->mqtt = NULL; - client->pal->free(subscribe_topic); return (ret == OPRT_TLS_HANDSHAKE_FAILED) ? OPRT_TLS_HANDSHAKE_FAILED : OPRT_COMMUNICATION_ERROR; } @@ -98,18 +91,16 @@ static int iot_client_message_try_connect(iot_client_t *client) log_error("Failed to subscribe to %s", subscribe_topic); mqtt_client_destroy(client->mqtt); client->mqtt = NULL; - client->pal->free(subscribe_topic); return OPRT_COMMUNICATION_ERROR; } log_info("MQTT connected and subscribed to %s", subscribe_topic); - client->pal->free(subscribe_topic); return OPRT_OK; } int iot_client_message_connect(iot_client_t *client) { - if (!client || !client->mqtt_url || client->devid[0] == '\0') { + if (!client || client->mqtt_url[0] == '\0' || client->devid[0] == '\0') { return OPRT_INVALID_PARAMETER; } @@ -161,29 +152,26 @@ int iot_client_message_publish(iot_client_t *client, return OPRT_COMMUNICATION_ERROR; } - size_t topic_len = 18 + strlen(client->devid) + 1; - char *topic = (char *)client->pal->malloc(topic_len); - if (!topic) { + /* Outbound topic is a short fixed format (prefix 17 + devid <=31 < 64); + * build it on the stack per publish (no persistent field), matching the + * subscribe topic in iot_client_message_try_connect. The publish path + * already mallocs the ciphertext, so a stack snprintf adds no churn. */ + char pub_topic[64]; + int sn = snprintf(pub_topic, sizeof(pub_topic), + "smart/device/out/%s", client->devid); + if (sn < 0 || (size_t)sn >= sizeof(pub_topic)) { client->pal->free(encrypted); - return OPRT_MALLOC_FAILED; - } - int sn = snprintf(topic, topic_len, "smart/device/out/%s", client->devid); - if (sn < 0 || (size_t)sn >= topic_len) { - client->pal->free(encrypted); - client->pal->free(topic); return OPRT_COMMUNICATION_ERROR; } - ret = mqtt_client_publish(client->mqtt, topic, encrypted, encrypted_len); + ret = mqtt_client_publish(client->mqtt, pub_topic, encrypted, encrypted_len); client->pal->free(encrypted); - if (ret != 0) { - log_error("Failed to publish to %s", topic); - client->pal->free(topic); + log_error("Failed to publish to %s", pub_topic); return OPRT_COMMUNICATION_ERROR; } - log_debug("Published encrypted message to %s (%u bytes)", topic, (unsigned)encrypted_len); - client->pal->free(topic); + log_debug("Published encrypted message to %s (%u bytes)", + pub_topic, (unsigned)encrypted_len); return OPRT_OK; } diff --git a/modules/iot-client/src/iot_dp.c b/modules/iot-client/src/iot_dp.c index 911b0ab..e8c3c68 100644 --- a/modules/iot-client/src/iot_dp.c +++ b/modules/iot-client/src/iot_dp.c @@ -56,6 +56,10 @@ struct iot_dp_context { iot_dp_save_callback_t save_cb; void *save_cb_user; }; +/* dp_storage in iot_client_t holds this struct inline; fail the build if it grows past it. */ +_Static_assert(sizeof(struct iot_dp_context) <= IOT_DP_CONTEXT_STORAGE, + "iot_dp_context outgrew IOT_DP_CONTEXT_STORAGE (bump it in iot_client.h)"); + typedef enum { DP_SEL_ALL, DP_SEL_DIRTY, DP_SEL_ONE } dp_sel_t; /* ============================================================================ @@ -98,18 +102,13 @@ static struct iot_dp_context *dp_ensure_context(iot_client_t *client) if (client->dp) return client->dp; const pal_t *pal = client->pal; - struct iot_dp_context *ctx = (struct iot_dp_context *)pal->malloc(sizeof(*ctx)); - if (!ctx) { - log_error("dp: context allocation failed"); - return NULL; - } + struct iot_dp_context *ctx = (struct iot_dp_context *)client->dp_storage; memset(ctx, 0, sizeof(*ctx)); ctx->loose = true; ctx->mutex = pal->mutex_create(); if (!ctx->mutex) { log_error("dp: mutex creation failed"); - pal->free(ctx); - return NULL; + return NULL; /* dp_storage is inline; nothing to free */ } client->dp = ctx; return ctx; @@ -523,12 +522,12 @@ static char *dp_build_state_json(iot_client_t *client, bool include_raw) if (dp_add_value(dps, key, e, pal) != OPRT_OK) { cJSON_Delete(root); return NULL; } } + /* cJSON's allocator is the pal allocator (cJSON_InitHooks in iot_init), so the + * printed string is already pal-owned -- return it directly; callers free it + * via pal->free. Avoids a second malloc + copy per state build. */ char *s = cJSON_PrintUnformatted(root); cJSON_Delete(root); - if (!s) return NULL; - char *copy = pal_strdup(pal, s); - cJSON_free(s); - return copy; + return s; } /* Build a {"protocol":3,"t":..,"data":{"dps":{...}}} report (lock held). */ @@ -561,13 +560,13 @@ static char *dp_build_report_json(iot_client_t *client, dp_sel_t sel, uint8_t on cnt++; } + /* cJSON allocs via the pal allocator -- return the printed string directly + * (caller frees via pal->free); avoids a second malloc + copy per report. */ char *s = cJSON_PrintUnformatted(root); cJSON_Delete(root); if (!s) return NULL; - char *copy = pal_strdup(pal, s); - cJSON_free(s); *count_out = cnt; - return copy; + return s; } /* Build the current snapshot under the lock, then fire the save callback OUTSIDE it. */ @@ -1138,5 +1137,4 @@ void iot_dp_deinit(iot_client_t *client) dp_entries_free(pal, entries, count); pal->mutex_destroy(ctx->mutex); client->dp = NULL; - pal->free(ctx); } diff --git a/modules/iot-client/test/iot_client_message_test.c b/modules/iot-client/test/iot_client_message_test.c index e1eb587..fe05bf7 100644 --- a/modules/iot-client/test/iot_client_message_test.c +++ b/modules/iot-client/test/iot_client_message_test.c @@ -224,7 +224,7 @@ static int test_raw_message(void) strncpy((char *)client->devid, TEST_DEVID, sizeof(client->devid) - 1); strncpy((char *)client->secret_key, TEST_SECRET_KEY, sizeof(client->secret_key) - 1); strncpy((char *)client->local_key, TEST_LOCAL_KEY, sizeof(client->local_key) - 1); - client->mqtt_url = TEST_MQTT_URL; + snprintf(client->mqtt_url, sizeof(client->mqtt_url), "%s", TEST_MQTT_URL); client->cacert = g_cacert; client->message_callback = test_message_callback; @@ -259,7 +259,7 @@ static int test_raw_message(void) } /* don't free mqtt_url/cacert — they point to static/global data */ - client->mqtt_url = NULL; + client->mqtt_url[0] = '\0'; client->cacert = NULL; pal->free(client); return result; @@ -280,7 +280,7 @@ static int test_invalid_format_message(void) strncpy((char *)client->devid, TEST_DEVID, sizeof(client->devid) - 1); strncpy((char *)client->secret_key, TEST_SECRET_KEY, sizeof(client->secret_key) - 1); strncpy((char *)client->local_key, TEST_LOCAL_KEY, sizeof(client->local_key) - 1); - client->mqtt_url = "mqtts://127.0.0.1:11886"; + snprintf(client->mqtt_url, sizeof(client->mqtt_url), "%s", "mqtts://127.0.0.1:11886"); client->cacert = g_cacert; client->message_callback = test_message_callback; @@ -306,7 +306,7 @@ static int test_invalid_format_message(void) } } - client->mqtt_url = NULL; + client->mqtt_url[0] = '\0'; client->cacert = NULL; pal->free(client); return result; @@ -327,7 +327,7 @@ static int test_decrypt_fail_wrong_key(void) strncpy((char *)client->devid, TEST_DEVID, sizeof(client->devid) - 1); strncpy((char *)client->secret_key, TEST_SECRET_KEY, sizeof(client->secret_key) - 1); strncpy((char *)client->local_key, TEST_LOCAL_KEY, sizeof(client->local_key) - 1); - client->mqtt_url = "mqtts://127.0.0.1:11887"; + snprintf(client->mqtt_url, sizeof(client->mqtt_url), "%s", "mqtts://127.0.0.1:11887"); client->cacert = g_cacert; client->message_callback = test_message_callback; @@ -353,7 +353,7 @@ static int test_decrypt_fail_wrong_key(void) } } - client->mqtt_url = NULL; + client->mqtt_url[0] = '\0'; client->cacert = NULL; pal->free(client); return result; @@ -374,7 +374,7 @@ static int test_encrypted_message(void) strncpy((char *)client->devid, TEST_DEVID, sizeof(client->devid) - 1); strncpy((char *)client->secret_key, TEST_SECRET_KEY, sizeof(client->secret_key) - 1); strncpy((char *)client->local_key, TEST_LOCAL_KEY, sizeof(client->local_key) - 1); - client->mqtt_url = TEST_MQTT_URL; + snprintf(client->mqtt_url, sizeof(client->mqtt_url), "%s", TEST_MQTT_URL); client->cacert = g_cacert; client->message_callback = test_message_callback; @@ -397,7 +397,7 @@ static int test_encrypted_message(void) if (ret != OPRT_OK) { printf(" iot_client_message_publish failed: %d\n", ret); iot_client_message_disconnect(client); - client->mqtt_url = NULL; + client->mqtt_url[0] = '\0'; client->cacert = NULL; pal->free(client); return -1; @@ -418,7 +418,7 @@ static int test_encrypted_message(void) result = 0; } - client->mqtt_url = NULL; + client->mqtt_url[0] = '\0'; client->cacert = NULL; pal->free(client); return result; @@ -442,8 +442,8 @@ static int test_iot_client_init_autoconnect_no_url(void) } int result = 0; - if (client->mqtt_url != NULL) { - printf(" expected mqtt_url=NULL (no devid → DNS skipped), got '%s'\n", client->mqtt_url); + if (client->mqtt_url[0] != '\0') { + printf(" expected mqtt_url unset (no devid → DNS skipped), got '%s'\n", client->mqtt_url); result = -1; } if (client->mqtt != NULL) { diff --git a/modules/iot-client/test/iot_dp_test.c b/modules/iot-client/test/iot_dp_test.c index 85653ff..e8c569f 100644 --- a/modules/iot-client/test/iot_dp_test.c +++ b/modules/iot-client/test/iot_dp_test.c @@ -538,7 +538,7 @@ static int test_report_roundtrip(void) strncpy(client->devid, TEST_DEVID, sizeof(client->devid) - 1); strncpy(client->secret_key, TEST_SECRET_KEY, sizeof(client->secret_key) - 1); strncpy(client->local_key, TEST_LOCAL_KEY, sizeof(client->local_key) - 1); - client->mqtt_url = MSG_MOCK_URL; + snprintf(client->mqtt_url, sizeof(client->mqtt_url), "%s", MSG_MOCK_URL); client->cacert = g_cacert; client->message_callback = msg_callback; client->schema = pal_strdup(pal, TEST_SCHEMA); @@ -567,7 +567,7 @@ static int test_report_roundtrip(void) iot_client_message_disconnect(client); iot_dp_deinit(client); if (client->schema) pal->free(client->schema); - client->mqtt_url = NULL; + client->mqtt_url[0] = '\0'; client->cacert = NULL; pal->free(client); return rc; @@ -584,7 +584,7 @@ static int test_schema_check_update(void) strncpy(client->devid, TEST_DEVID, sizeof(client->devid) - 1); strncpy(client->secret_key, TEST_SECRET_KEY, sizeof(client->secret_key) - 1); strncpy(client->schema_id, "dp_test_schema", sizeof(client->schema_id) - 1); - client->https_url = pal_strdup(pal, ATOP_MOCK_URL); + snprintf(client->https_url, sizeof(client->https_url), "%s", ATOP_MOCK_URL); client->cacert = g_cacert; /* Start with an OLD schema carrying live values, to prove the upgrade keeps them. */ client->schema = pal_strdup(pal, @@ -617,7 +617,6 @@ static int test_schema_check_update(void) rc = 0; out: iot_dp_deinit(client); - if (client->https_url) pal->free(client->https_url); if (client->schema) pal->free(client->schema); client->cacert = NULL; pal->free(client); diff --git a/modules/rtc-tcp-client/src/tai_client.c b/modules/rtc-tcp-client/src/tai_client.c index 5c2a3de..e861230 100644 --- a/modules/rtc-tcp-client/src/tai_client.c +++ b/modules/rtc-tcp-client/src/tai_client.c @@ -122,7 +122,7 @@ static void log_send_packet(tai_ctx_t *ctx, if (tai_packet_decode(ctx->proto_ver, app_bytes, app_len, &pkt_type, attrs, TAI_MAX_ATTRS, &attr_count, &payload, &payload_len) == TAI_OK) { - tai_log_packet(ctx->pal, ctx->proto_ver, 1, + tai_log_packet(ctx->proto_ver, 1, pkt_type, attrs, attr_count, payload, payload_len); } } @@ -628,7 +628,7 @@ static int process_app_packet(tai_ctx_t *ctx, return TAI_PROTO_ERR_PKT_DECODE; /* fatal cause, returned to the worker */ } - tai_log_packet(ctx->pal, ctx->proto_ver, 0, + tai_log_packet(ctx->proto_ver, 0, pkt_type, attrs, attr_count, payload, payload_len); /* Returns TAI_OK, a TAI_PROTO_ERR_* detail, or TAI_RX_PEER_CLOSE|code. */ diff --git a/modules/rtc-tcp-client/src/tai_internal.h b/modules/rtc-tcp-client/src/tai_internal.h index 0f6056a..530a004 100644 --- a/modules/rtc-tcp-client/src/tai_internal.h +++ b/modules/rtc-tcp-client/src/tai_internal.h @@ -501,7 +501,7 @@ int tai_crypto_derive_keys(uint8_t proto_ver, /* * tai_pkt_log.c */ -void tai_log_packet(const pal_t *pal, uint8_t proto_ver, +void tai_log_packet(uint8_t proto_ver, int is_send, uint8_t pkt_type, const tai_attr_t *attrs, int attr_count, diff --git a/modules/rtc-tcp-client/src/tai_pkt_log.c b/modules/rtc-tcp-client/src/tai_pkt_log.c index 487c1b6..c7d4d4b 100644 --- a/modules/rtc-tcp-client/src/tai_pkt_log.c +++ b/modules/rtc-tcp-client/src/tai_pkt_log.c @@ -304,12 +304,13 @@ static void put_b64(char *buf, size_t cap, size_t *pos, const uint8_t *data, size_t len) { size_t src_limit = len > 192 ? 192 : len; - char tmp[260]; - b64enc(data, src_limit, tmp, sizeof(tmp)); - if (len > 192) - bput(buf, cap, pos, "\"%s...\"", tmp); - else - bput(buf, cap, pos, "\"%s\"", tmp); + /* Encode straight into the output buffer -- no intermediate stack copy. + * This runs on the receive-worker stack, so keep the frame small; b64enc + * already honours the remaining capacity and NUL-terminates. */ + if (*pos + 1 < cap) buf[(*pos)++] = '"'; + *pos += b64enc(data, src_limit, buf + *pos, cap > *pos ? cap - *pos : 0); + if (len > 192) bput(buf, cap, pos, "..."); + if (*pos + 1 < cap) buf[(*pos)++] = '"'; } /* Write one attribute value */ @@ -467,19 +468,21 @@ static void put_payload(char *buf, size_t cap, size_t *pos, /* ========================================================================= * tai_log_packet * ========================================================================= */ -/* Heap-allocated formatting buffer; tuned to fit a typical structured log - * line after string/base64 truncation in put_jstr / put_b64. Long fields - * are truncated gracefully when this fills. */ -#define TAI_LOG_BUF_SIZE 512 +/* Stack formatting buffer (lives on the caller's stack -- the receive worker or + * the sender thread); tuned to fit a typical structured log line after + * string/base64 truncation in put_jstr / put_b64. Long fields are truncated + * gracefully when this fills. */ +#define TAI_LOG_BUF_SIZE 1024 -void tai_log_packet(const pal_t *pal, uint8_t proto_ver, +void tai_log_packet(uint8_t proto_ver, int is_send, uint8_t pkt_type, const tai_attr_t *attrs, int attr_count, const uint8_t *payload, size_t payload_len) { /* Skip keepalive noise */ - if (pkt_type == TAI_PKT_PING || pkt_type == TAI_PKT_PONG) return; + if (pkt_type == TAI_PKT_PING || pkt_type == TAI_PKT_PONG) + return; /* --- Decide effective log level / volume for media streams ---------- * @@ -552,8 +555,9 @@ void tai_log_packet(const pal_t *pal, uint8_t proto_ver, } } - char *buf = (char *)pal->malloc(TAI_LOG_BUF_SIZE); - if (!buf) return; + /* Written strictly sequentially via bput() and NUL-terminated at buf[pos] + * below, so no zero-init is needed (avoids a memset on the log hot path). */ + char buf[TAI_LOG_BUF_SIZE]; size_t cap = TAI_LOG_BUF_SIZE - 1; size_t pos = 0; @@ -579,5 +583,4 @@ void tai_log_packet(const pal_t *pal, uint8_t proto_ver, buf[pos] = '\0'; log_emit(log_level, "[" TAG "] %s: %s", dir, buf); - pal->free(buf); } diff --git a/pal/pal_freertos.c b/pal/pal_freertos.c index 9b7ff1e..c6d350e 100644 --- a/pal/pal_freertos.c +++ b/pal/pal_freertos.c @@ -62,7 +62,7 @@ /* Worker task tunables — override via -D at build time if needed. */ #ifndef PAL_FR_TASK_STACK_WORDS -#define PAL_FR_TASK_STACK_WORDS (6 * 1024 / sizeof(StackType_t)) +#define PAL_FR_TASK_STACK_WORDS 4096 #endif #ifndef PAL_FR_TASK_PRIORITY #define PAL_FR_TASK_PRIORITY (tskIDLE_PRIORITY + 5)