Memory Allocation in C++ (Exception thrown: read access violation.)

Question

I'm trying to learn a little bit more of C++! After working around with memory allocation for a while I got to a place where I'm struggling to understand it.

I wrote a code that works well (not really sure of that but at least doesn't show any memory violation) for a type of initialization (of an object of some class) but it crashes for a similar initialization.

I would appreciate if someone could me explain what is happening and how can I solve this problem.

My thought: The problem is in the line bellow because I'm trying to delete an array of allocated objects when in the problematic initialization I only have one object allocated and not an array.

delete[] pointer; //PROBLEMATIC LINE

PS.: I'm not looking for alternative solutions (like using smart-pointers or whatever). Sorry for my English!

The code:

class class1
{
private:
    unsigned int    s;
    double* pointer;
public:
/* Constructors */
    class1() { s = 0; pointer = nullptr; }
    class1(unsigned int us, double* uarray)
    {
        pointer = new double[us];
        for (unsigned int i = 0; i < us; i++)
            pointer[i] = uarray[i];
    }
    class1(const class1& other)
    {
        pointer = new double[s];
        for (unsigned int i = 0; i < s; i++)
            pointer[i] = other.pointer[i];
    }
    ~class1() { if (!s && pointer != nullptr) delete[] pointer; }

public:
/* Operators Overloading */
    class1& operator=(const class1& other)
    {
        s = other.s;
        pointer = new double[s];
        for (unsigned int i = 0; i < s; i++)
            pointer[i] = other.pointer[i];
        return *this;
    }
};

class class2
{
private:
    unsigned int    m;
    unsigned int    n;
    class1* pointer;

public:
/* Constructors */
    class2(unsigned int un, double* uarray, bool flag = false) : n(un)
    {
        m = 1;
        pointer = new class1(un, uarray);
        if (flag) { this->function(); }
    }
    ~class2() { if (!m && !n) delete[] pointer; }

public:
/* Public Methods */
    void function()
    {
        class1* newpointer = new class1[n];
        //**... some code (when commented show the same error)**
        delete[] pointer; //**PROBLEMATIC LINE**
        pointer = newpointer;
    }

public:
/*Template Constructor*/
    template<unsigned int m, unsigned int n>
    class2(unsigned int um, unsigned int un, double(&uarray)[m][n], bool flag = false) : m(um), n(un)
    {
        pointer = new class1[um];
        for (unsigned int i = 0; i < um; i++)
        {
            class1 object1(un, uarray[i]);
            pointer[i] = object1;
        }
        if (flag) { this->function(); }
    }
};

int main()
{
    double test3[] = { 1, 2, 3 };
    double test4[][3] = { {3, 2, 1}, {6, 5, 4}, {9, 8, 7} };
    double test5[][3] = { {1, 2, 3}, {4, 5, 6}, {7, 8, 9} };

    class2 m4(3, test3, true);      //**NOT OK - VIOLATION OF MEMORY**
    class2 m5(3, 3, test4, true);   //**OK**
}

Answer 1

Your copy constructor for class1 is not setting the s member, but uses its indeterminate value here:

pointer = new double[s];

causing undefined behavior. Set s from other.s before using it.

---

Your second constructor has the same problem.

---

Your assignment operator of class1 is leaking memory, because it doesn't delete[] the previous array.

---

In class2 you use new in the non-array form, e.g. here:

pointer = new class1(un, uarray);

but in the destructor you call delete[] to delete pointer. This is also causing undefined behavior. Pointers returned from the non-array version of new need to be deleted by delete, e.g. delete pointer.

But since you are also using the array version of new for pointer, you cannot use delete pointer either. As using delete instead of delete[] on a pointer returned from a array-new has also undefined behavior.

Be consistent and use always the array-new, e.g.:

pointer = new class1[1]{{un, uarray}};

---

class2 causes undefined behavior when an object of its type is copied or moved, because you didn't implement a copy constructor and assignment operator although you have defined a destructor. This is a violation of the rule-of-three.

---

There is probably more that I missed. The code is not readable at all. Please use proper variable names next time. (I hope that the real code does not use this naming scheme...) E.g. having a non-type template parameter m with the same name as a member of that class and then using m in multiple places of that context is not ok. I had to check the lookup rules to make sure that this actually compiles and does something sensible.