From 7b581c83c61ffcffa1fc99991c78b7506c873e3e Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 17 Sep 2026 08:36:04 -0500 Subject: [PATCH 1/4] Fix non-TLS build combos --- .github/workflows/feature-combo-check.yml | 83 +++++++++++++++++++++++ BROKER.md | 2 + ChangeLog.md | 18 +++++ wolfmqtt/mqtt_client.h | 7 +- wolfmqtt/mqtt_types.h | 21 +++++- 5 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/feature-combo-check.yml diff --git a/.github/workflows/feature-combo-check.yml b/.github/workflows/feature-combo-check.yml new file mode 100644 index 000000000..3cd52c51f --- /dev/null +++ b/.github/workflows/feature-combo-check.yml @@ -0,0 +1,83 @@ +name: Feature Combination Build Check + +# Compiles the library sources under feature-macro combinations that the +# autotools jobs never configure, so a header that only builds in the default +# combination fails here rather than in a user's port. + +on: + push: + branches: [ 'master', 'main', 'release/**' ] + pull_request: + branches: [ '*' ] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + compile: + name: ${{ matrix.name }} + runs-on: ubuntu-22.04 + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + include: + - name: baseline + defs: "" + - name: no-stdio + defs: "-DWOLFMQTT_NO_STDIO" + - name: no-stdio-v5-sn + defs: "-DWOLFMQTT_NO_STDIO -DWOLFMQTT_V5 -DWOLFMQTT_SN" + - name: no-stdio-broker + defs: "-DWOLFMQTT_NO_STDIO -DWOLFMQTT_BROKER" + - name: no-stdio-broker-v5-multithread + defs: "-DWOLFMQTT_NO_STDIO -DWOLFMQTT_BROKER -DWOLFMQTT_V5 -DWOLFMQTT_MULTITHREAD" + - name: custom-printf-broker + defs: "-DWOLFMQTT_NO_STDIO -DWOLFMQTT_BROKER -DWOLFMQTT_CUSTOM_PRINTF -DPRINTF(f,...)=user_printf(f,##__VA_ARGS__)" + - name: broker-no-log + defs: "-DWOLFMQTT_BROKER -DWOLFMQTT_BROKER_NO_LOG" + - name: no-error-strings + defs: "-DWOLFMQTT_NO_ERROR_STRINGS" + - name: custom-printf + defs: "-DWOLFMQTT_CUSTOM_PRINTF -DPRINTF(f,...)=user_printf(f,##__VA_ARGS__)" + - name: custom-malloc + defs: "-DWOLFMQTT_CUSTOM_MALLOC -DWOLFMQTT_MALLOC(s)=user_malloc(s) -DWOLFMQTT_FREE(p)=user_free(p)" + - name: session-id-track-forced + defs: "-DWOLFMQTT_SESSION_ID_TRACK" + - name: capped-qos-no-replay + defs: "-DWOLFMQTT_MAX_QOS=1 -DWOLFMQTT_NO_SESSION_REPLAY" + - name: capped-qos-no-replay-forced-track + defs: "-DWOLFMQTT_MAX_QOS=1 -DWOLFMQTT_NO_SESSION_REPLAY -DWOLFMQTT_SESSION_ID_TRACK" + - name: v5-sn-multithread-nonblock + defs: "-DWOLFMQTT_V5 -DWOLFMQTT_SN -DWOLFMQTT_MULTITHREAD -DWOLFMQTT_NONBLOCK" + - name: v5-static-memory + defs: "-DWOLFMQTT_V5 -DWOLFMQTT_USE_STATIC_MEMORY" + + steps: + - uses: actions/checkout@v4 + + # WOLFMQTT_USER_SETTINGS keeps the generated wolfmqtt/options.h out of + # the build, so each combination is exactly the macros listed above. + # The declarations satisfy the custom-printf and custom-malloc ports. + - name: Write user_settings.h + run: | + cat > user_settings.h <<'EOF' + extern int user_printf(const char* fmt, ...); + extern void* user_malloc(unsigned long size); + extern void user_free(void* ptr); + EOF + + # DEFS goes through the environment rather than being spliced into the + # command, so the parentheses in the function-like macros reach gcc + # instead of being parsed by the shell. + - name: Compile sources + env: + DEFS: ${{ matrix.defs }} + run: | + for src in src/*.c; do + echo "=== ${{ matrix.name }}: $src" + gcc -c -Werror -Wall -Wextra -I. \ + -DWOLFMQTT_USER_SETTINGS -DWOLFMQTT_NO_TLS $DEFS \ + "$src" -o /dev/null + done diff --git a/BROKER.md b/BROKER.md index c35cd1540..85ef10067 100644 --- a/BROKER.md +++ b/BROKER.md @@ -85,6 +85,8 @@ All broker features are enabled by default and can be disabled at build time to The maximum QoS the broker negotiates is capped by `--enable-max-qos=<0,1,2>` (default 2). Setting it to 1 or 0 compiles out the QoS 2 state machine and shrinks the broker. +`WOLFMQTT_NO_STDIO` turns `PRINTF` into a no-op, so it implies `WOLFMQTT_BROKER_NO_LOG` and the broker log calls are stripped. A port that supplies its own `PRINTF` through `WOLFMQTT_CUSTOM_PRINTF` keeps its logging. + ## Static memory tuning When built with `WOLFMQTT_STATIC_MEMORY`, the broker uses fixed-size arrays instead of dynamic allocation. The limits below can be overridden via CFLAGS at build time. diff --git a/ChangeLog.md b/ChangeLog.md index f1b1c6ada..ea0570eb9 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -205,6 +205,24 @@ ClientId that populated it. Reusing one `MqttClient` under a new ClientId no longer inherits the previous Session's pending packet ids when the server answers Session Present = 1 [MQTT-3.1.3-2] (#595) + - `WOLFMQTT_NO_STDIO` builds failed to compile because the default + `WOLFMQTT_MALLOC` / `WOLFMQTT_FREE` expand to `malloc()` / `free()` but + `` was only included alongside ``. The header is now + included with the allocator defaults that need it, which also fixes + `WOLFMQTT_CUSTOM_PRINTF` ports such as MPLAB Harmony, and a port that + supplies both macros still pulls in no libc header (#619) + - `WOLFMQTT_CUSTOM_MALLOC` without `WOLFMQTT_MALLOC` and `WOLFMQTT_FREE` + now fails in the header with a message naming both macros, instead of an + implicit declaration reported from inside `mqtt_client.c` (#619) + - `WOLFMQTT_SESSION_ID_TRACK` is derived from `WOLFMQTT_MAX_QOS` and + `WOLFMQTT_NO_SESSION_REPLAY`, so a build that also defined it on the + command line hit a macro redefinition, fatal under `-Werror`. Such a + define is now discarded rather than redefining the macro (#619) + - `WOLFMQTT_NO_STDIO` broker builds failed to compile because the log + calls drop their arguments once `PRINTF` is a no-op, leaving the log + string sanitizer with no callers. `WOLFMQTT_NO_STDIO` now implies + `WOLFMQTT_BROKER_NO_LOG`, except where `WOLFMQTT_CUSTOM_PRINTF` supplies + a working sink ### v2.1.0 (07/02/2026) Release 2.1.0 has been developed according to wolfSSL's development and QA diff --git a/wolfmqtt/mqtt_client.h b/wolfmqtt/mqtt_client.h index cad3004c8..9487ecc7c 100644 --- a/wolfmqtt/mqtt_client.h +++ b/wolfmqtt/mqtt_client.h @@ -332,7 +332,12 @@ typedef struct _MqttReplayMsg { /* [MQTT-3.1.3-2] The ClientId identifies the Client and its Session. Both the * inbound QoS 2 de-duplication table and the outbound Session replay store use * a fingerprint of it to tell a resumed Session from a different one, so the - * fingerprint exists whenever either of them does. */ + * fingerprint exists whenever either of them does. Derived from those two + * settings rather than set by hand: a command-line or user_settings.h define + * is discarded here, since on its own it would redefine the macro and select + * a build that records a fingerprint nothing reads. Use WOLFMQTT_MAX_QOS and + * WOLFMQTT_NO_SESSION_REPLAY to control it. */ +#undef WOLFMQTT_SESSION_ID_TRACK #if (WOLFMQTT_MAX_QOS >= 2) || !defined(WOLFMQTT_NO_SESSION_REPLAY) #define WOLFMQTT_SESSION_ID_TRACK /* Bytes of the ClientId retained for that comparison. It is an exact diff --git a/wolfmqtt/mqtt_types.h b/wolfmqtt/mqtt_types.h index 3196bf887..651ed4f23 100644 --- a/wolfmqtt/mqtt_types.h +++ b/wolfmqtt/mqtt_types.h @@ -309,13 +309,24 @@ enum MqttPacketResponseCodes { #endif #endif +/* Allocator backstop, mirroring the XMEMCHR one above. The malloc()/free() + * defaults need whether or not stdio is available, so it is + * included beside them rather than with . A custom-malloc port that + * intentionally avoids gets an explicit #error naming both macros + * it must define, instead of a confusing implicit-declaration diagnostic from + * the first WOLFMQTT_FREE call site in mqtt_client.c. */ #ifndef WOLFMQTT_CUSTOM_MALLOC + #if !defined(WOLFMQTT_MALLOC) || !defined(WOLFMQTT_FREE) + #include + #endif #ifndef WOLFMQTT_MALLOC #define WOLFMQTT_MALLOC(s) malloc((s)) #endif #ifndef WOLFMQTT_FREE #define WOLFMQTT_FREE(p) {void* xp = (p); if((xp)) free((xp));} #endif +#elif !defined(WOLFMQTT_MALLOC) || !defined(WOLFMQTT_FREE) + #error "WOLFMQTT_CUSTOM_MALLOC set: define WOLFMQTT_MALLOC/WOLFMQTT_FREE" #endif #ifndef WOLFMQTT_PACK @@ -376,8 +387,6 @@ enum MqttPacketResponseCodes { #endif #ifndef WOLFMQTT_NO_STDIO - #include - #include #include #else #undef PRINTF @@ -417,6 +426,14 @@ enum MqttPacketResponseCodes { #undef WOLFMQTT_DEBUG_SOCKET #endif +/* PRINTF became a no-op above, so the broker log calls have no sink and drop + * their arguments, leaving the helpers that only feed them with no callers. + * A port that supplies its own PRINTF keeps its logging. */ +#if defined(WOLFMQTT_NO_STDIO) && !defined(WOLFMQTT_CUSTOM_PRINTF) && \ + !defined(WOLFMQTT_BROKER_NO_LOG) + #define WOLFMQTT_BROKER_NO_LOG +#endif + #ifdef WOLFMQTT_DEBUG_TRACE #define MQTT_TRACE_ERROR(err) ({ PRINTF("ERROR: %d (%s:%d)", err, __FUNCTION__, __LINE__); err; }) #define MQTT_TRACE_MSG(msg) PRINTF("%s: (%s:%d)", msg, __FUNCTION__, __LINE__); From dced10790604f19e8127905a28a350c229008a31 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 17 Sep 2026 08:45:52 -0500 Subject: [PATCH 2/4] Fix static analysis issues --- examples/azure/azureiothub.c | 2 +- examples/mqttnet.c | 35 +++++++++++++++++++++++++---------- src/mqtt_sn_packet.c | 13 +++++++++---- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/examples/azure/azureiothub.c b/examples/azure/azureiothub.c index de3393775..cc6dc6cd7 100644 --- a/examples/azure/azureiothub.c +++ b/examples/azure/azureiothub.c @@ -542,9 +542,9 @@ int azureiothub_test(MQTTCtx *mqttCtx) } } while (1); } + #ifdef WOLFMQTT_NO_TIME FALL_THROUGH; - #ifdef WOLFMQTT_NO_TIME /* Manual keep-alive ping. With automatic keep-alive compiled in, the * core client sends PINGREQ itself and WMQ_WAIT_MSG never breaks out to * this state, so it is only compiled for WOLFMQTT_NO_TIME builds. */ diff --git a/examples/mqttnet.c b/examples/mqttnet.c index a5aeabe56..2f5c272a7 100644 --- a/examples/mqttnet.c +++ b/examples/mqttnet.c @@ -1618,6 +1618,9 @@ static int NetWrite(void *context, const byte* buf, int buf_len, SocketContext *sock = (SocketContext*)context; MQTTCtx* mqttCtx; int rc; + /* SOCK_SEND returns ssize_t on POSIX; hold the result at that width so the + * checks below run before any narrowing to int. */ + long sent; SOERROR_T so_error = 0; #ifndef WOLFMQTT_NO_TIMEOUT struct timeval tv; @@ -1663,12 +1666,12 @@ static int NetWrite(void *context, const byte* buf, int buf_len, sizeof(tv)); #endif - rc = (int)SOCK_SEND(sock->fd, buf, buf_len, 0); + sent = (long)SOCK_SEND(sock->fd, buf, buf_len, 0); #if defined(WOLFMQTT_DEBUG_SOCKET) - PRINTF("info: SOCK_SEND(%d) returned %d, buf_len is %d", - buf_len, rc, buf_len); + PRINTF("info: SOCK_SEND(%d) returned %ld, buf_len is %d", + buf_len, sent, buf_len); #endif - if (rc == -1) { + if (sent < 0) { { /* Get error */ GET_SOCK_ERROR(sock->fd, SOL_SOCKET, SO_ERROR, so_error); @@ -1691,6 +1694,14 @@ static int NetWrite(void *context, const byte* buf, int buf_len, PRINTF("NetWrite: Error %d", so_error); } } + else { + /* Never report more than the caller asked to write; guards against a + * platform send() that claims more than the requested length. */ + if (sent > (long)buf_len) { + sent = (long)buf_len; + } + rc = (int)sent; + } (void)timeout_ms; @@ -1754,6 +1765,9 @@ static int NetRead_ex(void *context, byte* buf, int buf_len, /* Loop until buf_len has been read, error or timeout */ while (bytes < buf_len) { int do_read = 0; + /* SOCK_RECV returns ssize_t on POSIX; hold the result at that width so + * the checks below run before any narrowing to int. */ + long recvd; #ifndef WOLFMQTT_NO_TIMEOUT #ifdef WOLFMQTT_NONBLOCK @@ -1819,24 +1833,25 @@ static int NetRead_ex(void *context, byte* buf, int buf_len, if (do_read) { /* Try and read number of buf_len provided, * minus what's already been read */ - rc = (int)SOCK_RECV(sock->fd, + recvd = (long)SOCK_RECV(sock->fd, &buf[bytes], buf_len - bytes, flags); #if defined(WOLFMQTT_DEBUG_SOCKET) - PRINTF("info: SOCK_RECV(%d) returned %d, buf_len - bytes is %d", - bytes, rc, buf_len - bytes); + PRINTF("info: SOCK_RECV(%d) returned %ld, buf_len - bytes is %d", + bytes, recvd, buf_len - bytes); #endif - if (rc <= 0) { + if (recvd <= 0) { rc = -1; goto exit; /* Error */ } else { /* Clamp return value: defensive check against * platform API returning more than requested */ - if (rc > buf_len - bytes) { - rc = buf_len - bytes; + if (recvd > (long)(buf_len - bytes)) { + recvd = (long)(buf_len - bytes); } + rc = (int)recvd; bytes += rc; /* Data */ #ifdef ENABLE_MQTT_TLS if (MqttClient_Flags(&mqttCtx->client, 0, 0) diff --git a/src/mqtt_sn_packet.c b/src/mqtt_sn_packet.c index 7c71bb17b..e903ee4cd 100644 --- a/src/mqtt_sn_packet.c +++ b/src/mqtt_sn_packet.c @@ -1740,6 +1740,11 @@ int SN_Packet_Read(MqttClient *client, byte* rx_buf, int rx_buf_len, if (client == NULL || rx_buf == NULL) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG); } + /* Every fixed header this function reads is either 2 bytes (length in the + * first byte) or 4 (SN_PACKET_LEN_IND, length in the next two), so a buffer + * that clears this check holds any header MQTT_PK_BEGIN builds below. A + * resume that enters at a later state carries header_len from an earlier + * call and is bounded where it is used, not here. */ if (rx_buf_len < MQTT_PACKET_HEADER_MIN_SIZE + MQTT_DATA_LEN_SIZE) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); } @@ -1766,9 +1771,6 @@ int SN_Packet_Read(MqttClient *client, byte* rx_buf, int rx_buf_len, if (rx_buf[0] == SN_PACKET_LEN_IND){ /* Read length stored in first three bytes, type in fourth */ - if (len + MQTT_DATA_LEN_SIZE > rx_buf_len) { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); - } if (MqttClient_Flags(client,0,0) & MQTT_CLIENT_FLAG_IS_DTLS) { rc = MqttSocket_Read(client, rx_buf+len, 2, timeout_ms); if (rc < 0) { @@ -1821,7 +1823,10 @@ int SN_Packet_Read(MqttClient *client, byte* rx_buf, int rx_buf_len, client->packet.remain_len = 0; } - /* Make sure it does not overflow rx_buf */ + /* Make sure it does not overflow rx_buf. header_len is + * carried in client state across calls, so a resume that + * enters here can hold a length the caller's buffer cannot + * cover; the entry check above does not bound it. */ if (rx_buf_len < client->packet.header_len) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); } From 7e9787eae492df24e3aec6e39c10196105784315 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 17 Sep 2026 09:40:15 -0500 Subject: [PATCH 3/4] Fixes from review --- .github/workflows/feature-combo-check.yml | 8 ++++++-- ChangeLog.md | 6 +++++- wolfmqtt/mqtt_types.h | 11 ++++++----- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/.github/workflows/feature-combo-check.yml b/.github/workflows/feature-combo-check.yml index 3cd52c51f..e4bbbc5da 100644 --- a/.github/workflows/feature-combo-check.yml +++ b/.github/workflows/feature-combo-check.yml @@ -52,7 +52,7 @@ jobs: - name: v5-sn-multithread-nonblock defs: "-DWOLFMQTT_V5 -DWOLFMQTT_SN -DWOLFMQTT_MULTITHREAD -DWOLFMQTT_NONBLOCK" - name: v5-static-memory - defs: "-DWOLFMQTT_V5 -DWOLFMQTT_USE_STATIC_MEMORY" + defs: "-DWOLFMQTT_V5 -DWOLFMQTT_STATIC_MEMORY" steps: - uses: actions/checkout@v4 @@ -71,6 +71,10 @@ jobs: # DEFS goes through the environment rather than being spliced into the # command, so the parentheses in the function-like macros reach gcc # instead of being parsed by the shell. + # + # Every job here is a non-TLS build because ENABLE_MQTT_TLS is simply + # not defined; there is no opt-out macro to pass. TLS combinations are + # covered by the autotools workflows. - name: Compile sources env: DEFS: ${{ matrix.defs }} @@ -78,6 +82,6 @@ jobs: for src in src/*.c; do echo "=== ${{ matrix.name }}: $src" gcc -c -Werror -Wall -Wextra -I. \ - -DWOLFMQTT_USER_SETTINGS -DWOLFMQTT_NO_TLS $DEFS \ + -DWOLFMQTT_USER_SETTINGS $DEFS \ "$src" -o /dev/null done diff --git a/ChangeLog.md b/ChangeLog.md index ea0570eb9..1d7acaab1 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -214,6 +214,10 @@ - `WOLFMQTT_CUSTOM_MALLOC` without `WOLFMQTT_MALLOC` and `WOLFMQTT_FREE` now fails in the header with a message naming both macros, instead of an implicit declaration reported from inside `mqtt_client.c` (#619) + - `` is no longer pulled in for `WOLFMQTT_CUSTOM_STRING` ports. + It was previously included alongside `` whatever the setting, so + such a port picked up the standard string declarations by accident; it + must now supply its own string macros and any headers those need (#619) - `WOLFMQTT_SESSION_ID_TRACK` is derived from `WOLFMQTT_MAX_QOS` and `WOLFMQTT_NO_SESSION_REPLAY`, so a build that also defined it on the command line hit a macro redefinition, fatal under `-Werror`. Such a @@ -222,7 +226,7 @@ calls drop their arguments once `PRINTF` is a no-op, leaving the log string sanitizer with no callers. `WOLFMQTT_NO_STDIO` now implies `WOLFMQTT_BROKER_NO_LOG`, except where `WOLFMQTT_CUSTOM_PRINTF` supplies - a working sink + a working sink (#619) ### v2.1.0 (07/02/2026) Release 2.1.0 has been developed according to wolfSSL's development and QA diff --git a/wolfmqtt/mqtt_types.h b/wolfmqtt/mqtt_types.h index 651ed4f23..419ebe895 100644 --- a/wolfmqtt/mqtt_types.h +++ b/wolfmqtt/mqtt_types.h @@ -296,11 +296,12 @@ enum MqttPacketResponseCodes { #endif #endif -/* XMEMCHR backstop. Standard builds and existing custom-string ports - * (which already pull in via other paths) keep building - * without changes. Custom-string ports that intentionally avoid - * get an explicit #error directing them to define XMEMCHR - * themselves, instead of a confusing missing-header diagnostic. */ +/* XMEMCHR backstop. Standard builds keep building without changes: the + * above declares memchr alongside the other defaults. A + * custom-string port supplies its own string macros and whatever headers + * those need - no wolfMQTT header includes for it - so leaving + * XMEMCHR undefined gets an explicit #error directing it to define XMEMCHR + * itself, instead of a confusing missing-header diagnostic. */ #ifndef XMEMCHR #ifdef WOLFMQTT_CUSTOM_STRING #error "WOLFMQTT_CUSTOM_STRING set: please define XMEMCHR" From 096ea5cc151b910d79a2db451c60d16649f63a18 Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Thu, 17 Sep 2026 12:10:08 -0500 Subject: [PATCH 4/4] Fix from review --- ChangeLog.md | 4 +++- wolfmqtt/mqtt_types.h | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index 1d7acaab1..c44e860eb 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -210,7 +210,9 @@ `` was only included alongside ``. The header is now included with the allocator defaults that need it, which also fixes `WOLFMQTT_CUSTOM_PRINTF` ports such as MPLAB Harmony, and a port that - supplies both macros still pulls in no libc header (#619) + supplies both macros no longer pulls it in for the allocator. The + default `XATOI` needs `atoi` from the same header, so it is included + with that macro too (#619) - `WOLFMQTT_CUSTOM_MALLOC` without `WOLFMQTT_MALLOC` and `WOLFMQTT_FREE` now fails in the header with a message naming both macros, instead of an implicit declaration reported from inside `mqtt_client.c` (#619) diff --git a/wolfmqtt/mqtt_types.h b/wolfmqtt/mqtt_types.h index 419ebe895..f46eb78b5 100644 --- a/wolfmqtt/mqtt_types.h +++ b/wolfmqtt/mqtt_types.h @@ -279,6 +279,11 @@ enum MqttPacketResponseCodes { #define XMEMCMP(s1,s2,n) memcmp((s1),(s2),(n)) #endif #ifndef XATOI + /* atoi is declared in , not the above. The + * allocator backstop further down includes that header only when its + * own defaults are needed, so a port supplying WOLFMQTT_MALLOC and + * WOLFMQTT_FREE would otherwise leave the default XATOI undeclared. */ + #include #define XATOI(s) atoi((s)) #endif #ifndef XISALNUM