views:

233

answers:

4

This is from a small library that I found online:

 const char* GetHandStateBrief(const PostFlopState* state)
{
    static std::ostringstream out;

... rest of the function ...

 return out.str().c_str()

Now in my code I am doing this:

const char *d = GetHandStateBrief(&post);
std::cout<< d << std::endl;

Now, at first d contained garbage. I then realized that the c string I am getting from the function is destroyed when the function returns because std::ostringstream is allocated on the stack. So I added:

return strdup( out.str().c_str());

And now I can get the text I need from the function.

I have two questions:

1) Am I understanding this correctly?

2) I later noticed that the ostringstream was was allocated with static storage. Doesn't that mean that the object is supposed to stay in memory until the program terminates? and if so , then why can't I access the string?

+7  A: 

strdup allocates a copy of the string on the heap, which you have to free manually later (with free() I think). If you have the option, it would be much better to return std::string.

The static storage of out doesn't help, because .str() returns a temporary std::string, which is destroyed when the function exits.

Marcelo Cantos
A: 

In GetHandStateBrief, variable out does not need to be static. You need an explicit static string to replace the temporary that was being created in your original call to out.str():

static std::string outStr;
std::ostringstream out;
... rest of function ...
outStr = out.str();
return outStr.c_str();
sean e
This is risky. The returned `char*` isn't guaranteed to be valid after a subsequent call to `GetHandStateBrief`.
Marcelo Cantos
True that every call to `GetHandStateBrief` will invalidate the pointer returned by the previous call. The risk is context dependent though.
sean e
You're right ... sorry, wasn't reading that right.
Brian Roach
downvote for the risk of shooting yourself in the foot?
sean e
@sean (re: downvote): I guess so, but not from me.
Marcelo Cantos
No downvote from me either, but the 'static variable in the function so we can return a pointer instead of just returning the object' is a a bit of a well-known antipattern in C++. See Scott Meyer's Effective C++ 2nd edition, Item 23.
Timo Geusch
I don't think this is no worse that the original code (except this one works). strdup in a C++ program isn't necessarily any better.
UncleBens
All true - but I'll leave it as this succeeds at what he was trying to do. I've had to use a similar anti-pattern across dlls when the CRT is not shared. OTOH, I agree that this should not be practiced if the object can be returned instead.
sean e
A: 

strdup() returns a char* pointer that is pointing to memory on the heap. You need to free() it when you're done with it, but yes, that will work.

The static local variable std::ostringstream out makes no sense in this case, unless the std::string being returned was also static which your observation is showing to be not true.

Brian Roach
+1  A: 

You're right that out is a static variable allocated on the data segment. But out.str() is a temporary allocated on the stack. So when you do return out.str().c_str() you're returning a pointer to a stack temporary's internal data. Note that even if a string is not a stack variable, c_str is "only granted to remain unchanged until the next call to a non-constant member function of the string object."

I think you've hit on a reasonable workaround, assuming you can't just return a string.

Matthew Flaschen
Hmm, "static variable allocated on the heap" - never heard of such thing :)
Nikolai N Fetissov
Thanks for catching that.
Matthew Flaschen
Well, the character data of the string is indeed stored on the heap with any std::string, whether static or whatever. It's the string descriptor stored on the data segment like other variables with global lifetime.
Ben Voigt