Skip to content

Commit aa03659

Browse files
Copilotdorkmo
andcommitted
Address code review: improve buffer handling and error checking
Co-authored-by: dorkmo <[email protected]>
1 parent 13d0950 commit aa03659

File tree

2 files changed

+44
-6
lines changed

2 files changed

+44
-6
lines changed

TankAlarm-112025-Client-BluesOpta/TankAlarm-112025-Client-BluesOpta.ino

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,8 +1969,19 @@ static void flushBufferedNotes() {
19691969
}
19701970

19711971
bool wroteFailures = false;
1972-
char lineBuffer[512];
1972+
char lineBuffer[1024]; // Larger buffer to accommodate payload data
19731973
while (fgets(lineBuffer, sizeof(lineBuffer), src) != nullptr) {
1974+
// Check if line was truncated (no newline at end of non-empty buffer)
1975+
size_t len = strlen(lineBuffer);
1976+
if (len == sizeof(lineBuffer) - 1 && lineBuffer[len - 1] != '\n') {
1977+
#ifdef DEBUG_MODE
1978+
Serial.println(F("Warning: truncated line in note buffer, skipping"));
1979+
#endif
1980+
// Skip rest of the truncated line
1981+
int ch;
1982+
while ((ch = fgetc(src)) != EOF && ch != '\n') {}
1983+
continue;
1984+
}
19741985
String line = String(lineBuffer);
19751986
line.trim();
19761987
if (line.length() == 0) {
@@ -2142,13 +2153,25 @@ static void pruneNoteBufferIfNeeded() {
21422153
return;
21432154
}
21442155

2145-
char ch;
2146-
while (fread(&ch, 1, 1, file) == 1) {
2147-
fwrite(&ch, 1, 1, tmp);
2156+
// Use buffered copy for better performance
2157+
char copyBuffer[256];
2158+
size_t bytesRead;
2159+
bool writeError = false;
2160+
while ((bytesRead = fread(copyBuffer, 1, sizeof(copyBuffer), file)) > 0) {
2161+
if (fwrite(copyBuffer, 1, bytesRead, tmp) != bytesRead) {
2162+
writeError = true;
2163+
break;
2164+
}
21482165
}
21492166

21502167
fclose(file);
21512168
fclose(tmp);
2169+
2170+
if (writeError) {
2171+
remove("/fs/pending_notes.tmp");
2172+
Serial.println(F("Failed to copy note buffer"));
2173+
return;
2174+
}
21522175
remove("/fs/pending_notes.log");
21532176
rename("/fs/pending_notes.tmp", "/fs/pending_notes.log");
21542177
Serial.println(F("Note buffer pruned"));

TankAlarm-112025-Server-BluesOpta/TankAlarm-112025-Server-BluesOpta.ino

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4215,8 +4215,18 @@ static void loadClientConfigSnapshots() {
42154215
return;
42164216
}
42174217

4218-
char lineBuffer[640]; // Should be large enough for uid + tab + payload
4218+
// Buffer size: uid + tab + payload + newline + null terminator
4219+
char lineBuffer[sizeof(((ClientConfigSnapshot*)0)->uid) + 1 + sizeof(((ClientConfigSnapshot*)0)->payload) + 2];
42194220
while (fgets(lineBuffer, sizeof(lineBuffer), file) != nullptr && gClientConfigCount < MAX_CLIENT_CONFIG_SNAPSHOTS) {
4221+
// Check if line was truncated (no newline at end of non-empty buffer)
4222+
size_t buflen = strlen(lineBuffer);
4223+
if (buflen == sizeof(lineBuffer) - 1 && lineBuffer[sizeof(lineBuffer) - 2] != '\n') {
4224+
Serial.println(F("Warning: truncated line in client config cache"));
4225+
// Skip the rest of the truncated line
4226+
int c;
4227+
while ((c = fgetc(file)) != '\n' && c != EOF) { }
4228+
continue;
4229+
}
42204230
String line = String(lineBuffer);
42214231
line.trim();
42224232
if (line.length() == 0) {
@@ -4313,7 +4323,12 @@ static void saveClientConfigSnapshots() {
43134323
}
43144324

43154325
for (uint8_t i = 0; i < gClientConfigCount; ++i) {
4316-
fprintf(file, "%s\t%s\n", gClientConfigs[i].uid, gClientConfigs[i].payload);
4326+
if (fprintf(file, "%s\t%s\n", gClientConfigs[i].uid, gClientConfigs[i].payload) < 0) {
4327+
Serial.println(F("Failed to write client config cache"));
4328+
fclose(file);
4329+
remove("/fs/client_config_cache.txt");
4330+
return;
4331+
}
43174332
}
43184333

43194334
fclose(file);

0 commit comments

Comments
 (0)