ESP32 esp-idf SPIFFS strcat problem

Thread Starter

zazas321

Joined Nov 29, 2015
936
Hello. I am using SPIFFS for my esp32 using esp-idf. See the function below that opens the file inside my SPIFFS:

Code:
static void read_hello_txt(char* file_name)
{
    ESP_LOGI(TAG, "Reading hello.txt");
    // Open for reading hello.txt
    char file_location[30];
    memset(file_location, 0, sizeof(file_location));
    strcat(file_location,"/spiffs/");
    strcat(file_location,file_name);
    printf("file location = %s \n",file_location);
    printf("size of string1 = %d \n ",strlen("/spiffs/hello.txt"));
    printf("size of string2 = %d \n ",strlen(file_location));

    //FILE* f = fopen("/spiffs/hello.txt", "r"); // works without any issues

    FILE* f = fopen(file_name, "r"); // does not work
    if (f == NULL) {
        ESP_LOGE(TAG, "Failed to open hello.txt");
        return;
    }
    char buf[64];
    memset(buf, 0, sizeof(buf));
    fread(buf, 1, sizeof(buf), f);
    fclose(f);
    // Display the read contents from the file
    ESP_LOGI(TAG, "Read from hello.txt: %s", buf);
}
I would like to know why the following works without any issues:
Code:
    //FILE* f = fopen("/spiffs/hello.txt", "r");
And why this does not work:
Code:
    char file_location[30];
    memset(file_location, 0, sizeof(file_location));
    strcat(file_location,"/spiffs/");
    strcat(file_location,file_name);
    FILE* f = fopen(file_name, "r");
I do not want to hardcode the name of the file. I want to pass the name of the file and then open it. I cannot understand why it does not work because I create exactly same string as if I hardcode it. The serial monitor:

Code:
file location = /spiffs/hello.txt
size of string1 = 17
 size of string2 = 17
 E (365) example: Failed to open hello.txt
 

click_here

Joined Sep 22, 2020
548
You are adding "/spiffs/" to file_location, then file_name to file_location, and then trying to open file_name instead of file_location

Code:
FILE* f = fopen(file_name, "r");

// should be

FILE* f = fopen(file_location, "r");
 

click_here

Joined Sep 22, 2020
548
Also the reason lines 3 and 4 are off
Code:
file location = /spiffs/hello.txt
size of string1 = 17
 size of string2 = 17
 E (365) example: Failed to open hello.txt
... is because there is a space in the previous printfs after the newline charactors
Code:
printf("size of string1 = %d \n ",strlen("/spiffs/hello.txt"));
// should be
printf("size of string1 = %d \n",strlen("/spiffs/hello.txt"));
 

click_here

Joined Sep 22, 2020
548
Other tips:
Code:
char file_location[30];
memset(file_location, 0, sizeof(file_location));
strcat(file_location,"/spiffs/");

// Can be

char file_location[30] = "/spiffs/";
You might be wondering about the left over charactors in file_location: The C standards say that if the initialisation of an array has left over elements, they will be initialised as a static type - which is 0. (C99: 6.7.8-14 / 6.7.8-10 / 6.7.8-19)

That is why you see this a lot:
Code:
char someArray[BUFSIZ] = {0};
... they have initialised the first element as 0, and the rest automatically get initialised to 0.

Note that this behaviour is only present at initialisation.

Also, because 'buf' and 'file_location' are never used at the same time, consider using the same memory - You can reuse the buffer, or declare a union of the two so they overlap.

You have an overflow bug as well - If file_name is larger than
Code:
30 - sizeof('\0') - strlen("/spiffs/")
and you try to put that into file_location, you will have a buffer overflow.

To get around this use strncat instead of strcat.
 
Last edited:

click_here

Joined Sep 22, 2020
548
Also because you are using fread with a text file (which I am assuming you are because you opened the file as "r" instead of "rb"), you have to make sure that there is a '\0' at the end of the array, because it doesn't do it automatically.

You should be reading sizeof(buf) - 1 so that you have at least one 0 at the end (otherwise you will have another overflow when you call ESP_LOGI() ).

The best way I find to read a text file is to use fgets. It is very safe from overflows, but you'll have to read one line at a time and remove the newline charactor (which is kindergarten stuff)
 

Thread Starter

zazas321

Joined Nov 29, 2015
936
Thank you very much for pointing all those errors out. Can you explain a little bit more about initialising char arrays and proper terminations? From what I understand, when I initialise a variable
char file_location[30];
, it reserves 30 bytes of memory in the stack. This memory could be '0' but not guaranteed. So I can ensure that it has '0' with the following:
Code:
memset(file_location, 0, sizeof(file_location));
or as you suggested
Code:
char someArray[BUFSIZ] = {0};


Also, I did not fully understand what you meant reading sizeof(buf) - 1 . How can I ensure that in my .txt file I have a null termination? And how does using sizeof(buf)-1 ensures that there is a null termination? In my code I have the following:

Code:
    char buf[64];
    memset(buf, 0, sizeof(buf));
    fread(buf, 1, sizeof(buf), f);
    fclose(f);
    ESP_LOGI(TAG, "Read from hello.txt: %s", buf);
 

click_here

Joined Sep 22, 2020
548
A string is usually (in English) a char array of charactors from the ASCII table.

the best way to explain why you need sizeof(buf) - 1 is to look at the "puts()" function.

If I use puts() it basically works like this...
Code:
char ch[] = "some string";
...
int i=0;
while(ch[i] != '\0')
{
    putchar(ch);
    i++;
}
So if I have a string "/spiffs/" it is the same as declaring it as...
Code:
char myString[30] = {'/', 's', 'p', 'i', 'f', 'f', 's', '/', '\0'};
Notice that there is a '\0' at the end. (In the ASCII format '\0'==0)

This is important because it tells puts() when to stop printing.

All functions with strings work in this way - the strings need to finish with a '\0' so that the functions knows when to stop.

Looking at the fread function, imagine if I had an array of 3 chars and I read 3 bytes - where would the '\0' go? I would have to read 2 bytes so that I had room to put the '\0' at the end.

Leaving that one byte at the end is why you need to only read sizeof(buf) - 1 bytes.


From what I understand, when I initialise a variable
char file_location[30];
, it reserves 30 bytes of memory in the stack. This memory could be '0' but not guaranteed
That is true if its storage is automatic, but if it is static it actually gets initialised to 0. You can manually declare it as static, or you can have it as a global variable.

When you initialise an array of chars (if you do choose to initialise the array) the chars that don't initialise will be automatically initialised the same as a static type - For a char this is 0 (which happens to be ASCII '\0').

That is why this works...
Code:
char apple[512] = {0}; // All 0
but not this...
Code:
char apple[512];  // Could be anything
 

Thread Starter

zazas321

Joined Nov 29, 2015
936
Thank you very much for taking your time to explain this. I have a few more questions regarding the static vs automatic declaration

For example if I declare my char array as static, does this guarantee that it will be '0's?
Code:
static char apple[512] // all 512 bytes will be 0?
automatic declaration:
Code:
char apple[512] // not guaranteed to be 0's
However, if I declare my char apple[512] as a global variable in my main.c. Is it guaranteed to have 0's in the memory even though I do not specify that?

declared as global in main.c :
Code:
char apple[512]  = {0}; // guaranteed to have 0 in memory
vs
Code:
char apple[512]; // is this guaranteed to have 0 or not if its global?
 

click_here

Joined Sep 22, 2020
548
First, in the standards array and structure types are collectively called "aggregate" types - This doesn't include unions

From C99...

6.7.8 Initialization
10 - If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. If an object that has static storage duration is not initialized explicitly,
then:
— if it has pointer type, it is initialized to a null pointer;
— if it has arithmetic type, it is initialized to (positive or unsigned) zero;
— if it is an aggregate, every member is initialized (recursively) according to these rules;
— if it is a union, the first named member is initialized (recursively) according to these
rules.

"Static allocation happens when you declare a static or global variable. The space is allocated once, when your program is started and remain reserved till the end of the program" (https://medium.com/@connect.ajkumar/memory-allocation-in-c-d48f2974e0c)

Global variables are static and the variables that you specify to be static.
Code:
char MyStaticGlobalVariable[BUBSIZ];

int main(void)
{
    static char myLocalStaticVariable[BUBSIZ];
    char myInitialisedLocalAutoVariable[BUBSIZ] = {0};
    char myLocalAutoVariable[BUBSIZ];

    ...
   
    return 0;
}
... only "myLocalAutoVariable" will have indeterminate values.

For access to the C standards look here


Does that help clear it up?
 
Top