Skip to content

Commit 7575db0

Browse files
capricornusxfifieldt
authored andcommitted
Add comprehensive documentation for safety measures
- Document memory protection mechanisms in getFilesRecursive() - Explain file count, path length, and recursion depth limits - Improve code maintainability with clear safety constraints
1 parent 9901cbb commit 7575db0

File tree

2 files changed

+31
-11
lines changed

2 files changed

+31
-11
lines changed

src/FSCommon.cpp

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,36 @@ bool renameFile(const char *pathFrom, const char *pathTo)
104104
#define MAX_FILES_IN_MANIFEST 50 // Reduced to be more conservative with memory
105105
#define MAX_PATH_LENGTH 200 // Maximum allowed path length to prevent overflow
106106

107+
/**
108+
* @brief Helper function to validate and get file path for current platform
109+
*
110+
* @param file The file object
111+
* @return const char* Valid path or nullptr if invalid
112+
*/
113+
static const char *getValidFilePath(File &file)
114+
{
115+
#ifdef ARCH_ESP32
116+
const char *filePath = file.path();
117+
return (filePath && strlen(filePath) < MAX_PATH_LENGTH) ? filePath : nullptr;
118+
#else
119+
const char *fileName = file.name();
120+
return (fileName && strlen(fileName) < MAX_PATH_LENGTH) ? fileName : nullptr;
121+
#endif
122+
}
123+
107124
/**
108125
* @brief Get the list of files in a directory (internal recursive helper).
109126
*
127+
* This function recursively lists files in the specified directory, subject to safety constraints
128+
* to protect memory and stack usage:
129+
* - Limits the total number of files collected to MAX_FILES_IN_MANIFEST (currently 50) to prevent memory overflow.
130+
* - Limits the maximum allowed path length to MAX_PATH_LENGTH (currently 200) to prevent buffer overflow.
131+
* - Limits recursion depth via the 'levels' parameter to avoid stack exhaustion.
132+
*
133+
* If any of these limits are reached, the function will stop collecting further files or recursing into subdirectories.
134+
*
110135
* @param dirname The name of the directory.
111-
* @param levels The number of levels of subdirectories to list.
136+
* @param levels The number of levels of subdirectories to list (recursion depth).
112137
* @param filenames Reference to vector to populate with file info.
113138
*/
114139
static void getFilesRecursive(const char *dirname, uint8_t levels, std::vector<meshtastic_FileInfo> &filenames)
@@ -137,16 +162,10 @@ static void getFilesRecursive(const char *dirname, uint8_t levels, std::vector<m
137162

138163
if (file.isDirectory() && !String(fileName).endsWith(".")) {
139164
if (levels > 0) { // Limit recursion depth
140-
#ifdef ARCH_ESP32
141-
const char *filePath = file.path();
142-
if (filePath && strlen(filePath) < MAX_PATH_LENGTH) {
143-
getFilesRecursive(filePath, levels - 1, filenames);
144-
}
145-
#else
146-
if (strlen(fileName) < MAX_PATH_LENGTH) {
147-
getFilesRecursive(fileName, levels - 1, filenames);
165+
const char *validPath = getValidFilePath(file);
166+
if (validPath) {
167+
getFilesRecursive(validPath, levels - 1, filenames);
148168
}
149-
#endif
150169
}
151170
file.close();
152171
} else {

src/mesh/PhoneAPI.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#endif
55
#ifdef ARCH_ESP32
66
#include <esp_heap_caps.h>
7+
#define MIN_HEAP_FOR_FILE_MANIFEST 8192 // Minimum heap required before generating file manifest
78
#endif
89

910
#include "Channels.h"
@@ -72,7 +73,7 @@ void PhoneAPI::handleStartConfig()
7273
// Check available heap before getting files to prevent crash
7374
#ifdef ARCH_ESP32
7475
size_t freeHeap = ESP.getFreeHeap();
75-
if (freeHeap < 8192) { // Require at least 8KB free heap
76+
if (freeHeap < MIN_HEAP_FOR_FILE_MANIFEST) {
7677
LOG_WARN("Low memory (%d bytes), skipping file manifest", freeHeap);
7778
filesManifest.clear();
7879
} else {

0 commit comments

Comments
 (0)