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 :D

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));

> temp=(link)malloc(sizeof(link));
It's also a fine example of why you should NOT typedef pointers(*).
Being able to see instantly how many levels of indirection there are is a lot easier without having to trawl through a bunch of typedefs.
Also, when you start making use of 'const' qualifiers, the typedef approach simply breaks down.
Save a character, and cause confusion is hardly a good tradeoff.

Also, it helps if you do this
temp = malloc( sizeof(*temp) );

then you don't have to worry about getting the right type in the sizeof - the compiler will do it for you. All you have to remember is p=malloc(sizeof(*p)) and you're good.


(*) the exception being function pointers, where the syntax is quite horrible to begin with, and there are less things to do with a function pointer.