fix(mqtt): render the mbedtls diagnostic magnitude correctly

ESP-IDF stores the mbedTLS stack error as a positive magnitude (it captures
-ret), so negating it before printing produced "mbedtls:-0xFFFF8100" instead
of "mbedtls:-0x7F00" for the record-buffer allocation failure. %04X is a
minimum width, so nothing masked it.

Normalisation moves into MQTTReplyFormat.h as mbedtlsErrorMagnitude() rather
than staying inline in the bridge: inline is why this survived, since the
existing test passes the already-correct magnitude straight into replyAppendf
and never exercised the caller. It accepts either sign so a later SDK storing
the real negative code still renders, and widens to int64_t before negating
because negating INT32_MIN is undefined behaviour.

MQTTReplyFormat.h also gains the stdint.h it was always missing: it compiled
only because MQTTBridge.cpp pulls stdint in via other headers, and the host
test includes the header standalone.
This commit is contained in:
agessaman committed 2026-08-18 13:54:55 -07:00
1 parent 73480d50a3
commit a0aec91c17
3 files changed
+34 -2

No files matched your search

+9
View File
@@ -2,6 +2,7 @@
#include <stddef.h>
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>
// Bounded, clamping printf-append for the fixed-size CLI reply buffers used by
@@ -41,3 +42,11 @@ static inline void replyAppendf(char* buf, size_t bufsize, int* pos, const char*
*pos += n;
if ((size_t)*pos >= bufsize) *pos = (int)bufsize - 1; // clamp truncated append
}
// Magnitude of an mbedTLS stack error, for printing as "-0x%04X". ESP-IDF 4.4 stores
// the positive magnitude (it captures -ret), so negating it produces a garbage unsigned;
// accepting either sign keeps this correct if a later SDK stores the negative code.
// The int64_t cast matters: negating INT32_MIN directly is undefined behaviour.
static inline uint32_t mbedtlsErrorMagnitude(int32_t stack_err) {
return (stack_err < 0) ? (uint32_t)(-(int64_t)stack_err) : (uint32_t)stack_err;
}
+4 -2
View File
@@ -559,9 +559,11 @@ void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index)
replyAppendf(buf, bufsize, &pos, ", tls:0x%04X", (unsigned)slot.last_tls_err);
}
}
// mbedTLS stack error (shown as negative hex per convention)
// mbedTLS stack error (shown as negative hex per convention). ESP-IDF stores the
// magnitude, not the negative mbedTLS code, so normalise instead of negating.
if (slot.last_tls_stack_err != 0) {
replyAppendf(buf, bufsize, &pos, ", mbedtls:-0x%04X", (unsigned)(-slot.last_tls_stack_err));
replyAppendf(buf, bufsize, &pos, ", mbedtls:-0x%04X",
(unsigned)mbedtlsErrorMagnitude(slot.last_tls_stack_err));
}
// Socket errno
if (slot.last_sock_errno != 0) {
@@ -88,6 +88,27 @@ TEST(ReplyAppendf, NoWritePastBufferAcrossOverflowingChain) {
EXPECT_EQ(c.buf()[c.logical - 1], '\0'); // still NUL-terminated
}
// The caller, not the helper, was where the mbedtls: line went wrong: production
// negated a value ESP-IDF already stores as a magnitude, so it rendered
// "mbedtls:-0xFFFF8100" instead of "-0x7F00". The chain test above never caught it
// because it passes the correct magnitude straight in.
TEST(MbedtlsErrorMagnitude, NormalisesEitherSignForTheNegativeHexConvention) {
EXPECT_EQ(mbedtlsErrorMagnitude(0x7F00), 0x7F00u); // ESP-IDF 4.4 stores +magnitude
EXPECT_EQ(mbedtlsErrorMagnitude(-0x7F00), 0x7F00u); // a later SDK may store the real code
EXPECT_EQ(mbedtlsErrorMagnitude(0), 0u);
char buf[32];
int pos = 0;
replyAppendf(buf, sizeof(buf), &pos, ", mbedtls:-0x%04X",
(unsigned)mbedtlsErrorMagnitude(0x7F00));
EXPECT_STREQ(buf, ", mbedtls:-0x7F00");
}
// Negating INT32_MIN is undefined behaviour, which is why the helper widens first.
TEST(MbedtlsErrorMagnitude, HandlesInt32MinWithoutOverflow) {
EXPECT_EQ(mbedtlsErrorMagnitude(INT32_MIN), 2147483648u);
}
TEST(ReplyAppendf, ExactFitBoundary) {
char buf[11]; // room for exactly "0123456789" + NUL
int pos = 0;