Thread: Still getting an error in deleting a node

Hi, its me again. I revised my code and tried to follow what you guys said already, but I'm still getting the stupid window popping out when I try to delete a node from a linked list. (i attached a pic if you're wondering what the window looks like)

I think the concept is right, but i dont understand why the window keeps popping out, is that normal? :\

Code:

void deleteNode(link *head, int input)
{
	link p;
	link dealloc;

	int match=0;

	if(*head==NULL)
	{
		printList(head);
		return;
	}

	p=*head;

	while(p!=NULL)
	{
		if(p->x==input)
			match++;
		p=p->next;
	}

	if(match>=1)
	{
	
	if((*head)->x == input)
	{	
		dealloc=*head;
		*head=(*head)->next;
		free(dealloc);
	}
	
	else
	{
		dealloc=*head;
		p=NULL;
		
		while(dealloc!=NULL && dealloc->x!=input)
		{
			p = dealloc;
			dealloc = dealloc->next;
		}

		p->next=dealloc->next;
		free(dealloc);
	}

	printList(head);

	}//end if

	else
	{
		printf("NUMBER NOT FOUND!");
		printList(head);

	}
}

You are probably not building your list correctly when you build it.

Here is my function for building the list...

Code:

void insert(link *head, int input)
{
	link temp;
	link curr;
	link p;

	temp=(link)malloc(sizeof(link));

	if(temp==NULL)
	{
		printf("Error in memory allocation");
		exit(0);
	}
	temp->x=input;
	temp->next=NULL;

	if(*head==NULL)
		*head=temp;
	else
	{
		p=*head;
		curr=p->next;

		if(input < p->x)
		{
			temp->next=*head;
			*head=temp;
		}
		else
		{
			while(curr!=NULL && input>curr->x)
			{
				p=p->next;
				curr=p->next;
			}

			p->next=temp;
			temp->next=curr;
		}		
	}	
	
	printList(head);
}

Is there anything wrong with it?

Lose all the *head stuff... you want to asign the pointer itself, not the content at the pointer.

Code:

temp=(link)malloc(sizeof(link));

As pointed out before, this is way wrong. If you are going to build a building, you need enough room to build the building, not enough room to build a sign that says "building over there".

Code:

temp=malloc(sizeof(*temp));

is what you want to do.

Damn, so that's what's making it wrong. I changed it to

Code:

temp=(link1*)malloc(sizeof(*temp))

;

since my compiler needs an explicit cast...Is that right? It works now...

Thank yoooouuu

Code:

link temp;
link curr;
link p;


temp=(link)malloc(sizeof(link));

It looks like you've defined link to be a pointer to your node structure. That means your malloc is wrong, or all 3 of those variables are the wrong type. I'm quite certain that it's the former, so your sizeof will be wrong. You're only getting sizeof(node *) instead of sizeof(node). This is one of the main reasons I advise against putting pointers in typedefs -- it makes it too easy to forget what you're actually doing. Also, it's a good idea to use the following construct for allocating structures, to make sure you always get the right size:

Code:

temp = malloc(sizeof(*temp));

Lastly, don't cast malloc: Cprogramming.com FAQ > Casting malloc.

Other than that, I think your code looks fine.

EDIT: Too slow. And, if your compiler needs an explicit cast, you should put the type of what you're casting to (temp) in parentheses. That is, if temp is of type "link" (not "link *"), then it should look like temp = (link)malloc(sizeof(*temp));