From 058980178c524a50b85eedaf7cb28c79be503744 Mon Sep 17 00:00:00 2001 From: GUVWAF Date: Thu, 19 Dec 2024 20:09:33 +0100 Subject: [PATCH] Add locks to SafeFile, remove from `readcb`, introduce some LockGuards --- src/FSCommon.cpp | 5 +---- src/SafeFile.cpp | 15 +++++++++++---- src/SafeFile.h | 1 + src/mesh/NodeDB.cpp | 7 ++----- src/mesh/mesh-pb-constants.cpp | 4 ---- src/modules/RangeTestModule.cpp | 7 +------ src/modules/Telemetry/Sensor/NAU7802Sensor.cpp | 4 ++-- 7 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/FSCommon.cpp b/src/FSCommon.cpp index a5c17c2a8..ef96deda3 100644 --- a/src/FSCommon.cpp +++ b/src/FSCommon.cpp @@ -95,20 +95,18 @@ bool copyFile(const char *from, const char *to) #elif defined(FSCom) // take SPI Lock - spiLock->lock(); + concurrency::LockGuard g(spiLock); unsigned char cbuffer[16]; File f1 = FSCom.open(from, FILE_O_READ); if (!f1) { LOG_ERROR("Failed to open source file %s", from); - spiLock->unlock(); return false; } File f2 = FSCom.open(to, FILE_O_WRITE); if (!f2) { LOG_ERROR("Failed to open destination file %s", to); - spiLock->unlock(); return false; } @@ -120,7 +118,6 @@ bool copyFile(const char *from, const char *to) f2.flush(); f2.close(); f1.close(); - spiLock->unlock(); return true; #endif } diff --git a/src/SafeFile.cpp b/src/SafeFile.cpp index c76ff8054..c5d7b335e 100644 --- a/src/SafeFile.cpp +++ b/src/SafeFile.cpp @@ -5,6 +5,7 @@ // Only way to work on both esp32 and nrf52 static File openFile(const char *filename, bool fullAtomic) { + concurrency::LockGuard g(spiLock); if (!fullAtomic) FSCom.remove(filename); // Nuke the old file to make space (ignore if it !exists) @@ -53,14 +54,19 @@ bool SafeFile::close() if (!f) return false; + spiLock->lock(); f.close(); + spiLock->unlock(); if (!testReadback()) return false; - // brief window of risk here ;-) - if (fullAtomic && FSCom.exists(filename.c_str()) && !FSCom.remove(filename.c_str())) { - LOG_ERROR("Can't remove old pref file"); - return false; + { // Scope for lock + concurrency::LockGuard g(spiLock); + // brief window of risk here ;-) + if (fullAtomic && FSCom.exists(filename.c_str()) && !FSCom.remove(filename.c_str())) { + LOG_ERROR("Can't remove old pref file"); + return false; + } } String filenameTmp = filename; @@ -76,6 +82,7 @@ bool SafeFile::close() /// Read our (closed) tempfile back in and compare the hash bool SafeFile::testReadback() { + concurrency::LockGuard g(spiLock); bool lfs_failed = lfs_assert_failed; lfs_assert_failed = false; diff --git a/src/SafeFile.h b/src/SafeFile.h index 61361d312..3d0f81cad 100644 --- a/src/SafeFile.h +++ b/src/SafeFile.h @@ -1,6 +1,7 @@ #pragma once #include "FSCommon.h" +#include "SPILock.h" #include "configuration.h" #ifdef FSCom diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 8b4f4f504..7efc9d79e 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -839,7 +839,7 @@ LoadFileResult NodeDB::loadProto(const char *filename, size_t protoSize, size_t { LoadFileResult state = LoadFileResult::OTHER_FAILURE; #ifdef FSCom - spiLock->lock(); + concurrency::LockGuard g(spiLock); auto f = FSCom.open(filename, FILE_O_READ); @@ -859,7 +859,6 @@ LoadFileResult NodeDB::loadProto(const char *filename, size_t protoSize, size_t } else { LOG_ERROR("Could not open / read %s", filename); } - spiLock->unlock(); #else LOG_ERROR("ERROR: Filesystem not implemented"); state = LoadFileResult::NO_FILESYSTEM; @@ -1026,7 +1025,6 @@ bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_ { bool okay = false; #ifdef FSCom - spiLock->lock(); auto f = SafeFile(filename, fullAtomic); LOG_INFO("Save %s", filename); @@ -1039,7 +1037,6 @@ bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_ } bool writeSucceeded = f.close(); - spiLock->unlock(); if (!okay || !writeSucceeded) { LOG_ERROR("Can't write prefs!"); @@ -1452,4 +1449,4 @@ void recordCriticalError(meshtastic_CriticalErrorCode code, uint32_t address, co LOG_ERROR("A critical failure occurred, portduino is exiting"); exit(2); #endif -} +} \ No newline at end of file diff --git a/src/mesh/mesh-pb-constants.cpp b/src/mesh/mesh-pb-constants.cpp index 43b513cdf..a8f4fd6d8 100644 --- a/src/mesh/mesh-pb-constants.cpp +++ b/src/mesh/mesh-pb-constants.cpp @@ -37,13 +37,11 @@ bool pb_decode_from_bytes(const uint8_t *srcbuf, size_t srcbufsize, const pb_msg bool readcb(pb_istream_t *stream, uint8_t *buf, size_t count) { bool status = false; - spiLock->lock(); File *file = (File *)stream->state; if (buf == NULL) { while (count-- && file->read() != EOF) ; - spiLock->unlock(); return count == 0; } @@ -52,8 +50,6 @@ bool readcb(pb_istream_t *stream, uint8_t *buf, size_t count) if (file->available() == 0) stream->bytes_left = 0; - spiLock->unlock(); - return status; } diff --git a/src/modules/RangeTestModule.cpp b/src/modules/RangeTestModule.cpp index e3c8a2e2f..d5992233e 100644 --- a/src/modules/RangeTestModule.cpp +++ b/src/modules/RangeTestModule.cpp @@ -210,16 +210,14 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp) LOG_DEBUG("gpsStatus->getDOP() %d", gpsStatus->getDOP()); LOG_DEBUG("-----------------------------------------"); */ - spiLock->lock(); + concurrency::LockGuard g(spiLock); if (!FSBegin()) { LOG_DEBUG("An Error has occurred while mounting the filesystem"); - spiLock->unlock(); return 0; } if (FSCom.totalBytes() - FSCom.usedBytes() < 51200) { LOG_DEBUG("Filesystem doesn't have enough free space. Aborting write"); - spiLock->unlock(); return 0; } @@ -232,7 +230,6 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp) if (!fileToWrite) { LOG_ERROR("There was an error opening the file for writing"); - spiLock->unlock(); return 0; } @@ -252,7 +249,6 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp) if (!fileToAppend) { LOG_ERROR("There was an error opening the file for appending"); - spiLock->unlock(); return 0; } @@ -295,7 +291,6 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp) fileToAppend.printf("\"%s\"\n", p.payload.bytes); fileToAppend.flush(); fileToAppend.close(); - spiLock->unlock(); #endif return 1; diff --git a/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp b/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp index 075aedc52..1329c8d90 100644 --- a/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp +++ b/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp @@ -99,7 +99,6 @@ void NAU7802Sensor::tare() bool NAU7802Sensor::saveCalibrationData() { - spiLock->lock(); auto file = SafeFile(nau7802ConfigFileName); nau7802config.zeroOffset = nau7802.getZeroOffset(); nau7802config.calibrationFactor = nau7802.getCalibrationFactor(); @@ -113,6 +112,7 @@ bool NAU7802Sensor::saveCalibrationData() } else { okay = true; } + spiLock->lock(); okay &= file.close(); spiLock->unlock(); @@ -142,4 +142,4 @@ bool NAU7802Sensor::loadCalibrationData() return okay; } -#endif +#endif \ No newline at end of file