Freeing Memory From A Function?

[Uprising]

Hi
I am not sure how i can properly pass a pointer that has been allocated memory to another function and then free it.

What i am doing currently is this

struct bullet *plasmaStart = NULL;
struct bullet *plasmaCurrent = NULL;
struct bullet *plasmaNew = NULL;
struct bullet *plasmaTemp = NULL;
int bulletNumP = 0;

main()
{
	code...

	freeBullets(plasmaStart,&bulletNumP);
}

int freeBullets(struct bullet *bullet,int *bullets)
{
	struct bullet *bulletPrevious;
	struct bullet *bulletTemp;
	struct bullet *bulletCurrent;
	int x;

	bulletCurrent = bulletTemp = bullet;
	for(x = 0; x < *bullets; x++)
	{
		bulletTemp = bulletCurrent->next;
		if(bulletCurrent->destroyed == 1 || bulletCurrent->framesMoved  >= bulletCurrent->maxDistance)
		{
			if(bulletCurrent == bullet)
				bullet = bulletCurrent->next;
			else
				bulletPrevious->next = bulletCurrent->next;
			free(bulletCurrent);
			*bullets = *bullets - 1;
			x --;
		}
		else
		{
			bulletPrevious = bulletCurrent;
		}
		bulletCurrent = bulletTemp;

	}

	return 0;
}

I keep getting the error
***glibc detected *** double free or corruption (fasttop): 0x001fc930 ***
Aborted

However if i change the function so that it just uses the original global pointers instead of passing them to the function.
*plasmaStart;
*plasmaCurrent;
*plasmaTemp;
It works fine (atleast i think it does... :lol: ).

 

[gmiller1018]

If your goal is to free the element of a link list and then return the new "head" pointer in C you will need to pass the address of the head pointer so you can change it.

struct bullet **ppBullet;
struct bullet *pBullet;

struct bullet *plasmaStart = NULL;
struct bullet *plasmaCurrent = NULL;
struct bullet *plasmaNew = NULL;
struct bullet *plasmaTemp = NULL;
int bulletNumP = 0;

main()
{
	code...

	freeBullets(&plasmaStart,&bulletNumP);
}

int freeBullets(struct bullet **ppbullet,int *bullets)
{
	struct bullet *bulletPrevious;
	struct bullet *bulletTemp;
	struct bullet *bulletCurrent;
	struct bullet *bullet = *ppbullet;
	int x;

	bulletCurrent = bulletTemp = bullet;
	for(x = 0; x < *bullets; x++)
	{
		bulletTemp = bulletCurrent->next;
		if(bulletCurrent->destroyed == 1 || bulletCurrent->framesMoved  >= bulletCurrent->maxDistance)
		{
			if(bulletCurrent == bullet)
				bullet = bulletCurrent->next;
			else
				bulletPrevious->next = bulletCurrent->next;
			free(bulletCurrent);
			*bullets = *bullets - 1;
			x --;
		}
		else
		{
			bulletPrevious = bulletCurrent;
		}
		bulletCurrent = bulletTemp;

	}
	*ppbullet = bullet; // return new bullet pointer value

	return 0;
}

I am not sure that the pointer to bullet is what you want to return since this will not be the "head" of the list. The concept is there but I would not use this approach. I would make the freeBullets function return the new head pointer and then assign it to plasmaStart.

 

[Uprising]

Thanks for that!

A pointer to pointer was what i was looking for.
If you have to do this
*ppbullet = bullet; // return new bullet pointer value

what exactly does *bullet point to when you have assigned it originally?
struct bullet *bullet = *ppbullet;

 

[gmiller1018]

The assignment take the address of where the value in plasmaStart is sitting and moves that into the local "bullet" so in your example the value that *bullet will point to is NULL and the address in bullet is the address of plasmaStart.

English does not do justice to this but that is the way I would explain it.

for example:

int foo; // say it is stored at 0x60000
int *junk; // say it is stored at 0x50000
int **stuff; // say it is stored at 0x40000
	 // addresses are dumb but this is an example

junk = &foo; // now junk contains 0x60000 stored at address 0x50000

stuff = &junk; // now stuff contains 0x50000 stored at 0x40000

foo = 4;  put a value 4 into memory starting at address 0x60000

*(*stuff) is 4 chasing the pointers *(*(0x40000)), *(*0x50000), *(0x60000), 4

Of course this stepping through is a little screwed up but I wanted to show the tracking of the pointer.  I am sure someone will point out the flaw.

 

[gmiller1018]

Here is my take on the code, with the following assumptions:

1. All link lists end with a NULL pointer.
2. Your criteria was to determine that the node in the list should be deleted.

Code:

sstruct bullet *plasmaStart = NULL;
struct bullet *plasmaCurrent = NULL;
struct bullet *plasmaNew = NULL;
struct bullet *plasmaTemp = NULL;
int bulletNumP = 0;

main()
{
	code...

	plasmaStart = freeBullets(plasmaStart,&bulletNumP);
} 
 
struct bullet * 
freeBullets(struct bullet *bullet,int *bullets)
{
	struct bullet *bulletPrevious;
	struct bullet *bulletTemp;
	struct bullet *bulletCurrent;
	int x; 
 
	bulletCurrent = bulletTemp = bullet;
	while (bulletCurrent != NULL) {
		bulletTemp = bulletCurrent->next;
		if(bulletCurrent->destroyed == 1 || 
		   bulletCurrent->framesMoved  >= bulletCurrent->maxDistance) {
			if(bulletCurrent == bullet) // at the head 
				bullet = bulletCurrent->next; //  move head
			else 
				bulletPrevious->next = bulletTemp; // link around
			free(bulletCurrent); // free current node 
			*bullets = *bullets - 1; // not sure why this is needed .. 
		}
		else
		{
			bulletPrevious = bulletCurrent;
		}
		bulletCurrent = bulletTemp;

	}

	return bullet;
}

 

[Uprising]

You are right about the assumptions, i dont know why i decided to track bullets with another variable. :blink:

I think i see the problem now, but if the address of bullet is now the same as plasmaStart why do you need to return bullet after the function ends in order for it to work?

Is it because when you pass plasmaStart to the function it loses its address?

 

[gmiller1018]

Since the head of the link list might change if we delete item from the beginning of the list the contents for the plasma start pointer might change so we might need to reset it. Also we are passing the value of the pointer not where it is stores so we cannot change the the value stored in the pointer. Since 'C' is a pass by value language you are passing a copy of the value in the pointer not the pointer itself. Also by returning the pointer value we avoid having to do pointers to pointers which everyone screws up from time to time.

 

[Uprising]

copy of the value in the pointer not the pointer itself

This bit was what was confusing me, thanks a lot for your help Gary!

Also by returning the pointer value we avoid having to do pointers to pointers which everyone screws up from time to time.

Very true.