Assigning to dereferenced string -- can this cause a memory leak?

Question

New to C++ here. Can the following cause a memory leak?

// debug is optional, for debugging purposes.
// Say my_string is an internal string class my company uses.

int DoSomething(my_string *debug) {
  const my_string& s = GetString();
  if (debug != nullptr) *debug = s;  // Could this cause a memory leak?
  return DoSomethingElse(s);
}

I wanted to check my understanding here: I think at the commented line the region of memory pointed to by debug will just get overwritten with (a copy of) the contents of s, unless there is a copy-constructor implemented for my_string, in which case it might do some other stuff too.

If there is not a special copy-constructor for my_string, then if my_string points to any dynamically allocated memory in its internal representation (which it probably does since it can hold strings of any length), then the above code would cause memory leaks.

Also, another question about expected C++ "etiquette" I guess — should I be able to assume that whoever wrote my_string wrote a copy-constructor to avoid memory leaks in cases like this? That is, since I don't have any new in my own code, would it be reasonable to say it wasn't my "fault" if this code causes memory leaks?

EDIT: I think maybe I meant copy assignment operator above instead of copy constructor.

Answer 1

This can in theory be a memory leak. However, whenever you're writing a class that internally manages any kind of resource (and we often do), always respect The Rule of Five and RAII. What I mean by this is, if the person writing the my_string class was sane, they explicitly defined the copy operator so the leak never happens.

Also, if you don't fully understand the difference between copy constructors and copy operators, this question might be of help.

Answer 2

The answer entirely depends on what my_string operator=(const my_string& s) does. If they reference memory owned by the local temporary string you created, that is their fault. And yours if you know about it. In that case it would likely crash.

If on the other hand they make a deep copy of the string you passed to operator= then all is well, and when your local string goes out of scope it's fine.

You should not be worried about a memory leak, but rather about a possible dangling pointer in the string debug.

Answer 3

yes, its your fault, using bad code you know is bad is always your fault. Fix it or work around it with lots of comments but never pretend its someone else's problem.

The code doesn't leak, but it will probably crash.

What happens is that you return a pointer to the string, but when you leave the function, the original string memory is deleted. Now usually the memory remains intact for a short time, but sooner or later (usually sooner) something else will use the memory that the string used to occupy and you get garbage at best, crash at worst.

You'll note that the my_string returned from GetString probably creates a temporary copy (you haven't shown the code) so s would point to this temporary object. If GetString returns a pointer to the original string, then you're on safer ground, but now you're at the mercy of whatever owns the original string that GetString returned.

Return a string object that gets copied and return the copy (the compiler will optimise this very well so you get a minimum of memory copies) or return a shared_ptr so you can handle ownership better.