Skip to content

Commit bc12729

Browse files
committed
Terminate filename strings and increase any array by 1 that uses strcpy_P.
strcpy_P null terminates the provided string. strlen does not count \0s so any string that uses strcpy_P needs an extra byte for \0.
1 parent 795dc84 commit bc12729

File tree

1 file changed

+60
-67
lines changed

1 file changed

+60
-67
lines changed

firmware/OpenLog_Firmware/OpenLog/OpenLog.ino

+60-67
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ SerialPort<0, 512, 0> NewSerial;
9696

9797
void(* Reset_AVR) (void) = 0; //Way of resetting the ATmega
9898

99-
#define CFG_FILENAME "config.txt" //This is the name of the file that contains the unit settings
99+
#define CFG_FILENAME "config.txt\0" //This is the name of the file that contains the unit settings
100100

101101
#define MAX_CFG "115200,255,255,1,1,1,1,255,255\0" // This is used to calculate the longest possible configuration string. These actual values are not used
102102
#define CFG_LENGTH (strlen(MAX_CFG) + 1) //Length of text found in config file. strlen ignores \0 so we have to add it back
103-
#define SEQ_FILENAME "SEQLOG00.TXT" //This is the name for the file when you're in sequential mode
103+
#define SEQ_FILENAME "SEQLOG00.TXT\0" //This is the name for the file when you're in sequential mode
104104

105105
//Internal EEPROM locations for the user settings
106106
#define LOCATION_SYSTEM_SETTING 0x02
@@ -272,12 +272,12 @@ void loop(void)
272272
{
273273
//If in MODE_NEWLOG, then just append the file name that newLog() returns and ignore return value of appendFile()
274274
//If in MODE_ROTATE, then as long as appendFile() keeps returning 0 (meaning the file is full and a new one
275-
// needs to be started) keep creating new files and logging new data. If appendFile() returns a 1 then the escape
275+
// needs to be started) keep creating new files and logging new data. If appendFile() returns a 1 then the escape
276276
// sequence has been triggered so drop out of this while() loop and let commandShell() run.
277277
while ((appendFile(newLog()) == 0) && (setting_systemMode == MODE_ROTATE))
278278
{ } // While loop purposely empty
279279
}
280-
280+
281281
//If we are in sequential log mode, determine if seqLog.txt has been created or not, and then open it for logging
282282
if (setting_systemMode == MODE_SEQLOG)
283283
seqLog();
@@ -341,11 +341,11 @@ char* newLog(void)
341341
//arbitrarily. This fixes that problem.
342342
/// TODO: (BPS) I don't understand why this is here. Your file number is then always one less than what's in EEPROM, which doesn't make sense.
343343
/// I'm commenting this out so that we always use the actual filenumber stored in EEPROM
344-
// if (newFileNumber > 0) newFileNumber--;
344+
// if (newFileNumber > 0) newFileNumber--;
345345

346346
static char newFileName[13]; //Bug fix from ystark's pull request: https://github.com/sparkfun/OpenLog/pull/189
347347

348-
// When in MODE_ROTATE, we don't care if the file exists, or if it is empty, or anything. We will always
348+
// When in MODE_ROTATE, we don't care if the file exists, or if it is empty, or anything. We will always
349349
// blindly create whatever the next filename is and use it.
350350
if (setting_systemMode == MODE_ROTATE)
351351
{
@@ -357,15 +357,15 @@ char* newLog(void)
357357
while (1)
358358
{
359359
sprintf_P(newFileName, PSTR("LOG%05u.TXT"), newFileNumber); //Splice the new file number into this file name
360-
360+
361361
// O_CREAT - create the file if it does not exist
362362
// O_APPEND - seek to the end of the file prior to each write
363363
// O_WRITE - open for write
364364
// O_EXCL - if O_CREAT and O_EXCEL are set, open() shall fail if file exists
365-
365+
366366
//Try to open file, if it opens (file doesn't exist), then break
367367
if (newFile.open(newFileName, O_CREAT | O_EXCL | O_WRITE)) break;
368-
368+
369369
//Try to open file and see if it is empty. If so, use it.
370370
if (newFile.open(newFileName, O_READ))
371371
{
@@ -376,7 +376,7 @@ char* newLog(void)
376376
}
377377
newFile.close(); // Close this existing file we just opened.
378378
}
379-
379+
380380
//Try the next number
381381
newFileNumber++;
382382
if (newFileNumber > 65533) //There is a max of 65534 logs
@@ -387,7 +387,7 @@ char* newLog(void)
387387
}
388388
newFile.close(); //Close this new file we just opened
389389
}
390-
390+
391391
newFileNumber++; //Increment so the next power up uses the next file #
392392

393393
//Record new_file number to EEPROM
@@ -420,7 +420,7 @@ void seqLog(void)
420420
{
421421
SdFile seqFile;
422422

423-
char sequentialFileName[strlen(SEQ_FILENAME)]; //Limited to 8.3
423+
char sequentialFileName[strlen(SEQ_FILENAME) + 1]; //Limited to 8.3
424424
strcpy_P(sequentialFileName, PSTR(SEQ_FILENAME)); //This is the name of the config file. 'config.sys' is probably a bad idea.
425425

426426
//Try to create sequential file
@@ -469,7 +469,7 @@ byte appendFile(char* fileName)
469469
// O_WRITE - open for write
470470
if (!workingFile.open(fileName, O_CREAT | O_TRUNC | O_WRITE)) systemError(ERROR_FILE_OPEN);
471471
}
472-
472+
473473
if (workingFile.fileSize() == 0) {
474474
//This is a trick to make sure first cluster is allocated - found in Bill's example/beta code
475475
workingFile.rewind();
@@ -517,13 +517,13 @@ byte appendFile(char* fileName)
517517
totalBytesWritten += charsToRecord; // Add these new bytes to our running total
518518
if (totalBytesWritten >= maxFilesizeBytes)
519519
{
520-
workingFile.sync();
521-
workingFile.close(); // Done recording, close out the file
522-
523-
digitalWrite(stat1, LOW); // Turn off indicator LED
524-
525-
NewSerial.print(F("~")); // Indicate a successful record
526-
return(0); // Indicate to caller that we are done writing to this file and desire another
520+
workingFile.sync();
521+
workingFile.close(); // Done recording, close out the file
522+
523+
digitalWrite(stat1, LOW); // Turn off indicator LED
524+
525+
NewSerial.print(F("~")); // Indicate a successful record
526+
return (0); // Indicate to caller that we are done writing to this file and desire another
527527
}
528528
}
529529
}
@@ -555,7 +555,7 @@ byte appendFile(char* fileName)
555555
}
556556
}
557557
}
558-
558+
559559
//We only get this far if escape characters are more than zero
560560

561561
//Start recording incoming characters
@@ -595,13 +595,13 @@ byte appendFile(char* fileName)
595595
totalBytesWritten += charsToRecord; // Add these new bytes to our running total
596596
if (totalBytesWritten >= maxFilesizeBytes)
597597
{
598-
workingFile.sync();
599-
workingFile.close(); // Done recording, close out the file
600-
601-
digitalWrite(stat1, LOW); // Turn off indicator LED
602-
603-
NewSerial.print(F("~")); // Indicate a successful record
604-
return(0); // Indicate to caller that we are done writing to this file and desire another
598+
workingFile.sync();
599+
workingFile.close(); // Done recording, close out the file
600+
601+
digitalWrite(stat1, LOW); // Turn off indicator LED
602+
603+
NewSerial.print(F("~")); // Indicate a successful record
604+
return (0); // Indicate to caller that we are done writing to this file and desire another
605605
}
606606
}
607607
}
@@ -638,7 +638,7 @@ byte appendFile(char* fileName)
638638
//Remove the escape characters from the end of the file
639639
if (setting_max_escape_character > 0)
640640
workingFile.truncate(workingFile.fileSize() - setting_max_escape_character);
641-
641+
642642
workingFile.close(); // Done recording, close out the file
643643

644644
digitalWrite(stat1, LOW); // Turn off indicator LED
@@ -847,7 +847,7 @@ void readConfigFile(void)
847847

848848
if (!sd.chdir()) systemError(ERROR_ROOT_INIT); // open the root directory
849849

850-
char configFileName[strlen(CFG_FILENAME)]; //Limited to 8.3
850+
char configFileName[strlen(CFG_FILENAME) + 1]; //Limited to 8.3
851851
strcpy_P(configFileName, PSTR(CFG_FILENAME)); //This is the name of the config file. 'config.sys' is probably a bad idea.
852852

853853
//Check to see if we have a config file
@@ -961,16 +961,16 @@ void readConfigFile(void)
961961
{
962962
new_setting_max_filesize_MB = newSettingInt;
963963
#if DEBUG
964-
NewSerial.print(F("MaxFilesize from file: "));
965-
NewSerial.println(new_setting_max_filesize_MB);
964+
NewSerial.print(F("MaxFilesize from file: "));
965+
NewSerial.println(new_setting_max_filesize_MB);
966966
#endif
967967
}
968968
else if (settingNumber == 8) // Rotate mode max filenumber setting
969969
{
970970
new_setting_max_filenumber = newSettingInt;
971971
#if DEBUG
972-
NewSerial.print(F("MaxFilenumber from file: "));
973-
NewSerial.println(new_setting_max_filenumber);
972+
NewSerial.print(F("MaxFilenumber from file: "));
973+
NewSerial.println(new_setting_max_filenumber);
974974
#endif
975975
}
976976
else
@@ -1067,7 +1067,7 @@ void readConfigFile(void)
10671067
NewSerial.println(F("Config file matches system settings"));
10681068
#endif
10691069
}
1070-
1070+
10711071
//All done! New settings are loaded. System will now operate off new config settings found in file.
10721072

10731073
//Set flags for extended mode options
@@ -1091,19 +1091,12 @@ void recordConfigFile(void)
10911091

10921092
if (!sd.chdir()) systemError(ERROR_ROOT_INIT); // open the root directory
10931093

1094-
char configFileName[strlen(CFG_FILENAME)];
1094+
char configFileName[strlen(CFG_FILENAME) + 1];
10951095
strcpy_P(configFileName, PSTR(CFG_FILENAME)); //This is the name of the config file. 'config.sys' is probably a bad idea.
10961096

10971097
//If there is currently a config file, trash it
1098-
if (myFile.open(configFileName, O_WRITE)) {
1099-
if (!myFile.remove()) {
1100-
NewSerial.println(F("Remove config failed"));
1101-
myFile.close(); //Close this file
1102-
return;
1103-
}
1104-
}
1105-
1106-
//myFile.close(); //Not sure if we need to close the file before we try to reopen it
1098+
if (sd.exists(configFileName))
1099+
sd.remove(configFileName);
11071100

11081101
//Create config file
11091102
myFile.open(configFileName, O_CREAT | O_APPEND | O_WRITE);
@@ -1125,14 +1118,14 @@ void recordConfigFile(void)
11251118

11261119
//Convert system settings to visible ASCII characters
11271120
sprintf_P(
1128-
settingsString,
1129-
PSTR("%ld,%d,%d,%d,%d,%d,%d,%d,%d\0"),
1130-
current_system_baud,
1131-
current_system_escape,
1132-
current_system_max_escape,
1133-
current_systemMode,
1134-
current_system_verbose,
1135-
current_system_echo,
1121+
settingsString,
1122+
PSTR("%ld,%d,%d,%d,%d,%d,%d,%d,%d\0"),
1123+
current_system_baud,
1124+
current_system_escape,
1125+
current_system_max_escape,
1126+
current_systemMode,
1127+
current_system_verbose,
1128+
current_system_echo,
11361129
current_system_ignore_RX,
11371130
current_system_max_filesize_MB,
11381131
current_system_max_filenumber
@@ -1184,7 +1177,7 @@ void commandShell(void)
11841177
SdFile tempFile;
11851178
sd.chdir("/", true); //Change to root directory
11861179

1187-
char buffer[30];
1180+
char commandBuffer[30];
11881181
byte tempVar;
11891182

11901183
//char parentDirectory[13]; //This tracks the current parent directory. Limited to 13 characters.
@@ -1211,7 +1204,7 @@ void commandShell(void)
12111204
NewSerial.print(F(">"));
12121205

12131206
//Read command
1214-
if (readLine(buffer, sizeof(buffer)) < 1)
1207+
if (readLine(commandBuffer, sizeof(commandBuffer)) < 1)
12151208
{
12161209
#ifdef INCLUDE_SIMPLE_EMBEDDED
12171210
commandSucceeded = 1;
@@ -1564,7 +1557,7 @@ void commandShell(void)
15641557
NewSerial.print(F("<")); //give a different prompt
15651558

15661559
//read one line of text
1567-
dataLen = readLine(buffer, sizeof(buffer));
1560+
dataLen = readLine(commandBuffer, sizeof(commandBuffer));
15681561
if (!dataLen) {
15691562
#ifdef INCLUDE_SIMPLE_EMBEDDED
15701563
commandSucceeded = 1;
@@ -1583,13 +1576,13 @@ void commandShell(void)
15831576
//}
15841577

15851578
//write text to file
1586-
if (tempFile.write((byte*) buffer, dataLen) != dataLen) {
1579+
if (tempFile.write((byte*) commandBuffer, dataLen) != dataLen) {
15871580
if ((feedbackMode & EXTENDED_INFO) > 0)
15881581
NewSerial.println(F("error writing to file"));
15891582
break;
15901583
}
15911584

1592-
if (dataLen < (sizeof(buffer) - 1)) tempFile.write("\n\r", 2); //If we didn't fill up the buffer then user must have sent NL. Append new line and return
1585+
if (dataLen < (sizeof(commandBuffer) - 1)) tempFile.write("\n\r", 2); //If we didn't fill up the buffer then user must have sent NL. Append new line and return
15931586
}
15941587

15951588
tempFile.close();
@@ -1831,9 +1824,9 @@ void commandShell(void)
18311824
}
18321825

18331826
//Reads a line until the \n enter character is found
1834-
byte readLine(char* buffer, byte bufferLength)
1827+
byte readLine(char* readBuffer, byte bufferLength)
18351828
{
1836-
memset(buffer, 0, bufferLength); //Clear buffer
1829+
memset(readBuffer, 0, bufferLength); //Clear buffer
18371830

18381831
byte readLength = 0;
18391832
while (readLength < bufferLength - 1) {
@@ -1847,7 +1840,7 @@ byte readLine(char* buffer, byte bufferLength)
18471840
continue;
18481841

18491842
--readLength;
1850-
buffer[readLength] = '\0'; //Put a terminator on the string in case we are finished
1843+
readBuffer[readLength] = '\0'; //Put a terminator on the string in case we are finished
18511844

18521845
NewSerial.print((char)0x08); //Move back one space
18531846
NewSerial.print(F(" ")); //Put a blank there to erase the letter from the terminal
@@ -1862,7 +1855,7 @@ byte readLine(char* buffer, byte bufferLength)
18621855

18631856
if (c == '\r') {
18641857
NewSerial.println();
1865-
buffer[readLength] = '\0';
1858+
readBuffer[readLength] = '\0';
18661859
break;
18671860
}
18681861
else if (c == '\n') {
@@ -1884,13 +1877,13 @@ byte readLine(char* buffer, byte bufferLength)
18841877
//See issue 168: https://github.com/sparkfun/OpenLog/issues/168
18851878
}*/
18861879
else {
1887-
buffer[readLength] = c;
1880+
readBuffer[readLength] = c;
18881881
++readLength;
18891882
}
18901883
}
18911884

18921885
//Split the command line into arguments
1893-
splitCmdLineArgs(buffer, bufferLength);
1886+
splitCmdLineArgs(readBuffer, bufferLength);
18941887

18951888
return readLength;
18961889
}
@@ -1985,7 +1978,7 @@ void baudMenu(void)
19851978
//1) New File Mode: Turn on unit, unit will create new file, and just start logging
19861979
//2) Append File Mode: Turn on, append to known file, and just start logging
19871980
//3) Command Mode: Turn on, sit at command prompt
1988-
//4) Rotate Mode: Turn on, append data to current log file until it becomes to big,
1981+
//4) Rotate Mode: Turn on, append data to current log file until it becomes to big,
19891982
// then move on to next, rotating to overwrite first one after max file count has been reached
19901983
//5) Resets the newLog file number to zero
19911984
//6) Change the escape charater
@@ -2155,7 +2148,7 @@ void systemMenu(void)
21552148
SdFile myFile;
21562149
if (!sd.chdir()) systemError(ERROR_ROOT_INIT); // open the root directory
21572150

2158-
char configFileName[strlen(CFG_FILENAME)];
2151+
char configFileName[strlen(CFG_FILENAME) + 1];
21592152
strcpy_P(configFileName, PSTR(CFG_FILENAME)); //This is the name of the config file. 'config.sys' is probably a bad idea.
21602153

21612154
//If there is currently a config file, trash it
@@ -2506,7 +2499,7 @@ void toggleLED(byte pinNumber)
25062499
else digitalWrite(pinNumber, HIGH);
25072500
}
25082501

2509-
// Read a numerical value from 0 to 255 from the serial port.
2502+
// Read a numerical value from 0 to 255 from the serial port.
25102503
// Return -1 if no value entered or if value entered > 255
25112504
// CR (Enter) or three digits terminate
25122505
int getSerialByte(void)

0 commit comments

Comments
 (0)