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

Question

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?

There's a ton of questions like this on Stack Overflow. Basically "why can't I return a pointer to a local variable" or "how to get around said issue". If you just want to read into a buffer, the standard way is to pass the buffer and its size into the function that is going to write to it. — paddy, Apr 02 '20 at 07:18
The idiomatic way is to use something other then a `char *`. Something with value semantics so you can return by value, like `std::string`. The ugly solution is to use `new` to allocate the array and require the caller to free it once it's been used. — super, Apr 02 '20 at 07:18
I do not see a return statement in case that the `for` never iterates or never uses the ´then`. What do you expect/intend the function to do in these cases? — Yunnosch, Apr 02 '20 at 07:20
Since we're also considering ugly solutions, you may consider just declaring `s` as static. In a small program that is not going to execute in a multi-threaded environment, this will work. It was good enough for various C library functions way back... Although we all regret that. — paddy, Apr 02 '20 at 07:21
You are creating the array on the stack, you want it on the heap: `char* s= new char[PIN_EPROM_STOP-PIN_EPROM_START+2];`. However, note that this means you need to control life time. If you don't want to use `std::string` for whatever reason, consider writing your own wrapper. — Aziuth, Apr 02 '20 at 07:25
An option slightly less ugly than using `malloc` is to simply return `strdup(s)` (assuming that `s` is a properly null-terminated string), as described [here](https://stackoverflow.com/a/60922107/10871073). — Adrian Mole, Apr 02 '20 at 07:33
_"This function seems to work."_ contradicts _"But the calling method gets back an empty string."_ — Jabberwocky, Apr 02 '20 at 07:34

score 7 · Accepted Answer · answered Apr 02 '20 at 07:50

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.

5b) `static thread_local char s[FLASH_STOP_ADDR-FLASH_START_ADDR+2];` :-) — Ted Lyngmo, Apr 02 '20 at 15:05

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

1 Answers1