0

This is the method used to handle addition of MyString class objects. Everything runs fine until the delete [] buff - statement. using visual studio community

MyString MyString::operator + (const MyString &rhs) const {
    size_t buff_size{ std::strlen(this->str) + 1 };
    char *buff = new char[buff_size];

    strcpy_s(buff, buff_size, str);
    size_t noOfEls{ std::strlen(str) + std::strlen(rhs.str) + 1 }; // total length + null terminator
    strcat_s(buff, noOfEls, rhs.str);

    MyString temp{ buff };

    delete[] buff; // complier error here
    return temp;
}
rioV8
  • 24,506
  • 3
  • 32
  • 49
eonsinde
  • 1
  • 1
  • 2
    Since you are returning `MyString` by value, you are more than likely violating the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three/4172961). Until you post your `MyString` class, that's the only conclusion to make. Please post a [mcve] – PaulMcKenzie Jul 16 '20 at 21:47
  • 2
    Your `buff` only has size `buff_size` but the call to `strcat_s` needs it to be at least of size `noOfEls`. So your buffer is too small which is undefined behaviour. – perivesta Jul 16 '20 at 21:50
  • 1
    use `std::vector` instead of doing the new/delete yourself, what is wrong with `std::string`? – rioV8 Jul 16 '20 at 21:53
  • Have a look here: https://stackoverflow.com/a/2425749/7066647 – dbaltor Jul 17 '20 at 00:01
  • dave - I passed in the correct parameters to the strcat_s function and that works fine the issue comes during freeing the buff memory. Paul - I'm not sure violating a rule is the problem here. – eonsinde Jul 18 '20 at 12:29
  • Dave - I've did everything correctly the issue comes in freeing memory ; Paul - returning by value isn't the issue here. – eonsinde Jul 18 '20 at 12:52

1 Answers1

0

So I finally figured out the issue in my code after I tried to write a non member function which is a friend of the MyString class and the function worked with out any errors. I wondered what I did right and found out that the buffer memory allocated wasn't actually enough to store data incoming from the right hand side paramter (rhs variable). Here is the fixed code and delete [] buff worked fine as expected.

MyString MyString::operator + (const MyString &rhs) const {
    size_t buff_size{ std::strlen(this->str) + std::strlen(rhs.str) + 1 };
    char *buff = new char[buff_size];

    strcpy_s(buff, buff_size, str);
    strcat_s(buff, buff_size, rhs.str);

    MyString temp{ buff };

    delete[] buff;
    return temp;
}

If you take a closer look at the buff_size variable, you'll see that I allocated memory for both the length of the current object(this) and the incoming rhs object(NB: plus the null terminator) as opposed to the initial code I wrote above

eonsinde
  • 1
  • 1