Skip to content

Commit 47e50cf

Browse files
hhvrchhvrc
committed
turn volatile bools into atomics
1 parent a49a490 commit 47e50cf

File tree

3 files changed

+77
-36
lines changed

3 files changed

+77
-36
lines changed

‎src/captiveportal/Manager.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,13 @@ const char* const TAG = "CaptivePortal";
1919

2020
#include "SimpleMutex.h"
2121

22+
#include <atomic>
2223
#include <memory>
2324

2425
using namespace OpenShock;
2526

26-
static volatile bool s_alwaysEnabled = false;
27-
static volatile bool s_forceClosed = false;
27+
static std::atomic<bool> s_alwaysEnabled = false;
28+
static std::atomic<bool> s_forceClosed = false;
2829
static esp_timer_handle_t s_captivePortalUpdateLoopTimer = nullptr;
2930
static SimpleMutex s_instanceMutex;
3031
static std::unique_ptr<CaptivePortal::CaptivePortalInstance> s_instance = nullptr;

‎src/http/JsonAPI.cpp‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
#include "http/JsonAPI.h"
22

3+
const char* const TAG = "JsonAPI";
4+
35
#include "Common.h"
6+
#include "Logging.h"
47
#include "config/Config.h"
58
#include "util/StringUtils.h"
69

@@ -14,7 +17,11 @@ HTTP::Response<Serialization::JsonAPI::AccountLinkResponse> HTTP::JsonAPI::LinkA
1417
}
1518

1619
char uri[OPENSHOCK_URI_BUFFER_SIZE];
17-
snprintf(uri, sizeof(uri), "https://%s/1/device/pair/%.*s", domain.c_str(), static_cast<int>(accountLinkCode.length()), accountLinkCode.data());
20+
int written = snprintf(uri, sizeof(uri), "https://%s/1/device/pair/%.*s", domain.c_str(), static_cast<int>(accountLinkCode.length()), accountLinkCode.data());
21+
if (written < 0 || static_cast<size_t>(written) >= sizeof(uri)) {
22+
OS_LOGE(TAG, "URI truncated for LinkAccount");
23+
return {HTTP::RequestResult::InternalError, 0, {}};
24+
}
1825

1926
return HTTP::GetJSON<Serialization::JsonAPI::AccountLinkResponse>(
2027
uri,
@@ -34,7 +41,11 @@ HTTP::Response<Serialization::JsonAPI::HubInfoResponse> HTTP::JsonAPI::GetHubInf
3441
}
3542

3643
char uri[OPENSHOCK_URI_BUFFER_SIZE];
37-
snprintf(uri, sizeof(uri), "https://%s/1/device/self", domain.c_str());
44+
int written = snprintf(uri, sizeof(uri), "https://%s/1/device/self", domain.c_str());
45+
if (written < 0 || static_cast<size_t>(written) >= sizeof(uri)) {
46+
OS_LOGE(TAG, "URI truncated for GetHubInfo");
47+
return {HTTP::RequestResult::InternalError, 0, {}};
48+
}
3849

3950
return HTTP::GetJSON<Serialization::JsonAPI::HubInfoResponse>(
4051
uri,
@@ -55,7 +66,11 @@ HTTP::Response<Serialization::JsonAPI::AssignLcgResponse> HTTP::JsonAPI::AssignL
5566
}
5667

5768
char uri[OPENSHOCK_URI_BUFFER_SIZE];
58-
snprintf(uri, sizeof(uri), "https://%s/2/device/assignLCG?version=2", domain.c_str());
69+
int written = snprintf(uri, sizeof(uri), "https://%s/2/device/assignLCG?version=2", domain.c_str());
70+
if (written < 0 || static_cast<size_t>(written) >= sizeof(uri)) {
71+
OS_LOGE(TAG, "URI truncated for AssignLcg");
72+
return {HTTP::RequestResult::InternalError, 0, {}};
73+
}
5974

6075
return HTTP::GetJSON<Serialization::JsonAPI::AssignLcgResponse>(
6176
uri,

‎src/wifi/WiFiManager.cpp‎

Lines changed: 56 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ enum class WiFiState : uint8_t {
3333
};
3434

3535
static std::atomic<WiFiState> s_wifiState {WiFiState::Disconnected};
36-
static uint8_t s_connectedBSSID[6] = {0};
37-
static uint8_t s_connectedCredentialsID = 0;
38-
static uint8_t s_preferredCredentialsID = 0;
36+
static uint8_t s_connectedBSSID[6] = {0};
37+
static std::atomic<uint8_t> s_connectedCredentialsID = 0;
38+
static std::atomic<uint8_t> s_preferredCredentialsID = 0;
3939
static OpenShock::SimpleMutex s_networksMutex;
4040
static std::vector<WiFiNetwork> s_wifiNetworks;
4141

@@ -214,14 +214,14 @@ void _evWiFiConnected(arduino_event_t* event)
214214

215215
auto it = _findNetworkByBSSID(info.bssid);
216216
if (it == s_wifiNetworks.end()) {
217-
s_connectedCredentialsID = 0;
217+
s_connectedCredentialsID.store(0, std::memory_order_relaxed);
218218

219219
OS_LOGW(TAG, "Connected to unscanned network \"%s\", BSSID: " BSSID_FMT, reinterpret_cast<char*>(info.ssid), BSSID_ARG(info.bssid));
220220

221221
return;
222222
}
223223

224-
s_connectedCredentialsID = it->credentialsID;
224+
s_connectedCredentialsID.store(it->credentialsID, std::memory_order_relaxed);
225225

226226
OS_LOGI(TAG, "Connected to network %s (" BSSID_FMT ")", reinterpret_cast<const char*>(info.ssid), BSSID_ARG(info.bssid));
227227

@@ -343,32 +343,45 @@ esp_err_t set_esp_interface_dns(esp_interface_t interface, IPAddress main_dns, I
343343
bool _tryConnect()
344344
{
345345
Config::WiFiCredentials creds;
346+
uint8_t bssid[6];
346347

347-
// Select target network under lock, then release before connecting
348+
// Select target network under lock, resolve BSSID and mark as attempted, then release before connecting
348349
{
349350
ScopedLock lock__(&s_networksMutex);
350351

351-
if (s_preferredCredentialsID != 0) {
352-
uint8_t preferredId = s_preferredCredentialsID;
353-
s_preferredCredentialsID = 0;
354-
352+
uint8_t preferredId = s_preferredCredentialsID.exchange(0, std::memory_order_relaxed);
353+
if (preferredId != 0) {
355354
if (!Config::TryGetWiFiCredentialsByID(preferredId, creds)) {
356355
OS_LOGE(TAG, "Failed to find credentials with ID %u", preferredId);
357356
return false;
358357
}
359-
360-
// creds populated — connect outside the lock
361358
} else if (!_getNextWiFiNetwork(creds)) {
362359
return false;
363360
}
361+
362+
// Resolve BSSID and mark as attempted while we still hold the lock
363+
auto it = _findNetworkBySSID(creds.ssid.c_str());
364+
if (it == s_wifiNetworks.end()) {
365+
OS_LOGE(TAG, "Failed to find network with SSID %s", creds.ssid.c_str());
366+
return false;
367+
}
368+
369+
memcpy(bssid, it->bssid, sizeof(bssid));
370+
it->connectAttempts++;
371+
it->lastConnectAttempt = OpenShock::millis();
364372
}
365373

366-
if (_connect(creds.ssid, creds.password)) {
367-
return true;
374+
// Connect outside the lock
375+
OS_LOGV(TAG, "Connecting to network %s (" BSSID_FMT ")", creds.ssid.c_str(), BSSID_ARG(bssid));
376+
377+
s_wifiState.store(WiFiState::Connecting, std::memory_order_relaxed);
378+
if (WiFi.begin(creds.ssid.c_str(), creds.password.c_str(), 0, bssid, true) == WL_CONNECT_FAILED) {
379+
s_wifiState.store(WiFiState::Disconnected, std::memory_order_relaxed);
380+
OS_LOGE(TAG, "Failed to connect to network %s", creds.ssid.c_str());
381+
return false;
368382
}
369383

370-
OS_LOGE(TAG, "Failed to connect to network %s", creds.ssid.c_str());
371-
return false;
384+
return true;
372385
}
373386

374387
void _wifimanagerUpdateTask(void*)
@@ -471,7 +484,7 @@ bool WiFiManager::Forget(const char* ssid)
471484
uint8_t credsId = it->credentialsID;
472485

473486
// Check if the network is currently connected
474-
if (s_connectedCredentialsID == credsId) {
487+
if (s_connectedCredentialsID.load(std::memory_order_relaxed) == credsId) {
475488
// Disconnect from the network
476489
WiFiManager::Disconnect();
477490
}
@@ -518,14 +531,14 @@ bool WiFiManager::Connect(const char* ssid)
518531
return false;
519532
}
520533

521-
if (s_connectedCredentialsID != creds.id) {
534+
if (s_connectedCredentialsID.load(std::memory_order_relaxed) != creds.id) {
522535
Disconnect();
523-
s_preferredCredentialsID = creds.id;
536+
s_preferredCredentialsID.store(creds.id, std::memory_order_relaxed);
524537
return true;
525538
}
526539

527540
if (s_wifiState.load(std::memory_order_relaxed) == WiFiState::Disconnected) {
528-
s_preferredCredentialsID = creds.id;
541+
s_preferredCredentialsID.store(creds.id, std::memory_order_relaxed);
529542
return true;
530543
}
531544

@@ -534,26 +547,34 @@ bool WiFiManager::Connect(const char* ssid)
534547

535548
bool WiFiManager::Connect(const uint8_t (&bssid)[6])
536549
{
537-
auto it = _findNetworkByBSSID(bssid);
538-
if (it == s_wifiNetworks.end()) {
539-
OS_LOGE(TAG, "Failed to find network " BSSID_FMT, BSSID_ARG(bssid));
540-
return false;
550+
char ssid[33];
551+
552+
{
553+
ScopedLock lock__(&s_networksMutex);
554+
555+
auto it = _findNetworkByBSSID(bssid);
556+
if (it == s_wifiNetworks.end()) {
557+
OS_LOGE(TAG, "Failed to find network " BSSID_FMT, BSSID_ARG(bssid));
558+
return false;
559+
}
560+
561+
memcpy(ssid, it->ssid, sizeof(ssid));
541562
}
542563

543564
Config::WiFiCredentials creds;
544-
if (!Config::TryGetWiFiCredentialsBySSID(it->ssid, creds)) {
545-
OS_LOGE(TAG, "Failed to find credentials for network %s (" BSSID_FMT ")", it->ssid, BSSID_ARG(it->bssid));
565+
if (!Config::TryGetWiFiCredentialsBySSID(ssid, creds)) {
566+
OS_LOGE(TAG, "Failed to find credentials for network %s (" BSSID_FMT ")", ssid, BSSID_ARG(bssid));
546567
return false;
547568
}
548569

549-
if (s_connectedCredentialsID != creds.id) {
570+
if (s_connectedCredentialsID.load(std::memory_order_relaxed) != creds.id) {
550571
Disconnect();
551-
s_preferredCredentialsID = creds.id;
572+
s_preferredCredentialsID.store(creds.id, std::memory_order_relaxed);
552573
return true;
553574
}
554575

555576
if (s_wifiState.load(std::memory_order_relaxed) == WiFiState::Disconnected) {
556-
s_preferredCredentialsID = creds.id;
577+
s_preferredCredentialsID.store(creds.id, std::memory_order_relaxed);
557578
return true;
558579
}
559580

@@ -571,7 +592,9 @@ bool WiFiManager::IsConnected()
571592
}
572593
bool WiFiManager::GetConnectedNetwork(OpenShock::WiFiNetwork& network)
573594
{
574-
if (s_connectedCredentialsID == 0) {
595+
uint8_t connectedId = s_connectedCredentialsID.load(std::memory_order_relaxed);
596+
597+
if (connectedId == 0) {
575598
if (IsConnected()) {
576599
// We connected without a scan, so populate the network with the current connection info manually
577600
network.credentialsID = 0;
@@ -589,7 +612,9 @@ bool WiFiManager::GetConnectedNetwork(OpenShock::WiFiNetwork& network)
589612
return false;
590613
}
591614

592-
auto it = _findNetwork([](const WiFiNetwork& net) noexcept { return net.credentialsID == s_connectedCredentialsID; });
615+
ScopedLock lock__(&s_networksMutex);
616+
617+
auto it = _findNetwork([connectedId](const WiFiNetwork& net) noexcept { return net.credentialsID == connectedId; });
593618
if (it == s_wifiNetworks.end()) {
594619
return false;
595620
}

0 commit comments

Comments
 (0)