Skip to content

Commit a3c2cb5

Browse files
committed
Safer creation of atoms in ESP32 network_driver.c
Removes the up-fron declaration of `static const char *const` atom strings in favor of inline ATOM_STR() for better redability. Changes to use of `globalcontext_existing_term_from_atom_string` where possible. AP and STA mode specific atoms are only created for the interface(s) that are used. All atoms that are created by the driver are now checked to be certain the creation was sucessful. Addresses concerns raised in issue #1442 for the ESP32 network driver. Signed-off-by: Winford <[email protected]>
1 parent 4f563ac commit a3c2cb5

File tree

2 files changed

+101
-55
lines changed

2 files changed

+101
-55
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ network. (See issue #1094)
8080
- ESP32 UART driver no longer aborts because of badargs in configuration, instead raising an error
8181
- ESP32: `v0.6.6` uses esp-idf v5.4.1 for pre-built images and `v5.4.x` is the suggested release
8282
also for custom builds
83+
- Updated atom initalization in esp32 network_driver.c for issue #1442
8384

8485
## [0.6.5] - 2024-10-15
8586

src/platforms/esp32/components/avm_builtins/network_driver.c

Lines changed: 100 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -60,27 +60,6 @@
6060
#define TAG "network_driver"
6161
#define PORT_REPLY_SIZE (TUPLE_SIZE(2) + REF_SIZE)
6262

63-
static const char *const ap_atom = ATOM_STR("\x2", "ap");
64-
static const char *const ap_channel_atom = ATOM_STR("\xA", "ap_channel");
65-
static const char *const ap_sta_connected_atom = ATOM_STR("\x10", "ap_sta_connected");
66-
static const char *const ap_sta_disconnected_atom = ATOM_STR("\x13", "ap_sta_disconnected");
67-
static const char *const ap_sta_ip_assigned_atom = ATOM_STR("\x12", "ap_sta_ip_assigned");
68-
static const char *const ap_started_atom = ATOM_STR("\xA", "ap_started");
69-
static const char *const dhcp_hostname_atom = ATOM_STR("\xD", "dhcp_hostname");
70-
static const char *const host_atom = ATOM_STR("\x4", "host");
71-
static const char *const max_connections_atom = ATOM_STR("\xF", "max_connections");
72-
static const char *const psk_atom = ATOM_STR("\x3", "psk");
73-
static const char *const sntp_atom = ATOM_STR("\x4", "sntp");
74-
static const char *const sntp_sync_atom = ATOM_STR("\x9", "sntp_sync");
75-
static const char *const ssid_atom = ATOM_STR("\x4", "ssid");
76-
static const char *const ssid_hidden_atom = ATOM_STR("\xB", "ssid_hidden");
77-
static const char *const sta_atom = ATOM_STR("\x3", "sta");
78-
static const char *const sta_connected_atom = ATOM_STR("\xD", "sta_connected");
79-
static const char *const sta_beacon_timeout_atom = ATOM_STR("\x12", "sta_beacon_timeout");
80-
static const char *const sta_disconnected_atom = ATOM_STR("\x10", "sta_disconnected");
81-
static const char *const sta_got_ip_atom = ATOM_STR("\xA", "sta_got_ip");
82-
static const char *const network_down_atom = ATOM_STR("\x0C", "network_down");
83-
8463
ESP_EVENT_DECLARE_BASE(sntp_event_base);
8564
ESP_EVENT_DEFINE_BASE(sntp_event_base);
8665

@@ -113,11 +92,6 @@ struct ClientData
11392
uint64_t ref_ticks;
11493
};
11594

116-
static inline term make_atom(GlobalContext *global, AtomString atom_str)
117-
{
118-
return globalcontext_make_atom(global, atom_str);
119-
}
120-
12195
static term tuple_from_addr(Heap *heap, uint32_t addr)
12296
{
12397
term terms[4];
@@ -152,20 +126,56 @@ static void send_got_ip(struct ClientData *data, esp_netif_ip_info_t *info)
152126
term gw = tuple_from_addr(&heap, ntohl(info->gw.addr));
153127

154128
term ip_info = port_heap_create_tuple3(&heap, ip, netmask, gw);
155-
term reply = port_heap_create_tuple2(&heap, make_atom(data->global, sta_got_ip_atom), ip_info);
129+
term reply = port_heap_create_tuple2(&heap, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\xA", "sta_got_ip")), ip_info);
156130
send_term(&heap, data, reply);
157131
}
158132
END_WITH_STACK_HEAP(heap, data->global);
159133
}
160134

135+
// Used to initialize atoms use in STA mode callbacks when sta mode is configured
136+
// callbacks should get the existing atom term from from the atom string.
137+
#define NUM_STA_ATOMS 5
138+
bool init_sta_atoms(GlobalContext *glb)
139+
{
140+
AtomString sta_cb_atom[NUM_STA_ATOMS] = { ATOM_STR("\xD", "sta_connected"), ATOM_STR("\x12", "sta_beacon_timeout"),
141+
ATOM_STR("\x10", "sta_disconnected"), ATOM_STR("\xA", "sta_got_ip"), ATOM_STR("\x0C", "network_down")};
142+
143+
for (int atom = 0; atom < NUM_STA_ATOMS; atom++) {
144+
int index = globalcontext_insert_atom(glb, sta_cb_atom[atom]);
145+
if (UNLIKELY(index < 0)) {
146+
return false;
147+
}
148+
}
149+
150+
return true;
151+
}
152+
153+
// Used to initialize atoms use in AP mode callbacks when ap mode is configured
154+
// callbacks should get the existing atom term from from the atom string.
155+
#define NUM_AP_ATOMS 4
156+
bool init_ap_cb_atoms(GlobalContext *glb)
157+
{
158+
AtomString ap_cb_atoms[NUM_AP_ATOMS] = { ATOM_STR("\x10", "ap_sta_connected"), ATOM_STR("\x13", "ap_sta_disconnected"),
159+
ATOM_STR("\x12", "ap_sta_ip_assigned"), ATOM_STR("\xA", "ap_started") };
160+
161+
for (int atom = 0; atom < NUM_AP_ATOMS; atom++) {
162+
int index = globalcontext_insert_atom(glb, ap_cb_atoms[atom]);
163+
if (UNLIKELY(index < 0)) {
164+
return false;
165+
}
166+
}
167+
168+
return true;
169+
}
170+
161171
static void send_sta_connected(struct ClientData *data)
162172
{
163173
TRACE("Sending sta_connected back to AtomVM\n");
164174

165175
// {Ref, sta_connected}
166176
BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE, heap);
167177
{
168-
send_term(&heap, data, make_atom(data->global, sta_connected_atom));
178+
send_term(&heap, data, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\xD", "sta_connected")));
169179
}
170180
END_WITH_STACK_HEAP(heap, data->global);
171181
}
@@ -177,7 +187,7 @@ static void send_sta_beacon_timeout(struct ClientData *data)
177187
// {Ref, sta_beacon_timeout}
178188
BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE, heap);
179189
{
180-
send_term(&heap, data, make_atom(data->global, sta_beacon_timeout_atom));
190+
send_term(&heap, data, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\x12", "sta_beacon_timeout")));
181191
}
182192
END_WITH_STACK_HEAP(heap, data->global);
183193
}
@@ -189,7 +199,7 @@ static void send_sta_disconnected(struct ClientData *data)
189199
// {Ref, sta_disconnected}
190200
BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE, heap);
191201
{
192-
send_term(&heap, data, make_atom(data->global, sta_disconnected_atom));
202+
send_term(&heap, data, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\x10", "sta_disconnected")));
193203
}
194204
END_WITH_STACK_HEAP(heap, data->global);
195205
}
@@ -201,7 +211,7 @@ static void send_ap_started(struct ClientData *data)
201211
// {Ref, ap_started}
202212
BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE, heap);
203213
{
204-
send_term(&heap, data, make_atom(data->global, ap_started_atom));
214+
send_term(&heap, data, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\xA", "ap_started")));
205215
}
206216
END_WITH_STACK_HEAP(heap, data->global);
207217
}
@@ -221,13 +231,13 @@ static void send_atom_mac(struct ClientData *data, term atom, uint8_t *mac)
221231
static void send_ap_sta_connected(struct ClientData *data, uint8_t *mac)
222232
{
223233
TRACE("Sending ap_sta_connected back to AtomVM\n");
224-
send_atom_mac(data, make_atom(data->global, ap_sta_connected_atom), mac);
234+
send_atom_mac(data, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\x10", "ap_sta_connected")), mac);
225235
}
226236

227237
static void send_ap_sta_disconnected(struct ClientData *data, uint8_t *mac)
228238
{
229239
TRACE("Sending ap_sta_disconnected back to AtomVM\n");
230-
send_atom_mac(data, make_atom(data->global, ap_sta_disconnected_atom), mac);
240+
send_atom_mac(data, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\x13", "ap_sta_disconnected")), mac);
231241
}
232242

233243
static void send_ap_sta_ip_assigned(struct ClientData *data, esp_ip4_addr_t *ip)
@@ -237,7 +247,7 @@ static void send_ap_sta_ip_assigned(struct ClientData *data, esp_ip4_addr_t *ip)
237247
BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE + TUPLE_SIZE(2) + TUPLE_SIZE(4), heap);
238248
{
239249
term ip_term = tuple_from_addr(&heap, ntohl(ip->addr));
240-
term reply = port_heap_create_tuple2(&heap, make_atom(data->global, ap_sta_ip_assigned_atom), ip_term);
250+
term reply = port_heap_create_tuple2(&heap, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\x12", "ap_sta_ip_assigned")), ip_term);
241251
send_term(&heap, data, reply);
242252
}
243253
END_WITH_STACK_HEAP(heap, data->global);
@@ -251,7 +261,7 @@ static void send_sntp_sync(struct ClientData *data, struct timeval *tv)
251261
BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE + TUPLE_SIZE(2) * 2 + BOXED_INT64_SIZE * 2, heap);
252262
{
253263
term tv_tuple = port_heap_create_tuple2(&heap, term_make_maybe_boxed_int64(tv->tv_sec, &heap), term_make_maybe_boxed_int64(tv->tv_usec, &heap));
254-
term reply = port_heap_create_tuple2(&heap, make_atom(data->global, sntp_sync_atom), tv_tuple);
264+
term reply = port_heap_create_tuple2(&heap, globalcontext_existing_term_from_atom_string(data->global, ATOM_STR("\x9", "sntp_sync")), tv_tuple);
255265
send_term(&heap, data, reply);
256266
}
257267
END_WITH_STACK_HEAP(heap, data->global);
@@ -381,8 +391,8 @@ static wifi_config_t *get_sta_wifi_config(term sta_config, GlobalContext *global
381391
TRACE("No STA config\n");
382392
return NULL;
383393
}
384-
term ssid_term = interop_kv_get_value(sta_config, ssid_atom, global);
385-
term pass_term = interop_kv_get_value(sta_config, psk_atom, global);
394+
term ssid_term = interop_kv_get_value(sta_config, ATOM_STR("\x4", "ssid"), global);
395+
term pass_term = interop_kv_get_value(sta_config, ATOM_STR("\x3", "psk"), global);
386396

387397
//
388398
// Check parameters
@@ -470,9 +480,9 @@ static wifi_config_t *get_ap_wifi_config(term ap_config, GlobalContext *global)
470480
TRACE("No AP config\n");
471481
return NULL;
472482
}
473-
term ssid_term = interop_kv_get_value(ap_config, ssid_atom, global);
474-
term pass_term = interop_kv_get_value(ap_config, psk_atom, global);
475-
term channel_term = interop_kv_get_value(ap_config, ap_channel_atom, global);
483+
term ssid_term = interop_kv_get_value(ap_config, ATOM_STR("\x4", "ssid"), global);
484+
term pass_term = interop_kv_get_value(ap_config, ATOM_STR("\x3", "psk"), global);
485+
term channel_term = interop_kv_get_value(ap_config, ATOM_STR("\xA", "ap_channel"), global);
476486

477487
//
478488
// Check parameters
@@ -546,9 +556,9 @@ static wifi_config_t *get_ap_wifi_config(term ap_config, GlobalContext *global)
546556
}
547557

548558
wifi_config->ap.authmode = IS_NULL_PTR(psk) ? WIFI_AUTH_OPEN : WIFI_AUTH_WPA_WPA2_PSK;
549-
term ssid_hidden_term = interop_kv_get_value(ap_config, ssid_hidden_atom, global);
559+
term ssid_hidden_term = interop_kv_get_value(ap_config, ATOM_STR("\xB", "ssid_hidden"), global);
550560
wifi_config->ap.ssid_hidden = term_is_invalid_term(ssid_hidden_term) ? 0 : ssid_hidden_term == TRUE_ATOM;
551-
term max_connections_term = interop_kv_get_value(ap_config, max_connections_atom, global);
561+
term max_connections_term = interop_kv_get_value(ap_config, ATOM_STR("\xF", "max_connections"), global);
552562
wifi_config->ap.max_connection = term_is_invalid_term(max_connections_term) ? 4 : term_to_int(max_connections_term);
553563

554564
ESP_LOGI(TAG, "AP ssid: %s", wifi_config->ap.ssid);
@@ -571,16 +581,21 @@ static void time_sync_notification_cb(struct timeval *tv)
571581

572582
static void maybe_set_sntp(term sntp_config, GlobalContext *global)
573583
{
574-
if (!term_is_invalid_term(sntp_config) && !term_is_invalid_term(interop_kv_get_value(sntp_config, host_atom, global))) {
584+
if (!term_is_invalid_term(sntp_config) && !term_is_invalid_term(interop_kv_get_value(sntp_config, ATOM_STR("\x4", "host"), global))) {
575585
int ok;
576-
char *host = interop_term_to_string(interop_kv_get_value(sntp_config, host_atom, global), &ok);
586+
char *host = interop_term_to_string(interop_kv_get_value(sntp_config, ATOM_STR("\x4", "host"), global), &ok);
577587
if (LIKELY(ok)) {
578588
// do not free(sntp)
579589
esp_sntp_setoperatingmode(SNTP_OPMODE_POLL);
580590
esp_sntp_setservername(0, host);
581591
sntp_set_time_sync_notification_cb(time_sync_notification_cb);
582592
esp_sntp_init();
583-
ESP_LOGI(TAG, "SNTP initialized with host set to %s", host);
593+
int sntp_sync_index = globalcontext_insert_atom(global, ATOM_STR("\x9", "sntp_sync"));
594+
if (UNLIKELY(sntp_sync_index < 0)) {
595+
ESP_LOGE(TAG, "Failed to create 'sntp_sync' atom! sntp_sync callbacks will fail.");
596+
} else {
597+
ESP_LOGI(TAG, "SNTP initialized with host set to %s", host);
598+
}
584599
} else {
585600
ESP_LOGE(TAG, "Unable to locate sntp host in configuration");
586601
}
@@ -628,8 +643,8 @@ static void start_network(Context *ctx, term pid, term ref, term config)
628643
//
629644
// Get the STA and AP config, if set
630645
//
631-
term sta_config = interop_kv_get_value_default(config, sta_atom, term_invalid_term(), ctx->global);
632-
term ap_config = interop_kv_get_value_default(config, ap_atom, term_invalid_term(), ctx->global);
646+
term sta_config = interop_kv_get_value_default(config, ATOM_STR("\x3", "sta"), term_invalid_term(), ctx->global);
647+
term ap_config = interop_kv_get_value_default(config, ATOM_STR("\x2", "ap"), term_invalid_term(), ctx->global);
633648
if (UNLIKELY(term_is_invalid_term(sta_config) && term_is_invalid_term(ap_config))) {
634649
ESP_LOGE(TAG, "Expected STA or AP configuration but got neither");
635650
term error = port_create_error_tuple(ctx, BADARG_ATOM);
@@ -726,10 +741,39 @@ static void start_network(Context *ctx, term pid, term ref, term config)
726741
wifi_mode_t wifi_mode = WIFI_MODE_NULL;
727742
if (!IS_NULL_PTR(sta_wifi_config) && !IS_NULL_PTR(ap_wifi_config)) {
728743
wifi_mode = WIFI_MODE_APSTA;
729-
} else if (!IS_NULL_PTR(sta_wifi_config)) {
730-
wifi_mode = WIFI_MODE_STA;
731-
} else {
744+
bool sta_atoms_ok = init_sta_atoms(ctx->global);
745+
bool ap_atoms_ok = init_ap_cb_atoms(ctx->global);
746+
if (UNLIKELY(!sta_atoms_ok || !ap_atoms_ok)) {
747+
ESP_LOGE(TAG, "Unable to insert callback atoms");
748+
free(ap_wifi_config);
749+
free(sta_wifi_config);
750+
port_ensure_available(ctx, TUPLE_SIZE(2));
751+
term error = port_create_error_tuple(ctx, OUT_OF_MEMORY_ATOM);
752+
port_send_reply(ctx, pid, ref, error);
753+
return;
754+
}
755+
} else if (!IS_NULL_PTR(ap_wifi_config)) {
732756
wifi_mode = WIFI_MODE_AP;
757+
bool ap_atoms_ok = init_ap_cb_atoms(ctx->global);
758+
if (UNLIKELY(!ap_atoms_ok)) {
759+
ESP_LOGE(TAG, "Unable to insert callback atoms");
760+
free(ap_wifi_config);
761+
port_ensure_available(ctx, TUPLE_SIZE(2));
762+
term error = port_create_error_tuple(ctx, OUT_OF_MEMORY_ATOM);
763+
port_send_reply(ctx, pid, ref, error);
764+
return;
765+
}
766+
} else {
767+
wifi_mode = WIFI_MODE_STA;
768+
bool sta_atoms_ok = init_sta_atoms(ctx->global);
769+
if (UNLIKELY(!sta_atoms_ok)) {
770+
ESP_LOGE(TAG, "Unable to insert callback atoms");
771+
free(sta_wifi_config);
772+
port_ensure_available(ctx, TUPLE_SIZE(2));
773+
term error = port_create_error_tuple(ctx, OUT_OF_MEMORY_ATOM);
774+
port_send_reply(ctx, pid, ref, error);
775+
return;
776+
}
733777
}
734778
if ((err = esp_wifi_set_mode(wifi_mode)) != ESP_OK) {
735779
ESP_LOGE(TAG, "Error setting wifi mode %d", err);
@@ -790,16 +834,16 @@ static void start_network(Context *ctx, term pid, term ref, term config)
790834
//
791835
// Set up simple NTP, if configured
792836
//
793-
maybe_set_sntp(interop_kv_get_value(config, sntp_atom, ctx->global), ctx->global);
837+
maybe_set_sntp(interop_kv_get_value(config, ATOM_STR("\x4", "sntp"), ctx->global), ctx->global);
794838

795839
//
796840
// Set the DHCP hostname, if STA mode is enabled
797841
//
798842
if (!IS_NULL_PTR(sta_wifi_config)) {
799-
set_dhcp_hostname(sta_wifi_interface, "STA", interop_kv_get_value(sta_config, dhcp_hostname_atom, ctx->global));
843+
set_dhcp_hostname(sta_wifi_interface, "STA", interop_kv_get_value(sta_config, ATOM_STR("\xD", "dhcp_hostname"), ctx->global));
800844
}
801845
if (!IS_NULL_PTR(ap_wifi_config)) {
802-
set_dhcp_hostname(ap_wifi_interface, "AP", interop_kv_get_value(ap_config, dhcp_hostname_atom, ctx->global));
846+
set_dhcp_hostname(ap_wifi_interface, "AP", interop_kv_get_value(ap_config, ATOM_STR("\xD", "dhcp_hostname"), ctx->global));
803847
}
804848

805849
//
@@ -865,14 +909,15 @@ static void get_sta_rssi(Context *ctx, term pid, term ref)
865909
ESP_LOGE(TAG, "Device is not connected to any AP.");
866910
// Reply: {Ref, {error, network_down}}
867911
port_ensure_available(ctx, tuple_reply_size);
868-
term error = port_create_error_tuple(ctx, make_atom(ctx->global, network_down_atom));
912+
term error = port_create_error_tuple(ctx, globalcontext_existing_term_from_atom_string(ctx->global, ATOM_STR("\x0C", "network_down")));
869913
port_send_reply(ctx, pid, ref, error);
870914
return;
871915
}
872916
term rssi = term_from_int(sta_rssi);
917+
int rssi_atom = globalcontext_existing_term_from_atom_string(ctx->global, ATOM_STR("\x4", "rssi"));
873918
// {Ref, {rssi, -25}}
874919
port_ensure_available(ctx, tuple_reply_size);
875-
term reply = port_create_tuple2(ctx, make_atom(ctx->global, ATOM_STR("\x4", "rssi")), rssi);
920+
term reply = port_create_tuple2(ctx, rssi_atom, rssi);
876921
port_send_reply(ctx, pid, ref, reply);
877922
}
878923

0 commit comments

Comments
 (0)