This is a discussion on Memory leak in Soln8_05.cpp from Ivor Horton's Beginning Visual C++ 2008 book? within the C++ Programming forums, part of the General Programming Boards category; Hi,
I've been working through Ivor Horton's Beginning Visual C++ 2008 book and have made good progress so far. However, ...

I've been working through Ivor Horton's Beginning Visual C++ 2008 book and have made good progress so far. However, I've come across what looks to be a memory leak in one of the exercise solutions and I'm sure it's got to be me that's wrong.

In chapter 8, exercises two through five you're asked:

2. Implement a simple string class in native C++ that holds a char* and an integer length as private data members. Provide a constructor which takes an argument of type const char*, and implement the copy constructor, assignment operator and destructor functions. Verify that your class works. You will find it easiest to use the string functions from the <cstring> header file.

3. What other constructors might you want to supply for your string class? Make
a list, and code them up.

4. (Advanced) Does your class correctly deal with cases such as this?
string s1;
...
s1 = s1;
If not, how should it be modified?

I thought that maybe since it was passing out of scope that it would be automatically "freed", but that only applies to automatic variables. Then I thought that maybe because it was a pointer to char* that it had static duration, but no, that applies to string literals.

I went back over the code and I keep getting hung up on the behavior of operator+ in this case. If I'm understanding you right, in your example the existing operator+= member function is used. To my untrained eyes, it looks like lhs is being modified.

Say you have three CSimpleString objects, str1, str2 and str3, all declared and initialized properly.

If I were to do:

Code:

str1 = str2 + str3

using the code you provided, wouldn't that modify str2?

Edit:

I went to test the code and got compile errors. I'm attaching the changed file and a diff between the author's original version, and my changes based on the code you suggested. I figure I'm overlooking something obvious.

Compile log:

Code:

ClCompile:
Soln8_05.cpp
Soln8_05.cpp(105): error C2679: binary '+' : no operator found which takes a right-hand operand of type 'CSimpleString' (or there is no acceptable conversion)
Soln8_05.cpp(24): could be 'CSimpleString CSimpleString::operator +(const char *)'
while trying to match the argument list '(CSimpleString, CSimpleString)'
Soln8_05.cpp(111): error C2679: binary '+' : no operator found which takes a right-hand operand of type 'const CSimpleString' (or there is no acceptable conversion)
Soln8_05.cpp(24): could be 'CSimpleString CSimpleString::operator +(const char *)'
while trying to match the argument list '(CSimpleString, const CSimpleString)'
Soln8_05.cpp(123): error C2678: binary '+=' : no operator found which takes a left-hand operand of type 'const CSimpleString' (or there is no acceptable conversion)
Soln8_05.cpp(22): could be 'CSimpleString &CSimpleString::operator +=(const CSimpleString &)'
while trying to match the argument list '(const CSimpleString, const CSimpleString)'

I went back over the code and I keep getting hung up on the behavior of operator+ in this case. If I'm understanding you right, in your example the existing operator+= member function is used. To my untrained eyes, it looks like lhs is being modified.

Yes, the idea is to modify lhs and return a copy of it. Unfortunately, you cannot do that, because lhs is a const reference. To fix that, change it to either:

Yes, the idea is to modify lhs and return a copy of it. Unfortunately, you cannot do that, because lhs is a const reference.

Okay, I think I understand you now. I should not implement your code from comment #3 because it is trying to modify a const object.

While on that thought, if the code from post #3 worked, wouldn't that be modifying str2 in the following code?

Code:

str1 = str2 + str3;

I know you can override the logic behind the operators when overloading them and that + doesn't have to mean addition, but when I read that statement I expect that str2 and str3 will remain untouched and only str1 will be changed.

On a different note, I went back and reran the code from post 5 and I'm not getting the all of the previous compile errors, only this one:

Code:

soln8_05.cpp(176): error C2678: binary '+=' : no operator found which takes a left-hand operand of type 'const CSimpleString' (or there is no acceptable conversion)
soln8_05.cpp(25): could be 'CSimpleString &CSimpleString::operator +=(const CSimpleString &)'

I also forgot to mention that I'm using Microsoft Visual Studio 2010 SP1 to compile the code.

I reverted back to the author's code, then made the changes you mentioned in the first code block of post 6 and got this error:

I've been taking your code examples and have pretty much dropped them in as-is, aside from adding the function prototype immediately following the class definition's closing curly brace and semicolon. The only other change has been removing the author's original code that yours replaces.

The second example gives the same error.

Could you look over the attached file and let me know what I'm doing wrong? It contains your second suggestion.

I should not implement your code from comment #3 because it is trying to modify a const object.

Yes.

Originally Posted by deoren

While on that thought, if the code from post #3 worked, wouldn't that be modifying str2 in the following code?

It does not work and trying to reason what would happen if it worked is futile.

Originally Posted by deoren

I know you can override the logic behind the operators when overloading them and that + doesn't have to mean addition, but when I read that statement I expect that str2 and str3 will remain untouched and only str1 will be changed.

BTW, the person's username who helped you before is "laserlight" not "lightning".

My apologies to laserlight on the mix-up. I knew a lightning from another forum and somehow made the (wrong) association on the name.

Originally Posted by oogabooga

And you're not really "adding" two strings but "concatenating" them.

I used the wrong terminology, but we're on the same page. I meant to convey that I associate the operator+() function with not changing the objects (str2 and str3 in my previous examples) and the operator+=() function with making changes.

Thanks for the macros.

Originally Posted by oogabooga

You're defining operator+= and operator+ each in terms of the other! You have to define at least one of them to be able to do its work without recourse to the other.

I was a little confused by that as well, but I was trying to take laserlight's code as-is and use it in place of the author's implementation. At this point, I have to admit I'm pretty confused.

I have my solution (which isn't near the quality as the author's, but works) and then I have the author's solution (Soln8_05.cpp, r1) where I noticed the leak after comparing my solution (not shown) with the author's.

oogabooga, you then responded with post #2, so I made the changes shown in (Soln8_05.cpp, r2) where I attempted to implement your suggested fix. It seems to work from what I can tell.

Then I saw laserlight's post and tried to implement the provided code as
(Soln8_05.cpp, r3).

I failed to do so, and received the errors mentioned in post #5.

laserlight responded again (thanks by the way, I do appreciate all help) on post #6 and provided more code. Based on the wording, it was an either/or approach so I implemented the first suggestion separately as (Soln8_05.cpp, r4) and the second suggestion as (Soln8_05.cpp, r5).

More errors, and likely because of how I implemented the provided code. I posted my results on post #7.

I reverted back to r2, oogabooga, and I've now included your macros as (Soln8_05.cpp, r6).

laserlight:

Are any of your code examples supposed to be usable as-is, or do I need to modify existing code to make it work? Please use (Soln8_05.cpp, r6) to compare against.

Thanks in advance.

P.S.

I'm using a public SVN repo of mine to provide code. I'm hoping this helps to make my thoughts clearer.

Are any of your code examples supposed to be usable as-is, or do I need to modify existing code to make it work?

If you defined operator+= correctly, without defining it in terms of the overloaded operator+, then yes, my examples in post #6 should work as is, assuming there are no problems with the copy constructor and/or destructor.

It does not work and trying to reason what would happen if it worked is futile.

Yes.

Thanks for the reply. I took forever to reply and by the time I did you had already responded.

I was referring more to a high-level pseudocode sort of view. I didn't understand the example of using += to modify an existing object inside of an overloaded + operator function, which is what I took this code to mean:

Originally Posted by laserlight

Incidentally, since you are already going to implement operator+=, you can use it to implement operator+ as non-member function:

If you defined operator+= correctly, without defining it in terms of the overloaded operator+, then yes, my examples in post #6 should work as is, assuming there are no problems with the copy constructor and/or destructor.

Which if I'm understanding you correctly, means the overloaded += function is defined in terms of the overloaded operator+ function. It appears we're on the same page now.

Ah yes. That is less common because operator+= modifies the left hand side argument, so there is no need for another object to be created, hence it is potentially more efficient than operator+. Implementing operator+= in terms of operator+ makes it no more efficient than operator+, which is a net loss.