What wrong with deallocating func?

81 views Asked by At

I am currently learning lists, and I am trying to write a function that deallocates the memory. I tried, but even though it compiles, it returns a non-zero value. I don't understand why my program doesn't return 0 when I use the deallocating function in the main function. However,the program is executed successfully when I don't use the deallocating function. To me the function seems fine. I can't spot the error.

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

typedef struct person {
    int age;
    struct person *next;
} person;

void insert_start(person **head, int x);
void insert_end(person **head, int x);
void insert_position(person **head, int x, int position);
void deallocating(person **head);

int main() {
    person *head = NULL;
    person p1 = { 2, NULL };
    person p2 = { 3, NULL };
    head = &p1;
    p1.next = &p2;
    insert_end(&head, 4);
    insert_start(&head, 12);
    insert_position(&head, 100, 3);

    for (person *curr = head; curr != NULL; curr = curr->next) {
        printf("%d\n", curr->age);
    }

    deallocate(&head);

    return 0;
}

void insert_start(person **head, int x) {
    person *nextnode = malloc(sizeof(person));
    nextnode->age = x;
    nextnode->next = *head;
    *head = nextnode;
}

void insert_end(person **head, int x) {
    person *nextnode = malloc(sizeof(person));
    nextnode->age = x;
    nextnode->next = NULL;

    person *find = *head;
    while (find->next != NULL) {
        find = find->next;
    }
    find->next = nextnode;
}

void insert_position(person **head, int x, int position) {
    person *nextnode = malloc(sizeof(person));
    nextnode->age = x;
    nextnode->next = NULL;

    person *find = *head;
    int curr = 1;
    while (curr != position) {
        find = find->next;
        curr++;
    }
    nextnode->next = find->next;
    find->next = nextnode;
}

void deallocate(person **head) {
    person *curr = *head;

    while (curr != NULL) {
        person *del = curr;
        curr = curr->next;
        free(del);
    }
    *head = NULL;
}
2

There are 2 answers

1
Alberto On

It seems to me that in your deallocate function:

The first time del takes the value of curr, then you deallocate del, but del and curr are now the same. Then you use curr->next, but curr is no longer valid.

0
Jabberwocky On

There are several problems:

These lines in main are pointless:

    person p1 = { 2, NULL };
    person p2 = { 3, NULL };
    head = &p1;
    p1.next = &p2;

It creates a mix of statically and dynamically allocated nodes which cannot be handled with without major hassle, because sooner or later you will end up freeing a statically allocated node which cannot work. Just don't do it

Then the prototype void deallocating(person **head); is wrong, the name of your function is deallocate. Your compiler should have warned you at least.

And last but not least, your insert functions are wrong. The case of the head being NULL needs special handling, as shown in the code below.

I leave it as an exercice to you to correct the other insert functions.

The deallocate functions looks OK to me.

Another thing is that malloc may return NULL (although this is unlikely with simple exercises like this one on a desktop computer) and you should check this. You can handle this at a later stage.

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

typedef struct person {
  int age;
  struct person* next;
} person;

void insert_end(person** head, int x);

int main() {
  person* head = NULL;
  
  insert_end(&head, 4);
  insert_end(&head, 5);

  for (person* curr = head; curr != NULL; curr = curr->next) {
    printf("%d\n", curr->age);
  }

  return 0;
}

void insert_end(person** head, int x) {
  person* nextnode = malloc(sizeof(person));
  nextnode->age = x;
  nextnode->next = NULL;

  if (*head == NULL)  // if head is null the person just created
  {                   // becomes the head
    *head = nextnode;
    return;
  }

  person* find = *head;
  while (find->next != NULL) {
    find = find->next;
  }
  find->next = nextnode;
}