C++

Forum

 
pointer being freed was not allocated

ygh929 (4)
Hi I'm trying to make a queue class by myself, and inside the queue, each element is a position (pair of integers).
So here is the code for "position.h"
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
 
#include <iostream>
using namespace std;


class position{
private:
	int *T;
public:
	explicit position( int a=-1, int b=-1){
		T=new int[2];
		T[0]=a;
		T[1]=b;
	};
	
	~position(){delete []T;};
	void set(const int &a,const int& b){
		T[0]=a;
		T[1]=b;};
	int x(){return T[0];};
	int y(){return T[1];};
	
	position & operator= (const position &p){
		if (this != &p){
			T[0]=p.T[0];
			T[1]=p.T[1];}
		return (*this);
	};
	
	bool & operator== (const position &p){
		bool temp;
		temp=T[0]==p.T[0]&T[1]==p.T[1];
		return temp;};

};


Here is the code for the queue: "positionqueue.h"

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
 
#include "position.h"
#include <iostream>
using namespace std;

class positionqueue { 
	//define a queue such that each element 
	//in it is the position of a point
	
private:
	int head; //front element index
	int l;//max length of the array
	int rear; // rear element index
	position *S;// the array we use to construct queue
	position *Snew;//used when extend the queue
	
public:
	positionqueue(int maxlength=100){
		l=maxlength;
		S=new position[l];
		
		head=rear=0;
	};
	
	
	~positionqueue(){
		delete [] S;
	};
	
	
	bool IsEmpty() const{return head==rear;};
	
	int size() const{
		int temp;
		temp= (rear-head+l)%l;

		cout <<"head:"<<head <<" arraylength:"<<l<< " rear:"<<rear<<" queuelength:"<<temp<<"\n";
		return temp;};
	
	 position front() const{
		return S[head];};
	
	position back() const{
		return S[rear-1];};
	
	void pop(){
		if (head==rear){
			cout << "the queue is empty!\n";}
		else{
			head=head+1;}
	};
	
	
	void push(const position& newvalue){
		int size=(rear-head+l)%l;
		if (size==l-1){
			
			Snew= new position [l*2];
			for(int i=0;i<l;i++){
				int index=(head+i+l)%l;
				Snew[i]=S[index];
	
			}
			
			*S=*Snew;
			head=0;
			rear=l-1;
			l=l*2;
		}
		S[rear]=newvalue;
		rear=(rear+1+l)%l;
		
	};
};


After finishing this, I found that when ever I use positionqueue.front(), or back(), there would be error like this:

test(15092) malloc: *** error for object 0x1001000b0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug


Also when the queue is full, and we need to double the length, there will be error like this:

Program received signal:  “EXC_BAD_ACCESS”.
sharedlibrary apply-load-rules all


However, both of them can show all the results when I run them. I used this main to test these two headers:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
#include "positionqueue.h"

using namespace std;

int main () {
	
	positionqueue a(5);
	
	a.front(); //this is used to test the first problem	
	
	 /* //this is used to test the second problem

         position b(1,2);
	 for (int i=0;i<10;i++){
		a.push(b);
		a.size();
		if(i%2==0){a.pop();
			a.size();
		}}*/

	
	return 0;
}


A cout line is in position::size() so that it can help us see the results.

By trial and error, I found that if I delete either line 67 or 69 or assign a value smaller than l to l in line 67 in "positionqueue.h", there will be no "bad access" problem.

I'm a beginner and these kind of errors are really strange to me.... Are there any one would help me? I would appreciate so much for your help!

ne555 (10692)
If you need a destructor, a copy constructor or an assignment operator, you need the three.

However, ¿why are you using dynamic allocation in position?
cire (8284)
It should be fairly obvious that you cannot use front() or back() on an empty queue, which a is in your code above. It wouldn't hurt to modify those methods so they aren't accessing memory they shouldn't.

1
2
3
4
5
6
7
8
9
10
11
 
	 position front() const{
                if ( isEmpty() )
                    return position() ;
		return S[head];
         }
	
	position back() const{
                if ( isEmpty() )
                    return position() ;
		return S[rear-1];
        }



This is a very naive implementation. The queue just grows and grows and grows if you keep popping and pushing stuff, even if you don't ever have more items in the queue than you did memory originally allocated.

head, l, and rear should probably be unsigned types.

You have a couple serious issues in position::operator==, the major one being that you're returning a reference to a variable that goes out of scope when the function ends.

Both front and back return copies of the type position. Since position doesn't have a custom copy constructor and member-wise copying won't do the trick, that is a problem.


Last edited on
ygh929 (4)
Thank you so much for your help! I've used another easier method to solve this problem since I'm really new to c++...
Topic archived. No new replies allowed.