Deleting linked list in C

Question

I am trying to create a function that deletes an entire list, but I keep getting an error. Everything works excepting cleaner() function.

#include <stdio.h>
#include <stdlib.h>

struct node{
    int n;
    struct node *next;
};

typedef struct node NOD;

NOD *create_first_node(int i)
{
    NOD *q;
    q=(NOD*)malloc(sizeof(NOD*));
    q->n=i;
    q->next=NULL;
    return q;
}

NOD * add_to(NOD *x)
{
    NOD *q;
    q=(NOD*)malloc(sizeof(NOD*));
    q->n=rand();
    x->next=q;
    q->next=NULL;
    return q;
}

void show_list(NOD *p)
{
    printf("root");
    while(p->next){
        printf(" -> %d",p->n);
        p=p->next;
    }
    printf("\n");
}


void cleaner(NOD *p)
{
    NOD *r;
    while(p)
    {
        r=p;
        p=r->next;
        free(r);
        r=NULL;
    }
}


int main()
{
    int i;
    NOD *root,*c,*r;
    root=create_first_node(1);
    c=r=root;
    c=add_to(root);
    for(i=0;i<10;i++)
    {
        r=c;
        c=add_to(r);
    }
    show_list(root);
    //cleaner(root);
    system("pause");
    return 0;
}

NetBeans:

Signal received: SIGTRAP (?) with sigcode ? (?) From process: ? For program list, pid -1

You may discard the signal or forward it and you may continue or pause the process To control which signals are caught or ignored use Debug->Dbx Configure

Visual Studio:

Debug Error!

HEAP CORRUPTION DETECTED: after Normal block (#57) at 0x00393230 CRC detected that the application wrote to memory after end of heap buffer.

(I get this error with #57,#58,...,#68 each time cleaner() is trying to free a list item)

The "heap corruption" is an immediate hint that you have made some kind of mistake with a pointer, btw. Walking off the end of an allocation, double free, following a un-intilized pointer, ... lots of choices, but all *pointer* related. In a case like this that might not seem very helpful, but it is worth remembering. — dmckee --- ex-moderator kitten, Dec 10 '11 at 00:17

score 3 · Accepted Answer · answered Dec 09 '11 at 23:25

3

q=(NOD*)malloc(sizeof(NOD*)); should be q=(NOD*)malloc(sizeof(NOD));, I think.

You need to allocate enough memory for the whole Node, but you are only allocating enough for a pointer to it.

answered Dec 09 '11 at 23:25

AShelly

34,686
15
91
152

score 1 · Answer 2 · answered Dec 09 '11 at 23:28

I admit I have just skimmed your code, but when you create and add nodes, you do a

 q=(NOD*)malloc(sizeof(NOD*));

but you should actually do a

 q=(NOD*)malloc(sizeof(NOD));

Otherwise you will just allocate the space for a NOD* which is 4 or 8 bytes depending on your system (and assuming it is a common PC). But your NOD struct will have at least 8 or 16 bytes (again depending on your system and the size of your int's and assuming you are using common PC hardware).

So when you access the n member you are already on barely legal ground but when you later touch the next member, you are in the middle of undefined behaviour, and lucky you your software causes only a heap corruption...

score 0 · Answer 3 · answered Dec 09 '11 at 23:30

0

NULL is not necessarily the same as 0. It usually is, but you "shouldn't" assume it will be. So, while(p) is dangerous … please use while (NULL != p) or (p != NULL) :-)
When using malloc, you're allocating the size of a NOD*, a pointer; you meant to use sizeof(NOD).
Also, don't put a typecast (NOD*) on the return of malloc. (Google "malloc typecast" for a laundry list of reasons, but suffice to say, it's uneccessary and fairly likely to break your code and hide bugs later.)
The r = NULL is superfluous, but it's a good habit to get into, to clear the pointer after free(), so that's not bad :-)

answered Dec 09 '11 at 23:30

BRPocock

13,638
3
31
50

I'm not sure I agree entirely with the first point. NULL is defined in . Any C programmer who redefines it as anything other than 0 should be removed from the building. And `while (p)` is guaranteed to be false when `p` is a Null Pointer. See http://stackoverflow.com/q/459743/10396 – AShelly Dec 09 '11 at 23:56
Wasn't aware of the special-case for while(p). As for "defined in `stddef.h` — yes, it's defined there, but it's implementation-specific: it may be defined by a particular OS/CPU combination to be anything; e.g. `0xffff` isn't entirely rare on embedded systems with 16-bit memory addressing; `-1` is actually used on many systems. The `NULL == 0` concept is jokingly referred to as, "All the world's a VAX" … it happens to often be true on Intel + (Linux, Windows, MacOS), but still not a great idea to "count on it"… – BRPocock Dec 10 '11 at 22:58

Deleting linked list in C

3 Answers3