Returning a local object results in garbage, while returning the same object temporary works fine

Question

I'm currently implementing my own string class (just for training), but I'm experiencing some problems in my substr() method:

MyString MyString::substr(size_t position, size_t length)
{
    if (checkBounds() || length == 0)
    {
        return MyString();
    }

    char* tmp = new char[length + 1];

    memcpy(tmp, this->s + position, length);

    tmp[length] = STRING_ESCAPE;

    MyString result(tmp);

    delete[] tmp;
    tmp = nullptr;

    return result;
}

When I call this method and print the return value (I'm printing the char array actuallay, not the object itself), I receive complete garbage, which is carried out as a bunch of squares.

But when I return a temporary object return MyString(tmp), everything works fine. Initially i suspected this issue is associated to the delete[] operation, but commenting it out shows no effect.

The MyString constructor which is called is the following:

MyString::MyString(const char* s)
{
    size_t length = this->strlen(s);

    this->sLength = length;

    this->s = new char[length + 1];

    for (size_t i = 0; i <= length; ++i)
    {
        this->s[i] = *s;

        ++s;
    }
}

So where is my mistake? Thank you!

It is hard to reconstruct what you actually do from the posted. Post a complete testable example. — n. m. could be an AI, Nov 08 '14 at 17:53
You can find a testable exmaple here: http://pastebin.com/LiaF5Ztx — BL9009, Nov 08 '14 at 18:08

score 0 · Accepted Answer · answered Nov 08 '14 at 18:55

0

In the two places you create MyString, they are being created within the context of your substr function. The return statement has to make a copy of them. Your copy constructor is probably not doing what you would need it to do.

A simpler design is for your substr function to return a pointer to a MyString that you create with operator new.

answered Nov 08 '14 at 18:55

KC-NH

748
3
6

that's it! the standard copy constructor just copied the pointer. i implemented a custom copy constructor which deep-copys the string buffer, and now it's working fine. thanks for the hint! – BL9009 Nov 08 '14 at 19:10

score 0 · Answer 2 · answered Nov 08 '14 at 20:49

0

You were indeed missing the copy constructor. Something like this would do the work:

MyString::MyString(const MyString& other) {
  size_t length = strlen(other.s);     
  s = new char[length+1];
  strcpy(s,other.s);
  this->sLength = length;
}

answered Nov 08 '14 at 20:49

chrphb

542
5
5

Returning a local object results in garbage, while returning the same object temporary works fine

2 Answers2