Question about free?

**c9dw2rm8** · 03-08-2008

Hello

I should build some kind of chess game
i build struct of piece and board -array 8*8 that it's cell is Piece
when i move tool in the game i'm trying to free this place, but it's failed and show me problem about heap?

Somebody know the reason for that?

**Banana Man** · 03-08-2008

What does your code look like? I'd guess that you're accidentally freeing memory twice.

**c9dw2rm8** · 03-08-2008

Code:

	if (Board[fromRow][fromCol]->Move(fromRow,fromCol,toRow,toCol)==TRUE){
		Board[toRow][toCol]=CreatePiece(Board[fromRow][fromCol]->color,Board[fromRow][fromCol]->tool,Board[fromRow][fromCol]->Move,Board[fromRow][fromCol]->Print);
		pi=Board[fromRow][fromCol];
		free(pi);

this release the cell that the piece move from

the definition of the board is

Code:

Piece Board[N][N];

the definition of piece is :

Code:

struct PieceType{
	enum Color color;
	enum Tool tool;
	MovePiece Move;
	PrintPiece Print;
};

typedef struct PieceType *Piece;

The first initialization of the board

Code:

void InitBoard(){
int i,j;
	for (i=0;i<N;i++)
		for (j=0;j<N;j++)
			Board[i][j]=NULL;
		
}

You see any problem?

**matsp** · 03-08-2008

Not an answer to your question, but:

Code:

		Board[toRow][toCol]=CreatePiece(Board[fromRow][fromCol]->color,Board[fromRow][fromCol]->tool,Board[fromRow][fromCol]->Move,Board[fromRow][fromCol]->Print);
		pi=Board[fromRow][fromCol];

I would create a temporary pointer to Board[fromRow][fromCol] so that you avoid all those long dereferences.

--
Mats

**Banana Man** · 03-08-2008

You see any problem?

I don't, sorry. Can you make a small and complete program that crashes? That way I can run it and play with it under the debugger.

**Adak** · 03-08-2008

My chess program is very rudimentary, so this may not apply to yours:

Why initialize the board to null? Don't you want the starting position of the pieces?
Why are you freeing a square on the board? Set the value to 0 or empty and you're done.

I'd be amiss in not mentioning http://TalkChess.com//
In the programming section you can find some of the most talented chess programmers in the world. Robert Hyatt is one to pay particularly close attention to.

**c9dw2rm8** · 03-08-2008

Originally Posted by Adak

My chess program is very rudimentary, so this may not apply to yours:

Why initialize the board to null? Don't you want the starting position of the pieces?
Why are you freeing a square on the board? Set the value to 0 or empty and you're done.

I'd be amiss in not mentioning http://TalkChess.com//
In the programming section you can find some of the most talented chess programmers in the world. Robert Hyatt is one to pay particularly close attention to.

In this work we should practice Free and Malloc functions
but what is the problem and how it's connected to the heap?

**c9dw2rm8** · 03-08-2008

Originally Posted by matsp

Not an answer to your question, but:

Code:

		Board[toRow][toCol]=CreatePiece(Board[fromRow][fromCol]->color,Board[fromRow][fromCol]->tool,Board[fromRow][fromCol]->Move,Board[fromRow][fromCol]->Print);
		pi=Board[fromRow][fromCol];

I would create a temporary pointer to Board[fromRow][fromCol] so that you avoid all those long dereferences.

--
Mats

I always save those kind of things to the end
You now the last finishes
But did you find any problem
Maybe because its array ?
if you do free to cell in this array ,maybe it's make problems ,??

**Adak** · 03-08-2008

Originally Posted by c9dw2rm8

In this work we should practice Free and Malloc functions
but what is the problem and how it's connected to the heap?

I can't understand why you'd malloc or free a square, or a piece on a square, of a chessboard.

Let's be clear - just because a square has no piece on it, (or a zero point value on it), at that moment, doesn't mean that it should be free'd (and why should it be malloc'd?

It's an array element, not a node on a linked list. You don't just malloc a new array element, one at a time, and you shouldn't try to free one, either.

You can malloc the whole array, and then free the whole array, but not an individual part of that array.

**anon** · 03-09-2008

That was what was hard for me to understand too.

Even if your array of pointers holds pointers to pieces that have been malloc'ed initially, you could just copy the pointer over to a new location in the array, and set the pointer in the old location to NULL.

You'll free the memory after the game(s) are finished (or perhaps when pieces are captured but in this case may-be you'd want to display the captured pieces beside the board).

You can move around the pointer itself in the array without needing to touch the struct (malloc'ed memory) it points to at all.

**c9dw2rm8** · 03-09-2008

Originally Posted by Adak

I can't understand why you'd malloc or free a square, or a piece on a square, of a chessboard.

Let's be clear - just because a square has no piece on it, (or a zero point value on it), at that moment, doesn't mean that it should be free'd (and why should it be malloc'd?

It's an array element, not a node on a linked list. You don't just malloc a new array element, one at a time, and you shouldn't try to free one, either.

You can malloc the whole array, and then free the whole array, but not an individual part of that array.

But every cell from the array is a Piece* type
and Piece is struct of (color,tool,movefunction,printfunction)
so every time i put a new tool on the board i should do malloc
where is the problem?

**matsp** · 03-09-2008

Originally Posted by c9dw2rm8

I always save those kind of things to the end
You now the last finishes
But did you find any problem
Maybe because its array ?
if you do free to cell in this array ,maybe it's make problems ,??

It would probably help if you show us the code that ALLOCATES the content of each position.
Also a data type declaration for Piece may help give us some ideas. I don't see anything wrong in the code posted.

As to the "save it for the last finish", that has several consequences:
1. Your current code is hard to read. Using a simpler form to access the data would help by making the code easier to read [e.g. it fits on a decent size monitor, rather than the last bit missing off the right edge of the window].
2. You never get to do all the "last finishes" anyways. There's usually too many other things to attend to at the "end of the project".

--
Mats

**King Mir** · 03-09-2008

Originally Posted by c9dw2rm8

Code:

	if (Board[fromRow][fromCol]->Move(fromRow,fromCol,toRow,toCol)==TRUE){
		Board[toRow][toCol]=CreatePiece(Board[fromRow][fromCol]->color,Board[fromRow][fromCol]->tool,Board[fromRow][fromCol]->Move,Board[fromRow][fromCol]->Print);
		pi=Board[fromRow][fromCol];
		free(pi);

this release the cell that the piece move from

Where do you check if the board position is null?

Also, freeing the pointer will invalidate the value of Board[fromRow][fromCol]. You should probably set that to null.

**matsp** · 03-10-2008

And I just realized the whole section can be rewritten as:

Code:

        pi = Board[fromRow][fromCol];
	if (pi->Move(fromRow,fromCol,toRow,toCol)==TRUE){
		Board[toRow][toCol]=CreatePiece(pi->color,pi->tool,pi->Move,pi->Print);
		free(pi);

Don't you think that's a bit easier on the eyes?

Again, no, it doesn't fix whatever the problem is in your code, and King Mir's suggestion to add

Code:

 Board[fromRow][fromCol] = NULL;

would be a good one.

--
Mats

Thread: Question about free?

Thread Tools

Search Thread

Display

Question about free?

Similar Threads

Design question

Simple question about free()

question about free()

SIGABRT upon free()

Very simple question, problem in my Code.