-2

Is there another efficient way to write code below not using static string variable? The reason is that, I use the code below to illustrate the crash happening in a bigger project that uses this static string variable. But if I remove the static keyword, the code will not crash, but the contents of string variable are nothing. 

std::string conversation;

const char *GetFoo()
{
   static std::string word;

   word ="hello ";
   word +="buddy.";
   word +="  How are things?";
   return word.c_str();
}

void CallGetFoo()
{
   const char *pp = GetFoo();
   conversation +=pp;

    cout<<pp;
}


int _tmain(int argc, _TCHAR* argv[])
{
   CallGetFoo();
   return 0;
}
Phillip C
  • 97
  • 1
  • 12
  • Why not return `std::string` – Ed Heal May 05 '15 at 20:36
  • Removing the `static` is a bug. You get undefined behaviour. – juanchopanza May 05 '15 at 20:38
  • ok thanks Ed Heal and Juanchopanza. I will try to return std::string instead. But there is another thing too. This project has been working for number of years. It only acts up on the new platform. – Phillip C May 05 '15 at 20:55
  • It has been "working", with or without the `static`? – juanchopanza May 05 '15 at 21:11
  • it had been working with static until now it crashes on the new device. – Phillip C May 05 '15 at 21:22
  • Interesting, because you selected an answer that tells you why it doesn't work *without* static. – juanchopanza May 05 '15 at 21:33
  • I strongly recommend avoiding this kind of pattern of returning references/pointers to statics inside the function whenever you can. They wreck unnecessary havoc on thread safety and can cause really confusing behavior if the client tries to capture a shallow copy to the return value and then proceeds to call the function a second time. I unfortunately worked at a place with a legacy codebase that did this type of stuff everywhere. –  May 05 '15 at 23:40
  • thanks lke for your recommendation. – Phillip C May 06 '15 at 21:55

1 Answers1

3

You're experiencing the classic issue of returning a pointer/reference to data that is local to a function. When you remove the static keyword, then the word variable is destructed at the end of the function. That means the c_str that it returns is going to be garbage and you'll end up with undefined behavior. The static keyword keeps the object around so that it will remain the same through multiple calls to the function. Like the comments say, you're better off returning a std::string.

Returning a std::string will copy the contents of the local variable to the caller's std::string. More than likely, the compiler will be able to optimize out the copy and do something called Return Value Optimization (RVO), but that is a separate topic.

Scott
  • 370
  • 2
  • 7
  • We return std::String instead because... finish the thought so OP understands why that is better than returning a local var. – Michael Dorgan May 05 '15 at 20:42
  • Thanks Scott and Michael.. I will try to return std::string. But on a side question, this code has been working for many years, but it crashes on new device of that statement so do you know what are potential issues can cause that? – Phillip C May 05 '15 at 21:00
  • When you enter the world of undefined behavior, it's really hard to determine exactly why things are working the way they are. It could be that the particular compiler or platform you were using before didn't do anything with the character array that was allocated for the string after it was destructed and the new one does. It's all platform/compiler speculation when you get into these situations. – Scott May 05 '15 at 21:04
  • thanks. Our senior engineer suggested to contact OS team of what they did. – Phillip C May 05 '15 at 21:17
  • @PhillipC `Our senior engineer suggested to contact OS team of what they did` Not sure what that really means, but the issue is not OS related. Your code had a bug that existed, and now it became exposed. `this code has been working for many years` Shrug -- when you make mistakes like this, code could run for years without error, until you change something, run it differently, run it on a different machine, build the program with different compiler options, etc... – PaulMcKenzie May 05 '15 at 21:23
  • Thanks Paul. I just talked to the previous owner of the code. According to him, there must be a memory corruption somewhere and suggested me to go back to review the changes that were made to the project for the last 6 months. But deadline is near so I just remove the static keyword from variable declarations then return value as std::string seem to fix the crash. – Phillip C May 06 '15 at 22:03
  • @PhillipC If returning a std::string is working for you, do bother trying to determine why the previous code worked. The old static string was being replaced with every call to the function. Only maybe the compiler and std::string implementer might know how that would affect any reference to the static instances c_str() pointer. The new code now gets its own copy which is the correct implementation for functions like these. – Scott May 06 '15 at 22:21