Skip to content

Commit 9f61578

Browse files
dgarskedgarske
committed
More peer review fixes
1 parent 75cf523 commit 9f61578

4 files changed

Lines changed: 99 additions & 70 deletions

File tree

‎examples/mqttnet.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1204,11 +1204,17 @@ static int NetConnect(void *context, const char* host, word16 port,
12041204

12051205
case SOCK_CONN:
12061206
{
1207-
/* Set up address and initiate connect */
1207+
/* Set up address and initiate connect.
1208+
* Note: atoip4() only supports dotted-quad IPv4 strings
1209+
* (e.g., "192.168.1.1") and does not resolve DNS hostnames. */
12081210
XMEMSET(&addr, 0, sizeof(addr));
12091211
addr.sin_family = AF_INET;
12101212
addr.sin_port = ee16(port);
12111213
addr.sin_addr.s_addr = atoip4(host);
1214+
if (addr.sin_addr.s_addr == 0) {
1215+
NetDisconnect(context);
1216+
return MQTT_CODE_ERROR_BAD_ARG;
1217+
}
12121218

12131219
rc = wolfIP_sock_connect(sock->stack, sock->fd,
12141220
(struct wolfIP_sockaddr *)&addr, sizeof(addr));

‎examples/mqttport.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@ extern "C" {
8080
#define SOCKET_T int
8181
#define SOCKET_INVALID (-1)
8282
#define SOCK_ADDR_IN struct wolfIP_sockaddr_in
83-
#define NO_FILESYSTEM
83+
/* For wolfIP targets without filesystem support, define NO_FILESYSTEM
84+
* via build configuration (e.g., compiler flags or user_settings.h). */
85+
#ifndef NO_FILESYSTEM
86+
#define NO_FILESYSTEM
87+
#endif
8488

8589
/* User defined IO */
8690
#elif defined(WOLFMQTT_USER_IO)

‎src/mqtt_broker.c‎

Lines changed: 86 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,10 @@
5656
/* -------------------------------------------------------------------------- */
5757
#ifndef WOLFMQTT_BROKER_GET_TIME_S
5858
#if defined(WOLFMQTT_WOLFIP)
59-
/* wolfIP: override WOLFMQTT_BROKER_GET_TIME_S in user_settings.h */
60-
#define WOLFMQTT_BROKER_GET_TIME_S() ((WOLFMQTT_BROKER_TIME_T)0)
59+
/* wolfIP has no default time source. Define
60+
* WOLFMQTT_BROKER_GET_TIME_S in user_settings.h to provide one.
61+
* Example: #define WOLFMQTT_BROKER_GET_TIME_S() myGetTimeSec() */
62+
#error "WOLFMQTT_WOLFIP requires WOLFMQTT_BROKER_GET_TIME_S to be defined"
6163
#else
6264
#define WOLFMQTT_BROKER_GET_TIME_S() \
6365
((WOLFMQTT_BROKER_TIME_T)time(NULL))
@@ -69,7 +71,11 @@
6971
/* -------------------------------------------------------------------------- */
7072
#ifndef BROKER_SLEEP_MS
7173
#if defined(WOLFMQTT_WOLFIP)
72-
#define BROKER_SLEEP_MS(ms) /* no-op: cooperative scheduling - Step() returns to caller for task switching */
74+
/* No-op: wolfIP uses cooperative scheduling via MqttBroker_Step().
75+
* Do not use MqttBroker_Run() on wolfIP - it will busy-spin.
76+
* Override BROKER_SLEEP_MS in user_settings.h if a yield/delay
77+
* primitive is available on your platform. */
78+
#define BROKER_SLEEP_MS(ms) do {} while(0)
7379
#elif defined(USE_WINDOWS_API)
7480
#define BROKER_SLEEP_MS(ms) Sleep(ms)
7581
#else
@@ -183,108 +189,112 @@ static void BrokerStore_String(char** dst_ptr,
183189
static int BrokerTls_Init(MqttBroker* broker)
184190
{
185191
WOLFSSL_CTX* ctx = NULL;
186-
int rc;
192+
int wolf_rc; /* wolfSSL return codes (compared against WOLFSSL_SUCCESS) */
193+
int mqtt_rc = MQTT_CODE_SUCCESS; /* normalized MQTT return code */
187194

188-
rc = wolfSSL_Init();
189-
if (rc != WOLFSSL_SUCCESS) {
190-
WBLOG_ERR(broker, "broker: wolfSSL_Init failed %d", rc);
191-
rc = MQTT_CODE_ERROR_BAD_ARG;
195+
wolf_rc = wolfSSL_Init();
196+
if (wolf_rc != WOLFSSL_SUCCESS) {
197+
WBLOG_ERR(broker, "broker: wolfSSL_Init failed %d", wolf_rc);
198+
return MQTT_CODE_ERROR_BAD_ARG;
192199
}
193200

194201
/* Select TLS method based on version preference */
195-
if (rc == WOLFSSL_SUCCESS) {
196-
if (broker->tls_version == 12) {
197-
ctx = wolfSSL_CTX_new(wolfTLSv1_2_server_method());
198-
}
199-
else if (broker->tls_version == 13) {
200-
ctx = wolfSSL_CTX_new(wolfTLSv1_3_server_method());
201-
}
202-
else {
203-
ctx = wolfSSL_CTX_new(wolfSSLv23_server_method());
204-
}
205-
if (ctx == NULL) {
206-
WBLOG_ERR(broker, "broker: wolfSSL_CTX_new failed");
207-
rc = MQTT_CODE_ERROR_MEMORY;
208-
}
202+
if (broker->tls_version == 12) {
203+
ctx = wolfSSL_CTX_new(wolfTLSv1_2_server_method());
204+
}
205+
else if (broker->tls_version == 13) {
206+
ctx = wolfSSL_CTX_new(wolfTLSv1_3_server_method());
207+
}
208+
else {
209+
ctx = wolfSSL_CTX_new(wolfSSLv23_server_method());
210+
}
211+
if (ctx == NULL) {
212+
WBLOG_ERR(broker, "broker: wolfSSL_CTX_new failed");
213+
mqtt_rc = MQTT_CODE_ERROR_MEMORY;
209214
}
210215

211216
/* Load server certificate */
212-
if (rc == WOLFSSL_SUCCESS) {
217+
if (mqtt_rc == MQTT_CODE_SUCCESS) {
213218
if (broker->tls_cert == NULL) {
214219
WBLOG_ERR(broker, "broker: TLS cert not set (-c)");
215-
rc = MQTT_CODE_ERROR_BAD_ARG;
220+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
216221
}
217222
}
218-
if (rc == WOLFSSL_SUCCESS) {
223+
if (mqtt_rc == MQTT_CODE_SUCCESS) {
219224
#ifndef NO_FILESYSTEM
220-
rc = wolfSSL_CTX_use_certificate_file(ctx, broker->tls_cert,
225+
wolf_rc = wolfSSL_CTX_use_certificate_file(ctx, broker->tls_cert,
221226
WOLFSSL_FILETYPE_PEM);
222-
if (rc != WOLFSSL_SUCCESS) {
223-
WBLOG_ERR(broker, "broker: load cert failed %d (%s)", rc, broker->tls_cert);
224-
rc = MQTT_CODE_ERROR_BAD_ARG;
227+
if (wolf_rc != WOLFSSL_SUCCESS) {
228+
WBLOG_ERR(broker, "broker: load cert failed %d (%s)",
229+
wolf_rc, broker->tls_cert);
230+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
225231
}
226232
#else
227233
/* File operations not available in NO_FILESYSTEM builds */
228-
rc = MQTT_CODE_ERROR_BAD_ARG;
234+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
229235
#endif
230236
}
231237

232238
/* Load server private key */
233-
if (rc == WOLFSSL_SUCCESS) {
239+
if (mqtt_rc == MQTT_CODE_SUCCESS) {
234240
if (broker->tls_key == NULL) {
235241
WBLOG_ERR(broker, "broker: TLS key not set (-K)");
236-
rc = MQTT_CODE_ERROR_BAD_ARG;
242+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
237243
}
238244
}
239-
if (rc == WOLFSSL_SUCCESS) {
245+
if (mqtt_rc == MQTT_CODE_SUCCESS) {
240246
#ifndef NO_FILESYSTEM
241-
rc = wolfSSL_CTX_use_PrivateKey_file(ctx, broker->tls_key,
247+
wolf_rc = wolfSSL_CTX_use_PrivateKey_file(ctx, broker->tls_key,
242248
WOLFSSL_FILETYPE_PEM);
243-
if (rc != WOLFSSL_SUCCESS) {
244-
WBLOG_ERR(broker, "broker: load key failed %d (%s)", rc, broker->tls_key);
245-
rc = MQTT_CODE_ERROR_BAD_ARG;
249+
if (wolf_rc != WOLFSSL_SUCCESS) {
250+
WBLOG_ERR(broker, "broker: load key failed %d (%s)",
251+
wolf_rc, broker->tls_key);
252+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
246253
}
247254
#else
248-
rc = MQTT_CODE_ERROR_BAD_ARG;
255+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
249256
#endif
250257
}
251258

252259
/* Set wolfSSL IO callbacks */
253-
if (rc == WOLFSSL_SUCCESS) {
260+
if (mqtt_rc == MQTT_CODE_SUCCESS) {
254261
wolfSSL_CTX_SetIORecv(ctx, MqttSocket_TlsSocketReceive);
255262
wolfSSL_CTX_SetIOSend(ctx, MqttSocket_TlsSocketSend);
256263
}
257264

258265
/* Mutual TLS: load CA and require client certificate */
259-
if (rc == WOLFSSL_SUCCESS && broker->tls_ca != NULL) {
266+
if (mqtt_rc == MQTT_CODE_SUCCESS && broker->tls_ca != NULL) {
260267
#ifndef NO_FILESYSTEM
261-
rc = wolfSSL_CTX_load_verify_locations(ctx, broker->tls_ca, NULL);
268+
wolf_rc = wolfSSL_CTX_load_verify_locations(ctx, broker->tls_ca,
269+
NULL);
270+
if (wolf_rc != WOLFSSL_SUCCESS) {
271+
WBLOG_ERR(broker, "broker: load CA failed %d (%s)",
272+
wolf_rc, broker->tls_ca);
273+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
274+
}
262275
#else
263-
rc = MQTT_CODE_ERROR_BAD_ARG;
276+
mqtt_rc = MQTT_CODE_ERROR_BAD_ARG;
264277
#endif
265-
if (rc != WOLFSSL_SUCCESS) {
266-
WBLOG_ERR(broker, "broker: load CA failed %d (%s)", rc, broker->tls_ca);
267-
rc = MQTT_CODE_ERROR_BAD_ARG;
268-
}
269-
else {
278+
if (mqtt_rc == MQTT_CODE_SUCCESS) {
270279
wolfSSL_CTX_set_verify(ctx,
271280
WOLFSSL_VERIFY_PEER | WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT,
272281
NULL);
273-
WBLOG_INFO(broker, "broker: mutual TLS enabled (CA=%s)", broker->tls_ca);
282+
WBLOG_INFO(broker, "broker: mutual TLS enabled (CA=%s)",
283+
broker->tls_ca);
274284
}
275285
}
276286

277-
if (rc == WOLFSSL_SUCCESS) {
287+
if (mqtt_rc == MQTT_CODE_SUCCESS) {
278288
broker->tls_ctx = ctx;
279-
rc = MQTT_CODE_SUCCESS;
289+
broker->tls_ctx_owned = 1;
280290
}
281291
else {
282292
if (ctx != NULL) {
283293
wolfSSL_CTX_free(ctx);
284294
}
285295
wolfSSL_Cleanup();
286296
}
287-
return rc;
297+
return mqtt_rc;
288298
}
289299

290300
static void BrokerTls_Free(MqttBroker* broker)
@@ -310,6 +320,9 @@ typedef struct BrokerWolfIP_Ctx {
310320
} BrokerWolfIP_Ctx;
311321
#endif
312322

323+
/* Single-instance context: wolfIP targets are typically embedded systems
324+
* with one broker instance. For multiple instances, use
325+
* WOLFMQTT_BROKER_CUSTOM_NET and provide per-instance context. */
313326
static BrokerWolfIP_Ctx broker_wolfip_ctx;
314327

315328
static int BrokerWolfIP_Listen(void* ctx, BROKER_SOCKET_T* sock,
@@ -358,10 +371,13 @@ static int BrokerWolfIP_Accept(void* ctx, BROKER_SOCKET_T listen_sock,
358371
}
359372

360373
fd = wolfIP_sock_accept(wctx->stack, listen_sock, NULL, NULL);
361-
if (fd < 0) {
362-
/* EAGAIN / no pending connection */
374+
if (fd == -WOLFIP_EAGAIN) {
375+
/* No pending connection */
363376
return MQTT_CODE_CONTINUE;
364377
}
378+
if (fd < 0) {
379+
return MQTT_CODE_ERROR_NETWORK;
380+
}
365381

366382
*client_sock = fd;
367383
return MQTT_CODE_SUCCESS;
@@ -379,12 +395,12 @@ static int BrokerWolfIP_Read(void* ctx, BROKER_SOCKET_T sock,
379395
}
380396

381397
rc = wolfIP_sock_recv(wctx->stack, sock, buf, (size_t)buf_len, 0);
382-
if (rc < 0) {
383-
/* EAGAIN / would block */
398+
/* -WOLFIP_EAGAIN: no data yet; -1: socket not yet in ESTABLISHED state */
399+
if (rc == -WOLFIP_EAGAIN || rc == -1) {
384400
return MQTT_CODE_CONTINUE;
385401
}
386-
if (rc == 0) {
387-
return MQTT_CODE_ERROR_NETWORK; /* Connection closed */
402+
if (rc <= 0) {
403+
return MQTT_CODE_ERROR_NETWORK;
388404
}
389405
return rc;
390406
}
@@ -401,11 +417,11 @@ static int BrokerWolfIP_Write(void* ctx, BROKER_SOCKET_T sock,
401417
}
402418

403419
rc = wolfIP_sock_send(wctx->stack, sock, buf, (size_t)buf_len, 0);
404-
if (rc < 0) {
405-
/* EAGAIN / would block */
420+
/* -WOLFIP_EAGAIN: send buffer full; -1: socket not yet in ESTABLISHED state */
421+
if (rc == -WOLFIP_EAGAIN || rc == -1) {
406422
return MQTT_CODE_CONTINUE;
407423
}
408-
if (rc == 0) {
424+
if (rc <= 0) {
409425
return MQTT_CODE_ERROR_NETWORK;
410426
}
411427
return rc;
@@ -3707,14 +3723,16 @@ int MqttBroker_Free(MqttBroker* broker)
37073723

37083724
#ifdef ENABLE_MQTT_TLS
37093725
if (broker->tls_ctx != NULL) {
3710-
#if !defined(WOLFMQTT_WOLFIP) && !defined(WOLFMQTT_BROKER_CUSTOM_NET)
3711-
BrokerTls_Free(broker);
3712-
#else
3713-
/* Application-provided TLS context: free ctx but skip
3714-
* wolfSSL_Cleanup() since wolfSSL may be shared */
3715-
wolfSSL_CTX_free(broker->tls_ctx);
3716-
broker->tls_ctx = NULL;
3717-
#endif
3726+
if (broker->tls_ctx_owned) {
3727+
/* Context was created by BrokerTls_Init: full cleanup */
3728+
BrokerTls_Free(broker);
3729+
}
3730+
else {
3731+
/* Application-provided TLS context: free ctx but skip
3732+
* wolfSSL_Cleanup() since wolfSSL may be shared */
3733+
wolfSSL_CTX_free(broker->tls_ctx);
3734+
broker->tls_ctx = NULL;
3735+
}
37183736
}
37193737
#endif
37203738

‎wolfmqtt/mqtt_broker.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ typedef struct MqttBroker {
327327
const char* tls_ca; /* CA cert for mutual auth (optional) */
328328
byte use_tls;
329329
byte tls_version; /* 0=auto (v23), 12=TLS 1.2, 13=TLS 1.3 */
330+
byte tls_ctx_owned; /* 1 if BrokerTls_Init created tls_ctx */
330331
#endif
331332
#ifdef WOLFMQTT_STATIC_MEMORY
332333
BrokerClient clients[BROKER_MAX_CLIENTS];

0 commit comments

Comments
 (0)