CODEWITHSUNDEEP
c++memory-leaksreturn-value
Smeagol
Smeagol

Reputation: 632

How to free returned value correctly

I need to create an + operator for my class, I did it like this:

class CDoubleString{
public:
   string textA="";
   string textB="";
   CDoubleString(string x,string y) : textA(x),textB(y){}

   CDoubleString & operator + (const CDoubleString & y){
       CDoubleString * n=new CDoubleString(textA,textB);
       n->textA+=y.textA;
       n->textB+=y.textB;
       delete n;
       return *n;
   }
}

It seems that it is working as expected, but I see that there is a problem with freeing the memory. In the moment I returned it, it already may be something else. So it is undefined behaviour, am I correct?
How to avoid that?

Upvotes: 0

Views: 56

Answers (2)

user743382
user743382

Reputation:

So it is undefined behaviour, am I correct?

Yes, but not quite for the reason you think. You're returning a reference to an already-deleted object, and that's invalid and has undefined behaviour. But the "it already may be something else" applies to the caller, because it's the caller that would actually end up reading from that dead reference, and that's something that could cause problems even if you were to delay the deletion of the object: you might still not delay it enough.

How to avoid that?

Returning by value is probably easiest.

CDoubleString operator + (const CDoubleString & y){
    CDoubleString n(textA,textB);
    n.textA+=y.textA;
    n.textB+=y.textB;
    return n;
}

Upvotes: 1

πάντα ῥεῖ
πάντα ῥεῖ

Reputation: 1

So it is undefined behaviour, am I correct?

Yes.

How to avoid that?

There are several ways.

  1. Return by value

    CDoubleString operator + (const CDoubleString & y){
    CDoubleString n(textA,textB);
    n.textA+=y.textA;
    n.textB+=y.textB;
    return n;
}

  2. Return a std::unique_ptr

    std::unique_ptr<CDoubleString> operator + (const CDoubleString & y){
    std::unique_ptr<CDoubleString> n = std::make_unique<CDoubleString>(textA,textB);
    n->textA+=y.textA;
    n->textB+=y.textB;
    return n;
}

I'd prefer the 1st variant for your case. You can rely on RVO and copyelistion for most modern compilers, so you don't need to worry about additional copies made.

Upvotes: 3

Related Questions