0

I'm still comparatively new to C++ but I'm mostly working off of C++11 features which generally save me from memory leaks. Still, working with other older libraries, there's unfortunately times I need to interface with older code.

I've seen similar questions, but couldn't easily locate one where the array was allocated without new; eg, so it's on the stack rather than the heap, to my understanding.

This situation happens to involve a string, but the mechanics are probably more to do with arrays.

ConfigStruct {
  const char *word;
}

ConfigStruct generateConfigStruct() {
  const char myWord[] = "word"; // <-- This
  ConfigStruct cfg;
  cfg.word = myWord;
  return cfg;
}

I believe this configuration object is going to be passed into a separate lower-level function. But, I'm having a hard time working out where the memory responsibilities should be in this case. Is the low-level library the only thing that can release that string's memory? Will it get destructed with the ConfigStruct automatically? Or, will that memory be inappropriately marked as "free" at the end of this function, and possibly cause the configuration object issues later?

EDIT: One final follow-on question; if ConfigStruct cannot be changed, and my task is to write a generateConfigStruct that returns a ConfigStruct, is there any way to do it that won't cause later usage any issues, and can somehow guarantee memory safety despite the dangling pointer?

Katana314
  • 8,429
  • 2
  • 28
  • 36
  • 4
    This is bad. `cfg.word` will dangle after the function ends for the same reason as http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope – chris Jul 01 '15 at 20:41
  • @chris Uh-oh! The thing here is, I didn't write `ConfigStruct`; it's an API feature. I definitely would have just used a `std::string` or similar. I'll take it under advisement; could just be that my library has memory leaks I can't fix. – Katana314 Jul 01 '15 at 20:45
  • @chris Here the literal "word" has a constant address isn't it? (Is it on the data section?) I would say it's OK to pass this char*... I'm not sure of me but, my cent... – johan d Jul 01 '15 at 20:46
  • 3
    @johand., It's okay to set `cfg.word` to point to a literal, but it's being set to the address of the first element of the local array that has a copy of the literal. – chris Jul 01 '15 at 20:49
  • @Katana314, While unfortunate that the class doesn't use a `std::string`, you'll just have to make sure what you set `cfg.word` to point to outlives `cfg.word` (at least until it's reassigned to something else). In this example, the easiest thing would be to make it point to the literal. If it came down to it, a static variable would work, too. – chris Jul 01 '15 at 20:51
  • Question title and code do not agree. The array is not static. – paddy Jul 01 '15 at 20:51
  • @paddy I thought that might be the case; I think the new title should be correct. – Katana314 Jul 01 '15 at 20:52
  • Making the array static is actually a solution. – paddy Jul 01 '15 at 20:52
  • 1
    In this case, it's not a problem with a memory leak, but with a dangling pointer--i.e., the memory itself has been cleaned up (hasn't leaked), but the pointer to it remains, even though the pointee is gone. – Jerry Coffin Jul 01 '15 at 20:53
  • @chris Oh! Yep you're right. Thank you! – johan d Jul 01 '15 at 20:54

3 Answers3

1

myWord is indeed on the stack, so it's not statically allocated - it's automatic. It is destroyed when the function exits - which is a problem, since cfg still has a pointer to it.

EDIT to address follow-up: it would be safer to just say

cfg.word = "word";

Then you have a pointer to a constant string, which lasts the entire duration of the program.

Otherwise you're stuck - you could allocate memory and make cfg.word point to it, but then you have a leak.

Alan Stokes
  • 18,815
  • 3
  • 45
  • 64
0

You declared a local variable which will be allocated on the stack. When the function generateConfigStruct() returns, it's call frame is popped from the stack. The next method that is called will have a call frame then be pushed on the stack and can overwrite the data in that location.

You do not want to return pointers to stack-allocated storage.

dsh
  • 12,037
  • 3
  • 33
  • 51
0

But, I'm having a hard time working out where the memory responsibilities should be in this case. Is the low-level library the only thing that can release that string's memory?

In your posted code, memory for the string literal "word" is managed by the run time library. The string literal lives in the read-only portion of the program. That memory is valid for the life time of the program. However, you also have a stack variable myWord. It holds a copy of "word". Memory used by myWord is valid while the function is being executed and becomes invalid when the function returns. By using:

cfg.word = myWord;
return cfg;

you are returning an object that holds on to a pointer that is not valid.

Will it get destructed with the ConfigStruct automatically?

Whether the destructor of ConfigStruct tries to deallocate the memory for its member veriable word is up to the definition of ConfigStruct. If it does not have a user defined destructor, then the compiler generates one but the compiler generated destructor will not deallocate memory used by word.

Or, will that memory be inappropriately marked as "free" at the end of this function, and possibly cause the configuration object issues later?

That is correct. If you use word member variable of the returned ConfigStruct object later, you will run into problems since that is a dangling pointer.

If ConfigStruct does not have a user defined destructor, you can use:

ConfigStruct generateConfigStruct() {
  static char myWord[] = "word";
  ConfigStruct cfg;
  cfg.word = myWord;
  return cfg;
}

or

ConfigStruct generateConfigStruct() {
  ConfigStruct cfg;
  cfg.word = "word";
  return cfg;
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270