CODEWITHSUNDEEP
c++memory
cipher
cipher

Reputation: 2484

"double free or corruption" error in C++

I was doing some sort of infix-to-postfix conversion.

My program works fine but at the end it gives some sort of error!

Error Info

I guess the error is on the void printQueueContents(Queue typeInfo) method of following class

template <class T>
class Queue{
private:
    int front, rear, max;
    T *q;
public:
    Queue(int size=0){
        front = rear = 0;
        max = size;
        q = new T[size];
    }
    ~Queue(){
        delete []q;
    }
    int enqueue(T);
    T dequeue();

    T getValueFromIndex(int index){
        return q[index];
    }

    void printQueueContents(Queue typeInfo){
        for(int i=rear; i<front;i++){
            if(typeInfo.getValueFromIndex(i)) {
            cout<<q[i]<<" ";
            } else {
                cout<<static_cast<char>(q[i])<<" ";
            }
        }
        cout<<"\n";
    }
};

which is called as: 

q1.printQueueContents(q2);

And declared as:

Queue<int> q1(200);
Queue<int> q2(200); // Holds data type information for q1

I cannot figure out why is this happening? Any suggestions?

EDIT: Thanks to peoples with answers. I finally managed to solve it by changing the method to: 

void printQueueContents(Queue& typeInfo)

so that while calling this function the default copy constructor does not make a copy and destructor does not accidently deletes the same memory location twice. Here's a link that has a intuitive explaination. http://www.drdobbs.com/c-made-easier-the-rule-of-three/184401400

Upvotes: 0

Views: 983

Answers (2)

Eric Fortin
Eric Fortin

Reputation: 7603

While it is true that since you declared a destructor you should declare copy assignment operator and copy constructor(rule of three). You might not want to have them public though.

Calling a function that takes your class by value will invoke the copy constructor. You rely on the compiler generated copy constructor if you don't declare any and it does member to member assignment. In your case, the q* gets copied as is to your second object. When returning from the function, the parameter passed by value goes out of scope and get destructed which will delete q for the first time. When your program exits, the destructor of the original object gets called as well so q is deleted a second time.

In your case, I think the printQueueContents method should take a const &(or ref) to your Queue instead to avoid (possibly) expensive copy of the Queue object. Same could be said for getValueFromIndex that should return a const & or ref to avoid copy. For int this is not a problem but for classes, it could get expensive quickly.

With that being said, you still have to either implemented copy assignment operator and copy constructor or declared them as private.

Upvotes: 2

marcinj
marcinj

Reputation: 49986

You must implement copy constructor and copy assignment operator

Queue(const Queue& rop){
 // copy rop to this, means allocate local q buffer, and copy contents of rop.q to it
}

Queue& operator=(const Queue& rop){
 // update this with rop, means the same as above
 return *this;
}

your class must obey rule of three, mostly because your class is using bare pointer to allocated buffer. You could also use shared_ptr for your buffer, it would manage life time of it.

Upvotes: 1

Related Questions