C++ : Crashing while filtering the node values

Question

Below code is a simulation of a problem which I am facing in my project code. In this code, I have a vector in which I have some values. Now, I want to filter out some values from this original vector based on some criteria and store them in a new vector.

Main catch here is the pointer variable in the "node". I am even doing deep copy in order to avoid double free(). But even then it is giving the same exception.

The code:

#include <iostream>
#include <vector>

using namespace std;

class node
{
    public:

    int a;
    double *ptr;

    node()
    {
        a = 0;
        ptr = NULL;
    }

    ~node()
    {
        if(ptr)
        {
            delete [] ptr;
        }
    }
};

int main()
{
    vector <node> original(10);
    cout << "Filling values in Original Copy : " << endl ;

    for(int i=0; i<10; i++)
    {
        original[i].a = 0;
        original[i].ptr = new double[20];
    }

    vector <node> mod2;
    cout << "Finding Nodes with value mod 2 : "  << endl ;

    for(int i=0; i<10; i++)
    {
        if(original[i].a%2 == 0)
        {
            node temp;
            temp.a = original[i].a;

            // Deep Copy
            temp.ptr = new double[20];    
            for(int j=0; j<20; j++)
            {
                temp.ptr[j] = original[i].ptr[j];
            }

            mod2.push_back(temp);
            temp.ptr = NULL; // To avoid memory deallocation
        }        
    }


   return 0;
}

why dont you use a vector or some std data structure instead of `ptr` — UmNyobe, Dec 27 '13 at 09:38
@UmNyobe: Actually, It is an existing code. I am just adding a new functionality to the code that is why I can't change the node structure. — Vasu, Dec 27 '13 at 09:46

score 2 · Answer 1 · edited May 23 '17 at 12:19

2

The node class violates the rule of three. You need to add a copy constructor and a copy assignment operator.

Without this, whenever an instance of node is copied around (as is routinely done by std::vector<node>), your code ends up double-deleting memory.

The hack of manually setting temp.ptr to NULL after adding tmp to the vector helps somewhat. However, this is neither a clean solution nor a robust one (since it doesn't take care of all circumstances when this needs to be done).

edited May 23 '17 at 12:19

Community

1
1

answered Dec 27 '13 at 09:34

NPE

486,780
108
951
1,012

1

For avoiding this, I am doing `temp.ptr = NULL;` – Vasu Dec 27 '13 at 09:38
Even then it is crashing. – Vasu Dec 27 '13 at 09:44

score 0 · Accepted Answer · edited Jun 20 '20 at 09:12

Each time you are pushing and element into the mod2 vector, it is increasing its size. In order to do that it is destructing the previously held memory and allocating a new chunk to it. While doing that it is calling the default copy constructor which is by default doing the shallow copy.

You can solve this problem by:

Overloading the default copy constructor
Reserving the size for mod2 in advance (as you know it can't be bigger than original vector)

EDIT: You can also check the rule of three as metioned by @NPE.

C++ : Crashing while filtering the node values

2 Answers2