CODEWITHSUNDEEP
cpointerslinked-list
buh
buh

Reputation: 363

Destroying a Linked list in C

So here's my code for destroying a linked list.

void destroy(node *h){
    if (h->next!=NULL){
        destroy(h->next);
    }
    free(h);
    h=NULL;

}

The problem is that printing still outputs a bunch of numbers:

11, 2, 15, 3, 9, //Before Destroy

28495936, 28495968, 28496064, 28496096, 0, //After Destroy

Unfortunately I cannot alter the void destroy(node *h) parameters due to assignment reasons. I've tried using a while loop method but I still get the same result. I've also tried shifting to the left and deleting from the end but then I wouldn't be able to delete the last node.

thanks in advance.

--edit--- as requested, here is the print function

void print(node* N){
        printf("%d, ", N->value);
    if (N->next)
        print_set(N->next);
    if (N == NULL)
        printf("Empty Set");
}

Upvotes: 1

Views: 1654

Answers (6)

alk
alk

Reputation: 70901

If the solution provided by Albert is not possible due to some rules, the only possibility you as the author of the source in question have is to remember the list's nodes had been deallocated and therefore contain invalid references to memory and that the code you write because of the latter may not dereference such pointers, that is they may not be passed to the printing functions as this would provoke undefined behaviour by accessing un/deallocated memory.

If writing such potential insecure code, it is in your responsibilty as the author to use it carefully and document it very well for your fellow programmers maintaining the code after you left the project.

Upvotes: 3

Albert
Albert

Reputation: 68130

You must set h->next = NULL. Also, after the call to destroy, make sure you don't use the pointer anymore because it is freed. So, always after destroy(n), make sure that you have n = NULL.

A better way is probably to change the signature to void destroy(node **h), so the code becomes:

void destroy(node **h){
    if ((*h)->next!=NULL){
        destroy(&h->next);
    }
    free(*h);
    *h=NULL;
}

Then you ensures that you are not using the pointer afterwards.

In your print function, you must add this check at the beginning:

if(N == NULL) return;

Upvotes: 3

Ashish
Ashish

Reputation: 1941

Try this code if u are working on singly linked list

void destroy(node *h){    
   node *n;    
   node *p; \\ variable to store previous term
   n=h;
   while(n->next!=NULL){
   p = n;
 }
  p->next=NULL;  
   free(n);

}

Upvotes: 1

Kaidjin
Kaidjin

Reputation: 1513

The problem here is that h=null in your function does not do anything. You're modifying a local parameter so it won't have any effect outside the function.

Thus, the only thing you do is freeing the memory, but you keep the address the same. Your list still exists, pointing to random memory location (well not random : the same as before, but the values at this memory locations are random)

When you print your list after that (which is strange because you're supposed to have destroyed it... Why would you want to print it again ?), you print random values in memory.

This is a problem because your program could also crash (you're accessing memory which is not allocated).

The solution, unfortunately, requires to change the signature of your function : 

void destroy(node **h){
    if ((*h)->next!=NULL){
        destroy((*h)->next);
    }
    free(*h);
    *h=NULL;

}

If you can't, you have to set the pointer to NULL after destroying it, like this:

void destroy(node *h){
    if (h->next!=NULL){
        destroy(h->next);
        h->next=NULL;
    }
    free(h);
}

and in the calling function:

destroy(myList);
myList=NULL;

Upvotes: 2

Roddy
Roddy

Reputation: 68023

The problem is probably in code you haven't posted!

I assume you keep a 'head' pointer to your list, and your code looks like this.

Node * myList;

.. do stuff..

destroy(myList);
print(myList);

The problem is that you don't set myList = NULL following the destroy.

destroy(myList);
myList = NULL;
print(myList);

Your h=NULL in destroy() doesn't do anything because it's modifying a local parameter.

Upvotes: 2

Emil Vikström
Emil Vikström

Reputation: 91912

I don't know what your structure looks like, but I'm guessing something like this:

struct {
  int something;
  int* value;
  list* next;
}

The problem is that even though h is the NULL pointer, h->value and h->next are not. They are pointers in NULL+1 and NULL+2, which may point to random places in memory.

Upvotes: 1

Related Questions