C++ Corrupted Heap

Question

My program seems to throw a rune-time exception about a corrupted heap when the main method returns. I have taken the proper precautions to not have this happen, including a copy constructor. Could anyone shed some light on why this is happening?

MyString.cpp

#include "MyString.h"
#include <cstdio>
#include <Windows.h>

MyString::MyString() {
    str = (char*)malloc(sizeof(char));
    *str = '\0';
}

MyString::MyString(char* src) {
    int size = sizeof(char)*(strlen(src) + 1);
    str = (char*)malloc(size);
    strcpy_s(str, size, src);
}


MyString MyString::operator+(char* add) {
    int addSize = sizeof(char)*strlen(add);
    int fullSize = sizeof(char)*(strlen(str) + 1) + addSize;
    str = (char*)realloc(str, fullSize);
    char* temp = str;
    temp += strlen(str);
    strcpy_s(temp, addSize + 1, add);
    return *this;
}

MyString::~MyString() {
    if (str)
        free(str);
}

MyString::MyString(const MyString &arg) {
    int size = sizeof(char) * (strlen(arg.str) + 1);
    str = (char*)malloc(size);
    strcpy_s(str, size, arg.str);
}

main.cpp

#include <iostream>
#include "MyString.h"
using namespace std;


int main(int argc, char *argv[]) {
    MyString test = MyString("hello!");
    test = test + " world";
    cout << test.toString() << endl;
    cout << strlen(test.toString()) << endl;
    system("pause");
    return 0; //runtime error here
}

I would encourage to use `new` and `delete` instead of `malloc` and `free`. — grigor, Aug 10 '16 at 20:35
Should be getting a compiler warning at `test = test + " world";` because `" world"` is a `const char *`, not a `char *`. Same with `MyString test = MyString("hello!");` — user4581301, Aug 10 '16 at 20:36
Normally I would agree @grigor , but then the `realloc` trick they use later would be a bit nasty. — user4581301, Aug 10 '16 at 20:36
Voting to close as off topic, requires [mcve], because it's been about 45 minutes since missing information was requested and it has not been supplied. What has been posted is flawed, but not fatally so; the cause of the crash cannot be determined without the complete definition and implementation `MyString`. The two answers so far are probably correct, but this can't be proved. — user4581301, Aug 10 '16 at 21:22

Amadeusz · Accepted Answer · 2016-08-10T21:20:35.210

I'm fixing my post according to @user4581301 suggestion:

You sholud modify your addition operator overload so it spawns a new object and also implement assigment operator overload like this:

MyString operator+(char* add) const {
    int thisSize = sizeof(char)*strlen(str);
    int addSize = sizeof(char)*(strlen(add) + 1);
    int fullSize = thisSize + addSize;

    char* tempStr = (char*)malloc(fullSize);
    strcpy_s(tempStr, fullSize, str);
    strcpy_s(tempStr + thisSize, fullSize, add);

    return MyString(tempStr);
}

MyString& operator=(const MyString& assign){

    int assignSize = sizeof(char)*(strlen(assign.str) + 1);

    str = (char*)realloc(str, assignSize);

    strcpy_s(str, assignSize, assign.str);

    return *this;
}

Off topic: There is a really good trick that uses `+=` to implement `+` covered here: http://stackoverflow.com/questions/4421706/operator-overloading/4421719#4421719 — user4581301, Aug 10 '16 at 21:26

score 0 · Answer 2 · answered Aug 10 '16 at 20:55

0

You have to learn about Rule Of Three

Implicit assigment operator is used and when the old object is destroyed new one uses already freed pointer and later it tries to free it again.

answered Aug 10 '16 at 20:55

KIIV

3,534
2
18
23

Very high likelihood you are correct. `test = test + " world";` may well be fatal, but we need to see `operator=` implementation, if any, to be sure. The copy constructor and destructor look Rule of Three compliant and we have not seen the assignment operator. Until then, the best we can do is guess. – user4581301 Aug 10 '16 at 21:17

C++ Corrupted Heap

2 Answers2