How to return a std::string.c_str()

Question

I have a method which returns the constant char pointer. It makes use of a std::string and finally returns its c_str() char pointer.

const char * returnCharPtr()
{
    std::string someString;

    // some processing!.

    return someString.c_str();
}

I have got a report from COVERITY tool that the above is not a good usage. I have googled and have found that the char pointer returned, would be invalidated as soon as someString meets its destruction.

Given this, how does one fix this issue? How to return char pointer accurately?

Returning std::string would resolve this issue. But I want to know if there is any other means of doing this.

Solution 1

What happens in this code is:

const char * returnCharPtr()
{
    std::string someString("something");
    return someString.c_str();
}

1. instance of std::string is created - it is an object with automatic storage duration
2. pointer to the internal memory of this string is returned
3. object someString is destructed and the its internal memory is cleaned up
4. caller of this function receives dangling pointer (invalid pointer) which yields undefined behavior

The best solution is to return an object:

std::string returnString()
{
    std::string someString("something");
    return someString;
}

When calling your function, DO NOT do this:

const char *returnedString = returnString().c_str();

because returnedString will still be dangling after the returned std::string is destructed. Instead store the entire std::string:

std::string returnedString = returnString();
// ... use returnedString.c_str() later ...

Solution 2

In C++, the simplest thing to do is to just returna std::string (which is also efficient thanks to optimizations like RVO and C++11 move semantics):

std::string returnSomeString()
{
    std::string someString;

    // some processing...

    return someString;
}

If you really need a raw C char* pointer, you can always call .c_str() on the returned value, e.g.

// void SomeLegacyFunction(const char * psz)

// .c_str() called on the returned string, to get the 'const char*' 
SomeLegacyFunction( returnSomeString().c_str() );

If you really want to return a char* pointer from the function, you can dynamically allocate string memory on the heap (e.g. using new[]), and return a pointer to that:

// NOTE: The caller owns the returned pointer, 
// and must free the string using delete[] !!!
const char* returnSomeString()
{
    std::string someString;

    // some processing...

    // Dynamically allocate memory for the returned string
    char* ptr = new char[someString.size() + 1]; // +1 for terminating NUL

    // Copy source string in dynamically allocated string buffer
    strcpy(ptr, someString.c_str());

    // Return the pointer to the dynamically allocated buffer
    return ptr;
}

An alternative is to provide a destination buffer pointer and the buffer size (to avoid buffer overruns!) as function parameters:

void returnSomeString(char* destination, size_t destinationSize)
{
    std::string someString;

    // some processing...

    // Copy string to destination buffer.
    // Use some safe string copy function to avoid buffer overruns.
    strcpy_s(destination, destinationSize, someString.c_str());
}

Solution 3

As this question is flagged C, do this:

#define _POSIX_C_SOURCE 200809L
#include <string.h>

const char * returnCharPtr()
{
  std::string someString;

  // some processing!.

  return strdup(someString.c_str()); /* Dynamically create a copy on the heap. */
}

Do not forget to free() what the function returned if of no use anymore.

Solution 4

Well, COVERITY is correct. The reason your current approach will fail is because the instance of std::string you created inside the function will only be valid for as long as that function is running. Once your program leaves the function's scope, std::string's destructor will be called and that will be the end of your string.

But if what you want is a C-string, how about...

const char * returnCharPtr()
{
    std::string someString;

    // some processing!.

    char * new_string = new char[someString.length() + 1];

    std::strcpy(new:string, someString.c_str());

    return new_string;
}

But wait... that's almost exactly as returning a std::string, isn't it?

std::string returnCharPtr()
{
    std::string someString;

    // some processing!.

    return new_string;
}

This will copy your string to a new one outside of the function's scope. It works, but it does create a new copy of the string.

Thanks to Return Value Optimization, this won't create a copy (thanks for all corrections!).

So, another option is to pass the parameter as an argument, so you process your string in a function but don't create a new copy. :

void returnCharPtr(std::string & someString)
{
    // some processing!.
}

Or, again, if you want C-Strings, you need to watch out for the length of your string:

void returnCharPtr(char*& someString, int n) // a reference to pointer, params by ref
{
    // some processing!.
}

Solution 5

A solution which hasn't been evoked in the other answers.

In case your method is a member of a class, like so:

class A {
public:
    const char *method();
};

And if the class instance will live beyond the usefulness of the pointer, you can do:

class A {
public: 
    const char *method() {
        string ret = "abc";
        cache.push_back(std::move(ret));
        return cache.last().c_str();
    }
private:
    vector<string> cache; //std::deque would be more appropriate but is less known
}

That way the pointers will be valid up till A's destruction.

If the function isn't part of a class, it still can use a class to store the data (like a static variable of the function or an external class instance that can be globally referenced, or even a static member of a class). Mechanisms can be done to delete data after some time, in order to not keep it forever.

View more solutions

Comments

[juanchopanza]

But why const? Now it can't be moved.

[Praetorian]

This assumes the caller knows how long the string is going to be, which is most often not the case.

[chris]

Don't return an rvalue reference. It has the same problem as an lvalue reference. (N)RVO takes care of expensive return copying even before C++11, and in C++11, the object will be moved out automatically if it can and (N)RVO doesn't work.

[LihO]

@juanchopanza: Well, it depends on how it's going to be used. But yeah, I admit that simple std::string will do better + it will be more flexible too.

[R. Martinho Fernandes]

You just committed the same crime you accused the OP of! </joke> Rvalue references are still references, and returning one doesn't change the fact that it's still a reference to a local variable.

[Praetorian]

To add to what chris said, the code where you return an rvalue reference won't even compile as written, you need to return move(new_string); (and then you get to deal with a dangling reference). And your C-string example doesn't make sense at all; the function is taking a pointer to const when the intent is to operate on the input string? Also, that signature assumes the caller knows the length of the result.

[ArthurChamz]

Oh, my... I don't deserve to live D: I got it all backwards!

[stefaanv]

1 more correction: the length of new_string in your first example is 1 short (nul-terminator)

[Yakk - Adam Nevraumont]

"This will copy your string to a new one outside of the function's scope. It works, but it does create a new copy of the string." -- no, that does not copy the string, it quite efficiently moves the string (actually, it doesn't even move it: the move is elided by the code, but that is a topic for a different post)

[Yakk - Adam Nevraumont]

I don't see the C flag, has it changed?

[alk]

@Yakk: The original posting (stackoverflow.com/revisions/22330250/1) carried the C tag.

[Spock77]

@ArthurChamz Here return CharPtr(char * someString, int n) you really cannot return pointer as it is a local parameter and the pointer is a copy of passed value. I suggest: return CharPtr(char*& someString, int n)

[Rapnar]

I have a situation where returnString().c_str() == 0 (returned string is "m") but if I save the return value then call c_str() on the temp it works. Ideas?

[marsh]

It is worth noting that the second example is probably not a very good idea. The caller is not going to expect that they have to delete that pointer and will most likely result in a memory leak.

[siwmas]

99% of the cases you should return std::string, but the most voted answer should cover the case where char* is actually needed as return type (this is what the question asks anyway). Mr.C64 answer looks more complete to me.

[siwmas]

@marsh It is the caller's responsibility to check if he owns the returned pointer.

[Tommaso Thea Cioni]

Thanks, I lost all my afternoon after a bug caused by something somewhat similar, but unfortunately more complicated. Anyway, thanks a lot.

[Yuval]

What about the case when one would like to override the what() virtual function from std::exception , virtual const char* what()const throw() override; if one would like to return anything that isn't a literal string, i.e returning some extra relavent run time information string, char* would be needed. The only solution I seem to think about is making a static std::string and then c_str() wouldn't be returned as a dangling pointer, but it seems as a too ugly of a solution, and frankly I hate the idea of static life duration for a string that only needs to be printed once.

[Mr.C64]

@FabioTurati Thanks. Sure, I meant string size, not pointer. Fixed.

[Yakov Galka]

@Yuval cannot make it static because that won't be thread-safe. The canonical solution is to have a regular std::string in your exception class, fill it out, and return c_str() of that. It will stay alive as long as your exception object is, which is conforming to the what() interface.

[HolyBlackCat]

Returning by const value almost never makes sense. You should return by const reference, or at least by non-const value (to allow move semantics).

[VojtaK]

I have to implement this method from standard library cplusplus.com/reference/exception/exception/what and thus I can nor really choose its return type.

[HolyBlackCat]

Returning a const pointer is fine. I was talking about const std::string get_msg().

[VojtaK]

Thank you, I have eddied the answer and changed it in my repository accordingly. Returning const object by non const value would probably produce compiler warning/error and I just wanted a quick fix, but const reference is obviously better solution.

[HolyBlackCat]

"Returning const object by non const value would probably produce compiler warning/error" No, it's perfectly fine and recommended.

[John]

@alk The caller has to free the memory is not a good idea. :(