Appending individual characters to char array and returning the char array

Question

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.

Answer 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.