Prevent character array from overflowing in c

Question

I'm designing the firmware for ESP32 using the ESP-IDF and FreeRTOS. I want to convert the reading of a sensor into a char array and store it in the Non-Volatile Storage. When a new reading is taken, it is to be added to the front of the char array pushing the older readings to the right.

I'm doing the array manipulation in this way:

#define MAX_BYTES 100
char oldData[MAX_BYTES];
nvs_get_str( nvsHandle, MASS_STRING_STORE, newData, &required_size);
char newData[15];
sprintf(newData, "%2.2f", Totalmass);
strcat(newData, ",");
printf("new data: %s\n", newData);
printf("strlen oldData: %d\n", strlen(oldData));
printf("strlen newData: %d\n", strlen(newData));
printf("sizeof oldData: %d\n", sizeof(oldData));
printf("i starts from: %d\n", (sizeof(oldData)-strlen(newData2)-1));
for(int i = (sizeof(oldData)-strlen(newData) - 1); i >= 0; i--)
{
    oldData [i + strlen( newData )] = oldData[i];
}
for(int i = 0; i < ( strlen(newData) ); i++)
{
    oldData[i] = newData[i];
}
nvs_set_str(nvsHandle, MASS_STRING_STORE, oldData);

Now coming to the problem I'm facing:

The code crashes as soon as the string length exceeds MAX_LENGTH i.e, 100.
The crash message is:

"Guru Meditation Error: Core 0 panic'ed (Interrupt wdt timeout on CPU0)"

After the crash reset occurs, the code continues to work fine until it crashes again. Strlen of oldData prints 104, and stays at 104(I guess the max should be 99?). Code crashes exactly after completion of one complete loop of the task's infinite loop.

Can someone please guide me on what I might be doing wrong here? I can provide more info if needed.

Thanks in advance!

EDIT:

So turns out the following line was uncommented:

strcpy(NEWDATA, oldData);

where NEWDATA is an array of size 20, which was obviously overflowing and hence causing the above problem, which is now solved. Another problem I'm currently facing is that a copy of my current reading is getting attached to the end of array. Below I've attached a copy of my logs:

new data: 5.00,
strlen oldData: 105
strlen newData: 5

Final Data: 5.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,0.00,5.00,

For a MAX_LENGTH of 100, my array is of size 105 and stays more than 100 depending upon the length of my new reading. However, my code isnt crashing. But the extra 5 length of my final data is always the current reading that I get. Can anyone help me out with this?

Answer 1

You're doing a copy of newData into oldData. strlen() returns the length of newData excluding the null terminator. Your resultant char array could therefore be unterminated, causing the nvs_set_str() call to perform undesired action, as well as taking enough time to trigger the WDT.

Also, your byte-by-byte shift of oldData MIGHT be sped up by memcpy(). I say might because I'm not sure memcpy works correctly on an in situ replacement; you'll need to read the docs on that.

Answer 2

"Interrupt wdt timeout on CPU0" is perfectly clear. You have a watchdog timeout reset. Which in turn means that your code is too slow or you aren't kicking the dog from anywhere in your code.

Using stdio.h in embedded systems is pretty much a big no-no, since it is extremely slow and resource consuming.

Another example, if optimizations are bad or disabled, for(int i = 0; i < ( strlen(newData) ); i++) might give very slow code that should be replaced with size_t length = strlen(newData); for(int i=0; i<length; ...

And finally, if you are writing to flash, the driver needs to erase the flash first, which takes a very long time.

Answer 3

All comments and answers are highly appreciated, and I'll be considering all opinions to make my code better.

However, turns out I had mistakenly uncommented a line in my code where my final array was being copied into a smaller array. That is why my code kept crashing. The problem, as of now is solved. I will go into the time consumption and optimization of this code later, once I'm done implementing other features of my code.

Thanks All!!

Answer 4

Adding a second answer due to the edit of the OP.

First off, it's preferable to show working code. This passage suggests that this wouldn't compile:

nvs_get_str( nvsHandle, MASS_STRING_STORE, newData, &required_size);
char newData[15];

Regarding your question: your printf() output makes me question whether you're really working on string data here. If you aren't, you don't want to use nvs_get_str(), as it will stop reading the NVS after it encounters a null character. Similarly, strlen() may not work correctly if you're using it on non-string data.

If you are storing and manipulating non-string data, use nvs_get_blob() to retrieve it, and memory functions for copying it in your program.