My templated queue's dequeue function works fine for a queue of string, but if I use my custom Robot class, it crashes upon trying to delete the pointer. I'm curious as to why.
For example, in main.cpp
#include <iostream>
#include <cstdlib>
#include <cstring>
#include "robotqueue.h"
#include "robotcustomer.h"
#include "servicestation.h"
using namespace std;
int main()
{
//- TEST ONE: QUEUE<STRING> -//
RobotQueue < string > stringQueue;
string a("Tim");
string b("Greg");
stringQueue.enqueue(a);
stringQueue.enqueue(b);
stringQueue.dequeue();
stringQueue.dequeue();
//- TEST TWO: QUEUE<RobotCustomer> -//
RobotQueue < RobotCustomer > robotQueue;
RobotCustomer e("Tim",3);
RobotCustomer f("Greg",5);
robotQueue.enqueue(e);
robotQueue.enqueue(f);
robotQueue.dequeue(); <--- Segfault occurs here
robotQueue.dequeue();
return 0;
}
the string queue works fine, but I get this error:
***Error in `q': munmap_chunk(): invalid pointer: 0x0000000001d6c108 ***
Aborted (core dumped)
My templated queue looks about like this (dunno if this ya need more).
robotqueue.hpp
// Default Constructor
template <typename T>
RobotQueue<T>::RobotQueue()
{
m_size = 0;
m_front = NULL;
m_back = NULL;
}
// Default destructor
template <typename T>
RobotQueue<T>::~RobotQueue()
{
Node<T>* currNode = m_front, *nextNode = NULL;
while ( currNode != NULL )
{
nextNode = currNode->m_next;
delete currNode;
currNode = nextNode;
}
m_size = 0;
}
template <typename T>
void RobotQueue<T>::enqueue(const T& x)
{
Node<T> *newNode = new Node<T>;
newNode->m_data = x;
newNode->m_next = NULL;
if(m_front == NULL)
m_front = newNode;
else
m_back->m_next = newNode;
m_back = newNode;
m_size++; // Increments queue size
return;
}
template <typename T>
void RobotQueue<T>::dequeue()
{
Node<T>* tempNode = new Node<T>;
if(m_front == NULL)
cout << "dequeue error: Queue is empty" << endl;
else
{
tempNode = m_front;
m_front = m_front->m_next;
delete tempNode; <-- Segfault occurs here in RobotCustomer class
m_size--; // Increments queue size
}
return;
}
I'm assuming it has to do with RobotCustomer being a class so m_data can't point to it or something? Not an expert here :p
RobotCustomer.h
/* ------------------ Class RobotCustomer ------------------ */
class RobotCustomer
{
private:
string m_name; // Name of Robot
string* m_reqServices; // Array of services requeseted
int m_numServices; // Number of services requested
int m_currService; // Number of services compelted
bool m_busy; // Logic for if robot is in line/servicing
bool m_done; // Logic for if robot is done
public:
//- functions and such that I don't think affect the queue -//
Thanks for your time :)
---------------------UPDATED_WITH CONSTRUCTORS/DECONSTRUCTORS-------------------
RobotCustomer.cpp
// Default Constructor
RobotCustomer::RobotCustomer()
{
m_name = "";
m_numServices = 0;
m_currService = 0;
m_busy = false;
m_done = false;
}
// Overloaded Constructor
RobotCustomer::RobotCustomer(string n, int x)
{
m_name = n;
m_numServices = x;
m_reqServices = new string[m_numServices];
m_currService = 0;
m_busy = false;
m_done = false;
}
// Default Destructor
RobotCustomer::~RobotCustomer()
{
delete m_reqServices;
}
Lets look at a few of the lines in your function:
Node<T>* tempNode = new Node<T>;
tempNode = m_front;
delete tempNode;
First you allocate memory and assign its pointer to the tempNode
variable. Then you reassign that variable to point to some other memory, losing the original pointer. Then you try to free the memory, which is no longer the original memory you allocated.
This should not cause the crash (as fas as I can see), but it is a memory leak.
My guess as to what is causing the crash is that you probably allocate memory dynamically in the RobotCustomer
constructor, for the m_reqServices
member. But if you don't implement a copy-constructor or copy-assignment operator, or just do a shallow copy of the pointer in those functions, then with the assignment
newNode->m_data = x
in the enqueue
function you will have two object with the same pointer, and when one of them is deleted its destructor deletes the allocated memory, leaving the other objects pointer now pointing to unallocated memory, leading to undefined behavior when you try to free it again.
You need to read about the rules of three, five and zero. My recommendation is to use std::vector
or std::array
(as applicable) and simply follow the rule of zero.
Of course, talking about using the standard containers makes me wonder why you don't use std::queue
to begin with?