Pointer Being Freed Was Not Allocated Error!

[Crthomas]

Hi there,

I have a program which creates a stack of doubles. in my test driver I needed to create another stack and copy the first one into it. if I do the 6 and 7 choice menu that is in my main file, when I exit, I get an error. Probably because the second stack didn't have memory allocated for it? Please help a desperate beginner code :(

My .cpp file

#include <iostream>
#include <stack>
#include <string>
#include "../include/DoubleStack.h"

using namespace std;

//Constructor
DoubleStack::DoubleStack(size_t capacity) {

	data = new double[capacity];
	stackSize = capacity;
	topOfStack = 0;
}

//Copy Constructor
DoubleStack::DoubleStack(const DoubleStack& rhs):stackSize(this->capacity()) {
	delete this;
	if (rhs.topOfStack!=0) {
		for (int i = 0; i < this->capacity(); i++) {
			push(rhs.data[i]);
		}
	}
}

//Destructor
DoubleStack::~DoubleStack(void) {
	delete data;
	// ** YOUR CODE GOES HERE **
	// ** About 2 lines of code **
}

// equal operator
DoubleStack& DoubleStack::operator= (DoubleStack& rhs) {

	this->data = rhs.data;
	this->stackSize = rhs.stackSize;
	return *this;
}

//Adds a double to top of stack
void DoubleStack::push(double& item) {
	
	if (topOfStack == stackSize){
		return;
	} else{
		data[topOfStack] = item;
		topOfStack++;		
	}
}

//Removes top value from stack
double DoubleStack::pop (double& item) {
	
	if (topOfStack == 0) {
		return 0;
	} 
	topOfStack--;
	return data[topOfStack]; // Replace with appropriate return statement

}

//Determines if stack is empty or not
int DoubleStack::empty(void) {
	if (topOfStack == 0) return 1;  
	else return 0;
}

//Returns the capacity of the stack
size_t DoubleStack::capacity(void) {

	return stackSize;
}

//Returns the number of data entries in the stack
size_t DoubleStack::size(void) {
	
	if (empty())	return 0;
	else if ( topOfStack > 0)	return (topOfStack);
}


//The == comparasion operator
int DoubleStack::operator==( DoubleStack& rhs) {
	for (int i = 0 ; i < this->size(); i++) {
		if (this->data[i]== rhs.data[i]) return 1;
		else return 0;
	}
}

//Displays the stack, one value per line
void DoubleStack::dispstk(void) {
	
	if (empty()) std::cout << "Stack is empty :( " << '\n';

	for (int i = 0 ; i < topOfStack; i++)
		std::cout << this->data[i] << '\n';
    
}

size_t DoubleStack::getTos() {


	return topOfStack;
}

double DoubleStack::displayTopVal() {
	if ( topOfStack == 0 )	return 0;
	else	return data[topOfStack-1];
}

my main.cpp file and tester:

#include "../include/DoubleStack.h"
#include <iostream>
using namespace std;

int main(void) {

	DoubleStack ds1(50), ds2(50);
	double x;
	int choice, end;
	end = 1;
	
	// Loop through choices until quit is choosen
	while(end == 1) {

		//Display choices
		cout <<  "\nDEVELOPMENT PROGRAM - PLEASE CHOOSE FROM THE FOLLOWING MENU \n\n";   
		cout <<  "1. Display stack 1" << "\n";
		cout <<  "2. Place a double value onto the top of stack 1" << "\n";
		cout <<  "3. Remove a value from the top of stack 1" << "\n";
	    cout <<  "4. Check the total capacity stack 1" << "\n";
		cout <<  "5. Check current number of items on stack 1" << "\n";
		cout <<  "6. Copy stack 1 to stack 2" << "\n";
		cout <<  "7. Check to see if the two stacks are equal" << "\n";
		cout <<  "8. Quit" << "\n";
		cout << "Enter your choice: ";
		cin >> choice;
		switch(choice) {
			case 1:	//Displays stack 1			
				ds1.dispstk();
				break;
			case 2: //adds new double to stack 1
				cout << "Enter Double: ";
				cin >> x;
				ds1.push(x);
				break;
			case 3: //pops top value of stack 1
				ds1.pop(x);
				cout << "the new top value is: " << ds1.displayTopVal() << endl;
				break;
			case 4: //Displays total capacity of stack 1
				cout << "the capacity of the stack is: " << ds1.capacity() << endl;
 				break;
			case 5: //Displays current items in stack 1
				cout << "Current items in the stack: " << ds1.size() << endl;
				break;
			case 6: // Copies stack 1 to stack 2
				ds2 = ds1;
				cout << "stack 1 was copied into stack 2. " << endl;
				break;
			case 7: //Checks to see if stack 1 and 2 are the same
				if ( ds1 == ds2) cout << "Stack 1 and stack 2 are the same :) " << '\n';
				else cout << "Stack 1 and stack 2 are NOT the same. " << '\n';
				break;
			case 8: //Ends program
				return 0;
				break;
		}  // End of switch statement
	}  // End of while loop
	
}

when I click choice 8 -- ONLY AFTER DOING CHOICE 6 & 7 (copies one stack into another) -- do I get the following error saying a pointer was freed incorrectly:

Enter your choice: 6
stack 1 was copied into stack 2. 

DEVELOPMENT PROGRAM - PLEASE CHOOSE FROM THE FOLLOWING MENU 

1. Display stack 1
2. Place a double value onto the top of stack 1
3. Remove a value from the top of stack 1
4. Check the total capacity stack 1
5. Check current number of items on stack 1
6. Copy stack 1 to stack 2
7. Check to see if the two stacks are equal
8. Quit
Enter your choice: 7
Stack 1 and stack 2 are the same :) 

DEVELOPMENT PROGRAM - PLEASE CHOOSE FROM THE FOLLOWING MENU 

1. Display stack 1
2. Place a double value onto the top of stack 1
3. Remove a value from the top of stack 1
4. Check the total capacity stack 1
5. Check current number of items on stack 1
6. Copy stack 1 to stack 2
7. Check to see if the two stacks are equal
8. Quit
Enter your choice: 8
stack(493,0x7fff79c81300) malloc: *** error for object 0x7ffb9b500100: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6
 

[Peter87]

This is wrong delete this;

[ne555]

> #include "../include/DoubleStack.h"
you forgot to provide that file.

DoubleStack::DoubleStack(size_t capacity) {
	data = new double[capacity];
	stackSize = capacity;
	topOfStack = 0;
}
//Destructor
DoubleStack::~DoubleStack(void) {
	delete data;
	// ** YOUR CODE GOES HERE **
	// ** About 2 lines of code **
}

if allocate with new, deallocate with delete
if allocate with new[], deallocate with delete[]
Otherwise, undefined behaviour.

//Copy Constructor
DoubleStack::DoubleStack(const DoubleStack& rhs):
   stackSize(this->capacity()) // ¿what do you think you are doing here? your object is not yet constructed, ¿how do you expect to call a member function?
{
	delete this;  //¡¿?! suicide is not a good idea
	if (rhs.topOfStack!=0) {
		for (int i = 0; i < this->capacity(); i++) {
			push(rhs.data[i]); //this->data is uninitialised
		}
	}
}

// equal assignment operator
DoubleStack& DoubleStack::operator= (DoubleStack& rhs) { //if you are not going to modify your parameters, declare them const
	this->data = rhs.data; //a shallow copy
	this->stackSize = rhs.stackSize;
	return *this;
}

[naraku9333]

In addition to what the others have mentioned, your operator== is only checking if the first elements of the arrays are equal.

[Crthomas]

naruka9333 - is my operator== now okay? its going through capacity:

int DoubleStack::operator==( DoubleStack& rhs) {
	for (int i = 0 ; i < this->capacity(); i++) {
		if (this->data[i]== rhs.data[i]) return 1;
		else return 0;
	}
}

ne555 - can you elaborate on how to initialize data in my copy constructor?

[ne555]

When i=0 you compare this->data[0] against rhs.data[0].
Whatever the result may be, you immediately return from the function, so `i' never gets increased and no other element is tested.


To initialise data

DoubleStack::DoubleStack(const DoubleStack& rhs):
   data( new double[rhs.size()] )

[Crthomas]

Thank you so much y'all!!! Everything works perfectly now and I am a happy person once again!!!!!

xoxo