2

I am trying to overload operator + for my class as follows:

MyClass MyClass::operator +(const MyClass& rval) const {
 MyClass ret(m_src); // m_src is member of MyClass: char* m_src;
 ret.Add(rval); // this->m_src + rval, this method work correctly 
 return ret; // so, in ret.m_src I have correct value
} // but after this C++ call destructor for ret

Destructor:

delete[] m_src; // because in some methods I allocate dynamic memory

So, destructor clear memory and function return trash. How I can avoid this situation? If I delete destructor, function work normal, but in this case I have memory leak :(

P.S: I can't change prototype of overloading +, unfortunately. Thanks.

m0stwanted
  • 35
  • 1
  • 7
  • 1
    are you talking about when you return from +? You need to implement a copy constructor that does deep copying. – thang Feb 03 '13 at 08:04
  • 1. This function is returning a `MyClass` , which is neither a reference or a pointer. 2. ret is not created with new constructor. Therefore, you don't need to `delete` it. – Konfle Dolex Feb 03 '13 at 08:07
  • Yes, after return from + Hm, I get to try it May be exist more ways to solve my problem? – m0stwanted Feb 03 '13 at 08:08
  • @KonfleDolex yes, and I'm not calling it. Destructor was called automatic after return from function. – m0stwanted Feb 03 '13 at 08:09
  • 1
    can you post all of your constructors? i think there are more issues than just there. – thang Feb 03 '13 at 08:09
  • @m0stwanted try returning a reference. Change the return type to `MyClass&` . – Konfle Dolex Feb 03 '13 at 08:11
  • @thang Yes, I have only two constructors: MyClass() : m_src(NULL), m_size(1) {} MyClass(const char* src); // assign src to m_src – m0stwanted Feb 03 '13 at 08:11
  • 1
    you need to make your copy constructor do deep copying. otherwise, you have bigger problems.... you'll end up with tons of dangling pointers. – thang Feb 03 '13 at 08:12
  • @KonfleDolex I try, but destructor was called anyway and it's doesn't worked. – m0stwanted Feb 03 '13 at 08:14
  • changing the return type to MyClass& doesn't help. Don't do it. May cause problems down the road. The stack object will get destroyed anyway. If you leave it as it is, it may be elided out. I don't know to what extent C++ is allowed to elide out stuff when you return a reference. – thang Feb 03 '13 at 08:16
  • @m0stwanted sorry, what I said was wrong. Returning reference will not work. Try returning a pointer. Change the return type to `MyClass*` and change the function content to `MyClass* ret = new MyClass(m_src);ret.Add(*rval);return ret;` – Konfle Dolex Feb 03 '13 at 08:16
  • @thang ok, I will try to make a copying constructor. Thank you – m0stwanted Feb 03 '13 at 08:16
  • returning a class can be expensive. I would say returning a pointer is a better approach. BTW, you have to remember to `delete` the pointer returned. – Konfle Dolex Feb 03 '13 at 08:18
  • @KonfleDolex I try it, but whether this will lead to a memory leak? – m0stwanted Feb 03 '13 at 08:20
  • returning pointer changes the semantic of the + operator. for example, if i want to do (a+b)+c... if (a+b) returns a pointer, then (a+b)+c fails because there is no operator for + with pointer on the left. with respect to returning a class, please see this thread http://stackoverflow.com/questions/12953127/what-are-copy-elision-and-return-value-optimization – thang Feb 03 '13 at 08:21
  • @m0stwanted as long as you `delete` the pointer returned, it will not cause memory leak. BTW, you can use software like `valgrind` to check for memory leak. – Konfle Dolex Feb 03 '13 at 08:22
  • @m0stwanted forget what I have said. thang was right. – Konfle Dolex Feb 03 '13 at 08:22
  • @thang you are right, if I change prototype with pointer, I can't write some of: MyClass sum = a + b; // a,b: MyClass – m0stwanted Feb 03 '13 at 08:23
  • So, I write copying constructor, and return from operator like this: return (MyClass(ret)); Destructor calling anyway and I again have a trash in sum :( – m0stwanted Feb 03 '13 at 08:40
  • So, in debugger first calling for ret, second - for copied variable > – m0stwanted Feb 03 '13 at 08:43
  • The semantics in this function are fine, the problem is somewhere else in your code. As you say, ret will be destructed, but not before it is copied to the destination of the caller. – Alex Chamberlain Feb 03 '13 at 08:47
  • But where can be mistake? All methods work fine, except this operator. – m0stwanted Feb 03 '13 at 08:57

2 Answers2

2

You avoid this by replacing:

char *m_src; 

by:

std::string m_src;

in your class MyClass.

With char * as a class member what you get is easy to go mess up manual memory management, you simply replace that with implicit RAII based memory management of std::string and save yourself all those troubles. And this is the very purpose of existence of std::string in C++.


If you cannot use standard library std::string class(though I simply fail to understand why), You need to make sure you are following the Rule of Three.

Community
  • 1
  • 1
Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • 2
    @m0stwanted You're using C++ but you can't use the C++ standard library? Do you know how silly that sounds? Is this some kind of homework question? – Rapptz Feb 03 '13 at 08:08
  • @m0stwanted: `std::string` is not STL. It is a C++ standard library class. It should be available for use with any Standard C++ compilant implementation. – Alok Save Feb 03 '13 at 08:11
  • std::string *can be* STL. depends on whom you ask. – thang Feb 03 '13 at 08:14
  • @Rapptz I know :) No, it is't a homework, but I need to make it without any library. – m0stwanted Feb 03 '13 at 08:16
  • 1
    @m0stwanted: Well you are already using that very same library when you used any of the standard C++ constructs, So there is no additional library. It is one and the same library.You are just restricting yourself to not use a part of the standard library because of your wrong notion that it is a different library. It is not. – Alok Save Feb 03 '13 at 08:17
0

What you need is a copy constructor that copies m_src

MyClass::MyClass( const MyClass& ref ) {
    m_src = new char[/*get len from ref*/];
    memcpy( m_src, ref.m_src, /* len from ref */ * sizeof(char) );
}

Now you can change the operator+ to work with the copy constructor

MyClass MyClass::operator +(const MyClass& rval) const {
    MyClass ret(rval); // use copy constructor
    ret.Add(rval); // this->m_src + rval, this method work correctly 
    return ret; // so, in ret.m_src I have correct value
}
Shai
  • 111,146
  • 38
  • 238
  • 371