Corrupting the heap while writing an object to a binary file

Question

I have this class with a constructor and destructor

class Monster
{
    char* nume;
    double hp;
    float* dmgAbilitati;
    int nrAbilitati;

public:

    Monster(const char* nume, double hp, int nrAbilitati, float* dmgAbilitati)
    {
        if (nume == nullptr)
            throw new exception("Nume invalid!\n");
        else
        {
            this->nume = new char[strlen(nume) + 1];
            strcpy_s(this->nume, strlen(nume) + 1, nume);
        }
        if (hp <= 0)
            throw new exception("Hp invalid!\n");
        else
            this->hp = hp;
        if (nrAbilitati <= 0 && dmgAbilitati == nullptr)
            throw new exception("nrAbilitati invalid sau dmgAbilitati invalid!\n");
        else
        {
            this->nrAbilitati = nrAbilitati;
            this->dmgAbilitati = new float[nrAbilitati];
            for (int i = 0; i < nrAbilitati; i++)
                this->dmgAbilitati[i] = dmgAbilitati[i];
        }
    }

    ~Monster()
    {
        if (this->nume != nullptr)
            delete[] this->nume;
        if (this->dmgAbilitati != nullptr)
            delete[] this->dmgAbilitati;
    }
}

And I am trying to write and read a Monster object to/from a binary file using these 2 functions

void scriereFisierBinar(ofstream& fisBinarOut)
    {
        int dim = strlen(nume) + 1;
        fisBinarOut.write((char*)&dim, sizeof(dim));
        fisBinarOut.write(this->nume, dim);

        fisBinarOut.write((char*)&this->hp, sizeof(this->hp));
        fisBinarOut.write((char*)&this->nrAbilitati, sizeof(this->nrAbilitati));

        for (int i = 0; i < this->nrAbilitati; i++)
            fisBinarOut.write((char*)(&this->dmgAbilitati[i]), sizeof(this->dmgAbilitati[i]));
    }

    void citireFisierBinar(ifstream& fisBinarIn)
    {
        int dim = 0;
        char aux[100];
        fisBinarIn.read((char*)&dim, sizeof(dim));

        fisBinarIn.read(aux, dim);
        if (nume != nullptr)
            delete[] nume;

        this->nume = new char[dim];
        strcpy_s(this->nume, dim, aux);

        fisBinarIn.read((char*)&this->hp, sizeof(this->hp));
        fisBinarIn.read((char*)&this->nrAbilitati, sizeof(this->nrAbilitati));

        for (int i = 0; i < this->nrAbilitati; i++)
            fisBinarIn.read((char*)(&this->dmgAbilitati[i]), sizeof(this->dmgAbilitati[i]));
        
    }

In main i have this:

float vvv[] = { 44,432,366,433,511 };
    Monster TEST000("Sper", 6969, 5, vvv);

    ofstream fisBinarOut("tt.bin", ios::out | ios::binary | ios::app);
    if (fisBinarOut.is_open())
    {
        TEST000.scriereFisierBinar(fisBinarOut);
        fisBinarOut.close();
    }
    else
        cout << "No!\n";

    float vvvv[] = { 1 };
    Monster mm("Nu", 3123, 1, vvvv);

    ifstream fisBinarIn("tt.bin", ios::in, ios::binary);
    if (fisBinarIn.is_open())
    {
        mm.citireFisierBinar(fisBinarIn);
        cout << mm;
        fisBinarIn.close();
    }
    else
        cout << "No\n";

I want the 'mm' object to have the attributes of 'TEST000', but only the name and Hp change plus I receive a heap error : "HEAP CORRUPTION DETECTED. CRT detected that the application wrote to memory." So i am guessing that the array implementation is wrong. I have attached a picture of the error along with the results. Using char* is mandatory for this project. It also throws an exception to the destructor.

https://i.sstatic.net/cRGGq.jpg

Answer 1

The problem is in citireFisierBinar: while there is a reallocation for the nume array member, there is no reallocation (or at least check for sufficient memory) for the dmgAbilitati array member.
There are some other isuues in your code, for example:

• Throwing exceptions: ctor code throws a pointer; it should throw a non-pointer value instead.
• It provides no exception guarantee: if new memory can't be allocated, bad_alloc exception will be thrown and the object will not be in a valid state. For example, the memory for numa is deallocated in citireFisierBinar, so how do you think, what would happen in destructor. Or in ctor, if an exception is thrown (because of invalid arg) your object can be like "half-constructed" and the memory is leaked.

Theese are the major issues.