0

I am once again having a lot of difficulties with chars and char arrays in my Arduino program. I use ESP32 device with SPIFFS where I save some data which I want to save into my ESP32 device. In my SPIFFS I have a wifi.txt file where I save wifi name. I then call function:

char* saved_networks; // create char array to save information from readfile
saved_networks = readFile(SPIFFS, "/wifi.txt");

readFile function is described here:

char* readFile(fs::FS &fs, const char *path)
{
    char* return_message; //create a variable return_message to hold the appended char data
    Serial.printf("Reading file: %s\n", path);

    File file = fs.open(path);
    if (!file || file.isDirectory())
    {
        Serial.println("Failed to open file for reading");
        return "FAIL";
    }

    Serial.print("Read from file: ");
    while (file.available())
    {
        //Serial.write(file.read());
        char c =  file.read(); // save one character at a time and save it to a temporaray variable c
        delayMicroseconds(100);
        Serial.print(c); // this prints the wifi name as expected so everything is working ok up to this point
        strcat(return_message, &c); // append char array (return_message)

    }
    Serial.print("printing final return message before closing file system=");
    Serial.println(return_message); //prints loads of garbage
    file.close();
    return return_message;
}

After printing the return_message, ESP32 device crashes and returns an error message:

Reading file: /wifi.txt
Read from file: Telia-33F8A3-Greitas
printing final return message before closing file sysetm=x⸮?⸮⸮?⸮⸮?T⸮    @?e⸮    @?l⸮    @?i⸮    @?a⸮    @?-⸮    @?3⸮    @?3⸮    @?F⸮    @?8⸮    @?A⸮    @?3⸮    @?-⸮    @?G⸮    @?r⸮    @?e⸮    @?i⸮    @?t⸮    @?a⸮


Stack smashing protect failure!

abort() was called at PC 0x401377bf on core 1

ELF file SHA256: 0000000000000000

Backtrace: 0x40088740:0x3ffb1480 0x400889bd:0x3ffb14a0 0x401377bf:0x3ffb14c0 0x400d1685:0x3ffb14e0 0x400d1872:0x3ffb1590 0x400d18b3:0x3ffb1f80 0x400d4cfe:0x3ffb1fb0 0x400899ce:0x3ffb1fd0

Rebooting...
ets Jun  8 2016 00:22:57

rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:1
load:0x3fff0018,len:4
load:0x3fff001c,len:1216
ho 0 tail 12 room 4
load:0x40078000,len:10944
load:0x40080400,len:6388
entry 0x400806b4

Any help is highly appreciated.

UPDATE1

Is this also the same when passing the char array to a void function by reference?:

If I declare my readFile function as following :

void readFile(fs::FS &fs, const char *path, char* return_data)
{
    Serial.printf("Reading file: %s\n", path);

    File file = fs.open(path);
    if (!file || file.isDirectory())
    {
        Serial.println("Failed to open file for reading");
        return;
    }

    Serial.print("Read from file: ");
    while (file.available())
    {
        //Serial.write(file.read());
        char c =  file.read();
        delayMicroseconds(100);
        Serial.print(c);
// Do I also must have allocated memory in order to append characters to my return_data char array

    }
    Serial.print("printing final return message before closing file sysetm=");
    Serial.println(return_data);
    file.close();

}

And then in my main, I call:

    char* saved_networks = NULL; // create char array to save information from readfile
    readFile(SPIFFS, "/wifi.txt",&saved_networks[0]);

UPDATE2

I have managed to do it by passing a char array as a reference. This method does not require malloc nor free.

void readFile123(fs::FS &fs, const char *path, char* return_data)
{
   
    int n=0;
    Serial.printf("Reading file: %s\n", path);

    File file = fs.open(path);
    if (!file || file.isDirectory())
    {
        Serial.println("Failed to open file for reading");
        return;
    }
    Serial.print("Read from file: ");
    while (file.available())
    {
        char c =  file.read();
        delayMicroseconds(100);
        Serial.print(c);
        //strcat(return_data, &c); //returns meditation error
        return_data[n]=c; //returns meditation error
        n=n+1;
    }
    file.close();

}

and main code:

    char saved_networks[100];
    readFile123(SPIFFS, "/wifi.txt",saved_networks);
    Serial.print("saved networks=");
    Serial.println(saved_networks);

This way of implementation makes much more sense to me. Why would I ever want to use malloc if I can pass an array by reference? This is way more simple to implement.

However, I still have one more concern. As you can see from the code above, I have initialized maximum length of my char array to be 100, however, I am not sure what will be the size of my SPIFFS contents. Is there any way to overcome this issue? I know I can just declare the size for example 10000, and hope that the contents will never get that big, but that does not seem like a most efficient way of going about this issue. I hope someone can comment on that. Thanks in advance.

1 Answer 1

1

Here you append c to an uninitalized pointer:

strcat(return_message, &c); // append char array (return_message)

Since return_message is not initialized, this invokes undefined behavior, because stract expects a 0 terminated C string. So you have to allocate memory for the message, as well as terminating it with a 0-byte.

You must allocate memory to append it.

char* return_message = nullptr;
size_t message_len = 0;
...
while (file.available())
{
    //Serial.write(file.read());
    char c =  file.read(); // save one character at a time and save it to a temporaray variable c
    delayMicroseconds(100);
    Serial.print(c); // this prints the wifi name as expected so everything is working ok up to this point

    return_message = realloc(return_message, len+1);
    return_message[len++] = c;
}
return_message = realloc(return_message, len+1);
return_message[len++] = 0;

Or if you know the size of the expected message:

size_t max_message = 100;
char* return_message = malloc(max_message);
size_t message_len = 0;
*return_message = 0;
...
while (file.available())
{
    //Serial.write(file.read());
    char c =  file.read(); // save one character at a time and save it to a temporaray variable c
    delayMicroseconds(100);
    Serial.print(c); // this prints the wifi name as expected so everything is working ok up to this point

    return_message[len++] = c;
    if (len == max_message-1)
        break;
}
return_message[len++] = 0;

You also should not return a string literal with a non-const pointer:

if (!file || file.isDirectory())
{
    Serial.println("Failed to open file for reading");
    free(return_message);
    return strdup("FAIL");
}

The caller of this function must free the returned string, when it is done with it, otherwise you create a memory leak.

Sign up to request clarification or add additional context in comments.

14 Comments

Thanks. Can you explain what is the purpose of this code : return_message = realloc(return_message, len+1);
And also, would that still be the same if instead of creating a char pointer return_message I create a char return_message[100]. I would then create a char array with a maximum size of 100, maybe I then can avoid using a allocation?
If you know the size of the message or limit it to a known value, then it is more efficient than using realloc like I did, which is the naive approach, but not that efficient.
Yes that works indeed.. Could you please explain on a little lower level what exactly is the problem with the way I was trying to do it initially and why we need to manually allocate memory. Do I also must free the memory at some point at the end of this function ? If I free the memory, I wont be able to return the string, will I?
I updated the answer with additional info.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.