returning a pointer or a value from a operator

Question

0.00/5 (No votes)

See more:

Hi, imagine I have some String class, e.g., String.
I also have following code:

C++

String x,y,z;
z=x+y;

Now, if I implement the + operator of my String class in
the following way:

C#

String operator + (const String& other) const
       {

             String result;
             //..
             // copy existing string to result
             // append other to it
             return result;
            
       }

My initial code will have problem - because z will point to a object (returned by x+y)
which was destroyed.

How to avoid this situation, assuming I don't want to write an assignment operator which does deep copying. I could have the + operator in the following way:

C#

String operator + (const String& other) const
       {

             String *result=new String();
             //..
             // copy existing string to result
             // append other to it
             return *result;
            
       }

but then how do I delete the memory I allocated inside operator +?

Thanks.

Posted 14-Mar-13 0:56am

Matra Divat

Updated 14-Mar-13 1:01am

v2

Add a Solution

3 solutions

Solution 1

You delete the allocated memory in the same way as you always do by calling delete with the address of the allocated memory. In this case something like.

String& z = x + y;
...
delete &z;

This is not how most string classes work because they have the allocated memory assigned to an internal member variable, e.g.

class String
{
...
private:
 unsigned char* m_pStringMemory;
...
};

The internal pointer is always deleted in the destructor of the containing class e.g.

String::~String()
{
  delete m_pStringMemory;
}

so given correct assignment, copy construction and + operator member functions you can then do

String x, y, z;
x = y + z;

and no need to clean up at this external level because everything here is on the stack. The automatic destruction of x when it's stack frame is unwound will call String::~String() and free the memory allocated by the + operator.

Posted 14-Mar-13 1:06am

Matthew Faithfull

Comments

[no name] 14-Mar-13 7:27am

Hm... what you suggest is interesting actually in my case also my String class inside has an internal member variable e.g., (the copy constructor just does a shallow copy)
class String
{
...
StringBuff* m_pStringMemory;
...
}
I am also deleting it in destructor. So you suggest if I implement + operator as in my initial post (using new) and do x=y+z - I will not need to care about deleting the memory that was allocated in the + operator?

Matthew Faithfull 14-Mar-13 7:41am

As long as you manage the memory correctly inside the class, for example delete and reallocate in the assignment operator, reuse the assignment operator to implement the copy constructor etc then yes the destructor will always do the final cleanup for you. Note what nv3 says below about loosing assigned pointer values by returning copies of the item they point to but this should not be an issue if you are keeping the m_pStringMemory internally, only ever assigning allocated memory to that, initialising it to NULL in all your constructors ( preferably in the initializer list ) and always remembering to delete it before assigning a new allocation to it ( delete NULL is always safe after early versions of VC6 ).

[no name] 14-Mar-13 7:33am

This solution:
String& z = x + y;
...
delete &z;
here would not I have to change the signature of the + operator to:

String& operator + (const String& other) const?

Matthew Faithfull 14-Mar-13 7:43am

No. A reference can be constructed by assigning an object to it. In fact it can't be constructed otherwise.

[no name] 14-Mar-13 7:55am

yes but will this make sense? as nv3 said, when operator + returns, it will return a copy of *result - meaning delete &z will delete something else, not the initial String *result = new String() variable which I allocated inside the operator +.

Matthew Faithfull 14-Mar-13 8:06am

He's right about that you should use your original style of + Operator that returns a String instance that is a copy from one on the stack. My inital solution only works if you return the pointer to a new String but you shouldn't do that if the String manages its own memory.

[no name] 14-Mar-13 8:24am

Yes but as you saw in my solution I was not returning pointer to the new string rather I was returning *result. So, what shall I do in this case, because as I mentioned in my code, my original code has problem. Particularly z will be pointing to data structures which have been destroyed because they were allocated on the stack. And my copy constructor just does a shallow copy, and I don't want to write a deep copy yet. Thank you.

Matthew Faithfull 14-Mar-13 8:30am

Then I'm afraid you're out of luck. In your original code z already exists before you assign the result of x + y to it. Hence you are doing assignment to z. Assignment without a deep copy or data transfer simply won't be assigning properly so it will never work. In short any sensible implementation of the + operator will be dependent on assignment which will in almost all cases be the same as copy construction, i.e. a deep copy.

Solution 3

// How to avoid this situation ?

Just make sure, that the String has a copy constructor.
In this case, the result will be copied onto the stack, before its destroying :)

Posted 14-Mar-13 5:08am

Eugen Podsypalnikov

Comments

nv3 14-Mar-13 11:12am

Eugen, please see OPs comments above. He tries to avoid a deep copy, which is smart for a good string class. I just have the feeling that the ref counting scheme is not implemented correctly.

Eugen Podsypalnikov 14-Mar-13 11:25am

OK... Then we will need a memory manager for all such strings, to register and handle their references(inc/dec), sharing and modifing to/of the data :)

nv3 14-Mar-13 11:38am

Not necessarily. The ref counting could be done in the buffer struct. Matthew has described it nicely in his comment above. That's btw. how MS implemented their CString class.

Eugen Podsypalnikov 14-Mar-13 12:00pm

OK, in this case the String must handle its ref/unref/clone needs by itself... :)

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

nv3 · Accepted Answer · 2013-03-14T01:32:00

Solution 2

Your original version is actually the correct one. You create a temporary String object inside your operator+ function and return it by value to the caller. In the function return code, the value will be copied to another temporary string on the stack and then via the assignment operation that value will be copied to your variable z. (Don't worry, the compiler can optimize a lot of this copying away for you.)

What's probably not correct is the way in which you implemented the copy constructor and assignment operators. If you are not receiving the correct value in z, then look into these functions; you may also show us the code of them by using the green "Improve Question" button, so we can give you a little help with that.

Now to your alternative code:

// DON'T DO THAT!!!
String operator + (const String& other) const
       {

             String *result=new String();
             //..
             // copy existing string to result
             // append other to it
             return *result;

       }

Here is what happens. You allocate a new String on the heap and store its pointer in result. So far so good. On return the value of this object is put on the stack and returned the caller, i.e. it is copied because you are returning a String value and not a String*. Your pointer to the object on the heap is lost forever and hence you have created a memory leak. That's why this is no good idea.

There is a second reason: Allocating an object on the heap is much more expensive in terms of time than an allocation on the stack, because it requires thread synchronization in multi-threaded applications. So avoid that in low-level functions whenever possible.

Posted 14-Mar-13 1:32am

nv3

Comments

[no name] 14-Mar-13 7:51am

My copy constructor just does a shallow copy, e.g., if I have
class String
{
StringBuff *m_buff;
}
The copy constructor will just to a shallow copy, make m_buff point to the m_buff address of the object which we want to copy. That is why my above code is problematic, the m_buff starts to point to a variable which was destroyed. I don't want to make a deep copy in my copy constructor yet. That is why I asked this question. ps. this is how my = operator looks like:
String& operator = (const char* source)
{
(*this).Assign(source);

return *this;
}

nv3 14-Mar-13 8:26am

You can continue doing it in this economical way, but then you have to implement reference counting on that buffer pointer. That is a little bit tricky, but manageable. The way you have implemented it now will not only cause problems in the given scenario, but also in various other examples.

[no name] 14-Mar-13 8:40am

I have reference counters. The reason I don't want deep copy is because my string is "lazy" , when you copy a string, you don't want to do a deep copy unless someone tries to modify the string (and then, you make a brand new copy of the string and modify that one). So, what is the solution in my situation? Thank you. ps. you could also check the last comment of Mathew Faithfull.

nv3 14-Mar-13 10:30am

If you do reference counting on the buffer you actually should not have the problem. Here is the scenario: Let's considers what should happen in your original code:

1. you construct String result on the stack and assign to it the concatenation of x and y
2. you execute the "return result" statement, which will:
a) create a temporary String object on the stack by using the copy constructor; that will increase ref count on your buffer
b) destroy the object "result", which will not delete the buffer, because its ref count is 2; instead it just decrements the ref count to 1.
3) assign the temp object to z, which will first increment the ref count to 2 again
4) the temp object is destroyed, which again decrements the ref count to 1 and does not delete the buffer.

So now z is using the buffer that you had originally allocated in String "result". And all is fine.

If you like, then post your ref counting code as amendment to your original question and I will take a look at it.

[no name] 14-Mar-13 11:48am

H, nav3. Thanks for your feedback, here is my copy constructor:
String(const String& s): data(s.data)
{

// Increase reference count.
++data->ref;
}
data - member variable is some data struct, let's say StringBuff.
I am getting an exception due to x=y+z code, but one clue that we may have
in solving this issue, is that in the debugger, after I cross the x=y+z line,
indeed x's data structure is pointing to the buffer we allocated in + operator,
however ,it's reference count is not initialized and is a negative value and *also* the char * buffer which is located inside the StringBuff class points to a _badpointer_.
I think this is the problem. What can be causing this? Thank you.

nv3 14-Mar-13 12:06pm

Looks like the ref count was not initialized correctly. Or you have bug in the destructor code. There you should decrement the ref count and if you have arrived at 0 then delete the buffer. Best is you set a couple breakpoints in the constructors, copy constructor and destructor and check what exactly is going on there. I think you are very close to solving it now.

[no name] 14-Mar-13 12:19pm

Hi, nv3, thanks for your reply. Indeed that's what I am doing in my destructor just checking < 1 in the references after I decrement them (also please see my updated post above). I am not sure what is wrong with this, it enters three times in my destructor of String - and the last time it enters, the ref count as well as char * pointer located inside StringBuff are not initialized... and hence the problem I guess...

ps. this is code inside: + operator

Stringobj(this->data->data);
obj.append(other.data->data);
return obj;

btw. which is correct to write: new StringBuff, or new StringBuff()? Thanks. (this is not the cause of the problem I guess, but still).

ps. I was thinking maybe the problem is inside my assignment operator, but as I see if I put a breakpoint inside it, it never gets hit -- why would this be the case? I am using = sign in my code, as in for example: x = y+z

btw. What should my assignment operator ideally to? Here is the signature of my current overloaded assignment operator: String& operator = (const char* source)

I see now... I actually have not implemented assignment operator which takes on the right side a String object for example.........

nv3 14-Mar-13 12:54pm

And here is the error: If you haven't implemented the assignment operator yet, it uses the default assignment operator, which is a plain copy. And thereby your reference counting fails. The temp string's ref count goes down to zero and the buffer is deleted. Then when you look at the receiving variable x, you are looking via the pointer to the deleted buffer and the ref count in there is no undefined and in the debug version some nice patter like 0xdededede.

[no name] 14-Mar-13 12:59pm

Hi, nv3 yes, thanks I think I solved the issue and now even when I write: x=y+z and program exits, no exception is thrown. This I did as we mentioned already by writing a new overloaded assignment operator. Just one more question what should the assignment operator do in my case? I think similar things as the copy constructor right? Nice, it's not crashing anymore.

nv3 14-Mar-13 13:06pm

Congrats!

Now, the assignment operator is a little different. You must take into account that your target String is already pointing to some other buffer. So first detach from that buffer (if the pointer is non-null) by decrementing the ref count there and if necessary destroying that buffer. Then, assign the pointer to the new buffer and increment its ref count.

[no name] 14-Mar-13 13:42pm

Ok, thanks nv3 we have nailed it down. But when you say "if neccessary destroy the buffer" this means I must also check the reference if it is 0 then i destroy the buffer, otherwise we have a problem right?
And finally, how would you make such a String class safe??

nv3 14-Mar-13 14:20pm

Yes, that's what I meant in my comment. I would recommend putting the incrementing and decrementing of the ref count in member functions. Then you have a centralized place to check whether the ref count is down to zero, in which case you do a "delete this".

I would not worry in such an elementary class about thread safety. That is usually done in higher levels. Anyone who uses your String class must make sure by appropriate syncronization mechanisms that the participating objects are stable while you are working on them. If you implement thread safety inside your String class you will be paying a high price in terms of performance for it.

As a last remark: You might want to take a look at Microsoft's CString class in MFC, which is similar in its implementation to yours. You might get some pretty good ideas from their implementation. If you don't have the MFC sources available, google for one of the free substitutes for CString.

You are welcome. I am glad I could help.

[no name] 14-Mar-13 14:26pm

Hi, nv3, thanks indeed our discussion was fruitful I think. But nevertheless, if I decide to do thread safety as I described, where can I find info on how to do it for my class as I described? or how to do it, any tipps?

nv3 14-Mar-13 15:17pm

I'd start learning about the syncronization methods, like semaphores, mutexes, critical sections. And of course learning what threads are and in which ways they interfere with each other. I am sure your find thousands of good links by just putting these buzzwords into Google or into the search box of CodeProject.

nv3 15-Mar-13 8:01am

That of course depends a lot an your abilities and aptitude. To get the basic concepts down, perhaps a couple days. To master all the subtleties, a couple years.

[no name] 15-Mar-13 8:44am

no, not to master all subtleties, just to gain enough skills to make above class thread safe for instance... thanks.

nv3 15-Mar-13 9:38am

For an average MFC-experience programmer perhaps a couple of hours. For a beginner it takes already a couple of days to understand the concepts, then another couple of days to implement and test it.

Again, I would by no means consider it reasonable to implement thread interlocking for a String class.

[no name] 17-Mar-13 16:13pm

Hi, nv3, I will bother you once more. Can you please give me some nice link which explains all the above preferrably in C++ context? I can't seem to find nice links, most of them are scattered sometimes, or dicuss these concepts in general (e.g., such as Wikipedia) thanks.

Matthew Faithfull 14-Mar-13 11:29am

I agree with nv3's approach. Reference counting is the exceptional case which was the reason for mentioning 'data transfer' and 'almost all cases' in my last comment above. In a reference counted implementation you would usually upgrade your internal buffer to be a struct or a class in its own right. This would contain both the data buffer and reference count. It must also do the buffer memory management jobs mentioned before and can then be referenced by more more than one outer string object. The string object itself manipulates the inner object by increasing and decreasing its reference count rather than allocating and deallocating but with a very similar pattern of code. If you're doing this already and it isn't working then do post your code as recommended. There's a good chance someone will will be able to spot what's worng.

nv3 14-Mar-13 11:36am

Thanks Matthew, that's exactly how I would implement the ref counting.