How to return a const char * from a C/C++ function?

I have the following C++ function that reads a string from the flash memory and returns it. I want to avoid using the String class because this is on an Arduino and I've been advised that the String class for the Arduino is buggy.

const char* readFromFlashMemory()
{
    char s[FLASH_STOP_ADDR-FLASH_START_ADDR+2];
    memset(s, (byte)'\0', FLASH_STOP_ADDR-FLASH_START_ADDR+2);
    unsigned short int numChars = 0;

    for (int i = FLASH_START_ADDRESS; i <= FLASH_STOP_ADDRESS; i++)
    {
        byte b = EEPROM.read(i);
        if (b == (byte)'\0')
            return s;
        else
            s[numChars++] = (char)b;
    }
}

This function seems to work. But the calling method gets back an empty string. Am I not allowed to return a pointer to a character array that is on this function's stack? How is the best/most idiomatic way I should write this function so the calling function receives the value that I want to pass to it?

Solution

The comments are probably going to lead you astray or at best confuse you. Let me break it down with some options.

Firstly, the problem is as you say: the array whose address you are returning no longer exists when the function is popped off the stack to the caller. Ignoring this results in Undefined Behavior.

Here are a few options, along with some discussion:

The caller owns the buffer
```
void readFromFlashMemory(char *s, size_t len)
```
Advantage is that the caller chooses how this memory is allocated, which is useful in embedded environments.

Note you could also choose to return s from this function as a convenience, or to convey some extra meaning.

For me, if I was working in an embedded environment such as Arduino, this would be my preference 100%.
Use std::string, std::vector or similar
```
std::string readFromFlashMemory()
```
This is probably the way you'd do it if you didn't care about allocation overhead and other potential issues such as fragmentation over time.
Allocate memory yourself
```
char* readFromFlashMemory()
```
If you want to ensure the allocated memory is exactly the right size, then you'd probably read into a local buffer first, and then allocate the memory and copy. Same memory considerations as std::string or other solutions dealing with heap memory.

This form also has the nightmarish property of the caller being responsible for managing the returned pointer and eventually calling delete[]. It's highly inadvisable. It's also distressingly common. :(
Better way to return dynamically allocated memory, if you absolutely must
```
std::unique_ptr<char[]> readFromFlashMemory()
```
Same as #3, but the pointer is managed safely. Requires C++11 or later.
Use a static buffer
```
const char* readFromFlashMemory()
{
    static char s[FLASH_STOP_ADDR-FLASH_START_ADDR+2];
    // ...
    return s;
}
```
Generally frowned upon. Particularly because this type of pattern results in nasty problems in multi-threaded environments.

People mostly choose this approach because they're lazy and want a simple interface. I guess one benefit is that the caller doesn't have to know anything about what size buffer is acceptable.
Make your own class with an internal buffer
```
class FlashReader
{
public:
    const char* Read();
private:
    char buffer[FLASH_STOP_ADDR-FLASH_START_ADDR+2];
};
```
This is more of a verbose solution, and may start to smell like over-engineering. But it does combine the best parts of both #1 and #5. That is, you get stack allocation of your buffer, you don't need to know the size, and the function itself doesn't need extra arguments.

If you did want to have a static buffer, then you could just define one static instance of the class, but the difference would be a clear intent of this in the code.