Seg fault when deallocating memory in destructor

Question

I have two simple classes. Everything is just fine, when I have an empty destructor in A class (empty ~A(){};). When I add there lines to free an array, my program gives me segmentation fault.

#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

class A
{
    private :

        int n;
        int m;
        int **array;

    public :

        A();
        A(int nn, int mm);
        A(const A& a);
        A& operator=(A a);
        int getAt(int i, int j);
        int getN(){return n;}
        int getM(){return m;}
        ~A();
};

A::A()
{
    n = 0;
    m = 0;
    array = NULL;
}

A::A(int nn, int mm)
{
    n = nn;
    m = mm;

    array = new int*[n];
    for(int i = 0; i <n; i++)
        array[i] = new int[m];

    for(int i=0; i<n; i++)
        for(int j=0; j<m; j++)
            array[i][j] = rand()%2;
}

A::A(const A& a)
{
    for(int i=0; i <n; i++)
        delete[] array[i];
    delete [] array;
    array = NULL;

    n = a.n;
    m = a.m;

    array = new int*[n];
    for(int i = 0; i <n; i++)
        array[i] = new int[m];

    for(int i=0; i<n; i++)
        for(int j=0; j<m; j++)
            array[i][j] = a.array[i][j];
}

A& A::operator=(A a)
{
    for(int i=0; i <n; i++)
        delete[] array[i];
    delete [] array;
    array = NULL;

    n = a.n;
    m = a.m;

    array = new int*[n];
    for(int i = 0; i <n; i++)
        array[i] = new int[m];

    for(int i=0; i<n; i++)
        for(int j=0; j<m; j++)
            array[i][j] = a.array[i][j];

    return *this;
}

int A::getAt(int i, int j)
{
    return array[i][j];
}

A::~A()
{
    for(int i=0; i <n; i++)
        delete[] array[i];
    delete [] array;
    array = NULL;
}

class B
{
    private :

            A a;

    public :

            B(){}
            B(A aa);
            void setA(A aa);
            void showA();
};

B::B(A aa)
{
    a = aa;
}

void B::setA(A aa)
{
    a = aa;
}

void B::showA()
{
    int n = a.getN();
    int m = a.getM();

    for(int i=0; i<n; i++){
        for(int j=0; j<m; j++)
            cout << a.getAt(i,j) << " ";
        cout << "\n";
    }
    cout << "\n";
}

int main()
{
    A a(3, 4);
    B b(a);

    b.showA();

    return 0;
}

How to fix this? And why did it occur?

SOLVED:

#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

class A
{
    private :

        int n;
        int m;
        int **array;

    public :

        A();
        A(int nn, int mm);
        A(const A& a);
        A& operator=(const A& a);
        int getAt(int i, int j);
        int getN(){return n;}
        int getM(){return m;}
        ~A();
};

A::A()
{
    n = 0;
    m = 0;
    array = NULL;
}

A::A(int nn, int mm)
{
    n = nn;
    m = mm;

    array = new int*[n];
    for(int i = 0; i <n; i++)
        array[i] = new int[m];

    for(int i=0; i<n; i++)
        for(int j=0; j<m; j++)
            array[i][j] = rand()%2;
}

A::A(const A& a)
{
    n = a.n;
    m = a.m;

    array = new int*[n];
    for(int i = 0; i <n; i++)
        array[i] = new int[m];

    for(int i=0; i<n; i++)
        for(int j=0; j<m; j++)
            array[i][j] = a.array[i][j];
}

A& A::operator=(const A& a)
{
    for(int i=0; i <n; i++)
        delete[] array[i];
    delete [] array;
    array = NULL;

    n = a.n;
    m = a.m;

    array = new int*[n];
    for(int i = 0; i <n; i++)
        array[i] = new int[m];

    for(int i=0; i<n; i++)
        for(int j=0; j<m; j++)
            array[i][j] = a.array[i][j];

    return *this;
}

int A::getAt(int i, int j)
{
    return array[i][j];
}

A::~A()
{
    for(int i=0; i <n; i++)
        delete[] array[i];
    delete [] array;
    array = NULL;
}

class B
{
    private :

            A a;

    public :

            B(){}
            B(A aa);
            void setA(A aa);
            void showA();
};

B::B(A aa)
{
    a = aa;
}

void B::setA(A aa)
{
    a = aa;
}

void B::showA()
{
    int n = a.getN();
    int m = a.getM();

    for(int i=0; i<n; i++){
        for(int j=0; j<m; j++)
            cout << a.getAt(i,j) << " ";
        cout << "\n";
    }
    cout << "\n";
}

int main()
{
    A a(3, 4);
    B b(a);

    b.showA();

    return 0;
}

You need to define your won copy constructor and assignment operator that do the right thing with the array. See http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — Michael Burr, May 10 '13 at 09:22
You need to follow the [rule of three](http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29). — juanchopanza, May 10 '13 at 09:22
@LightnessRacesinOrbit: not for me. Added copy constructor and assignment operator to A class but still have seg fault. — yak, May 10 '13 at 09:31
@Yak: What debugging did you perform? This question has been asked 500,000,000 times on Stack Overflow -- please go look at one of the existing posts and in future search before posting. — Lightness Races in Orbit, May 10 '13 at 09:33
@yak: In fact, something did change (though there's still a segfault - it's a different one). Your copy constructor and assignment operators are bugged. As Lightness Races in Orbit suggests, a single debug session should point out the problem right away. — Michael Burr, May 10 '13 at 09:44

score 2 · Accepted Answer · answered May 10 '13 at 09:34

2

Your copy constructor first overwrites the old n with the new n, then attempts to delete n elements of the old array. But what if the arrays are of different sizes? Overwrite n after the loop.

Or, better still, get rid of this horrid pointer monstrosity and use std::vector instead.

answered May 10 '13 at 09:34

Lightness Races in Orbit

378,754
76
643
1,055

hm, nothing changed really. Run valgrind: http://pastie.org/private/gvbtdoqmotwc2fi1bjweg. Still dont know where the problem is, changed what you said but still - seg fault. – yak May 10 '13 at 09:54

score 1 · Answer 2 · answered May 10 '13 at 09:29

First, you are violating the "rule of three" (in any class that allocates memory, you need to have a copy constructor and assignment operator that "deals" with the allocated memory in an appropriate way).

Your destructor is also broken in the case where the "array" is NULL - you need to check that array is not NULL before running the loop to delete the inner parts.

Seg fault when deallocating memory in destructor

2 Answers2