views:

182

answers:

4

So the code in question is this:

const String String::operator+ (const String& rhs)  
{  
    String tmp;  
    tmp.Set(this->mString);  
    tmp.Append(rhs.mString);  
    return tmp;  
}  

This of course places the String on the stack and it gets removed and returns garbage. And placing it on the heap would leak memory. So how should I do this?

+4  A: 

You should implement a copy constructor, a copy assignment operator, and a destructor, according to the rule of three. Then the stack-allocated temporary will be safely copied to the storage accepting the return value.

Nikolai N Fetissov
As also seen on Wikipedia (http://en.wikipedia.org/wiki/Rule_of_three_(C++_programming)
Motti
+11  A: 

Your solution doesn't return garbage if you have a working copy constructor - the String object tmp is copied into the result object before it is destroyed at the end of the block.

You could do this better by replacing

String tmp;
tmp.Set(this->mString);

with

String tmp(*this);

(you need a correctly working copy constructor for this, but you need it anyways for your return statement)

hjhill
+1  A: 

if you use std::string this neither leaks nor return garbage

does your class have a copy constructor (that works)

Either way it wont leak (unless String is very poorly designed, ie doesnt free its internal memory when its destructor gets invoked)

pm100
Didn't say it leaked, I said it WOULD leak if I allocated it on the heap and returned it. It was my copy constructor that was failing.
xokmzxoo
A: 

There is no memory leak. But you might want to change the return type to String instead of "const String". Otherwise this function wont be of much use

Yogesh Arora