Destructor function doesnt work as intended and I get a segmentation fault (C)

Question

I am currently working on some homework and I need some help.

The function free_lnode triggers a Segmentation fault(core dumped) once called. Before the core gets dumped I receieve the message that I am trying to free an unknown pointer.

I have narrowed down the problem and commented it in my code below, however I am unaware how to fix the error in the free_lnode function.

The implementation of the functions free_lnode and free_node need to be done via recursion!!!

Note: My code uses 2 function from the professors lib:

• xcalloc works the same way as calloc with the addition that it checks for NULL by itself*

• s_copy works like strcpy, however the copy is dynamically allocated

  These function are not the cause for the errors and are handled in the destructor functions.

Thanks in advance and a happy new year!

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


typedef struct Node {
    char* value;
    int count;
    struct Node *next;
} Node;

typedef struct WishList {
    Node *first;
    char *name;
} WishList;

typedef struct Lnode{
    WishList* value;
    struct Lnode* next;
} Lnode;

/********* Constructor functions ********/


Lnode* new_lnode(WishList* value, Lnode* next){
    Lnode* lnode = xcalloc(1, sizeof(Lnode)); //same as calloc only checks for NULL itself
    lnode->value = value;
    lnode->next = next;
    return lnode;
}


WishList* new_wish_list(char* name, Node* node) {
    WishList* list = xcalloc(1, sizeof(WishList)); //same as calloc only checks for NULL itself
    list->name = s_copy(name); // s_copy performs dynamic allocation
    list->first = node;
    return list;
}

Node* new_node(char* value, int count, Node* next) {
    Node* node = xcalloc(1, sizeof(Node)); //same as calloc only checks for NULL itself
    node->value = s_copy(value); // s_copy performs dynamic allocation
    node->count = count;
    node->next = next;
    return node;
}

/********* Destructor functions ********/

// Recursive destructor function ==> Works
void free_node(Node* node){
    if(node->next){
      free(node->value);
      free_node(node->next);
    }

    if(node->next == NULL){
      free(node->value);
    }
    free(node);
}

// destructor function ==> Works
void free_wish_list(WishList* list){
    free_node(list->first);
    free(list->name);
    free(list);
}

// Recursive destructor function => TODO: DEBUG
void free_lnode(Lnode* lnode){
    if(lnode->next){
      printf("DEBUG\n"); // is triggered twice before segmentation is dumped
      free_wish_list(lnode->value); //fails to free second wishlist, Console output: trying to free an unknown pointer <some memory location>, segmentation fault (core dumped)
      printf("DEBUG\n"); // is only triggered once before segmentation dumped
      free_lnode(lnode->next);
    }
    if(lnode->next == NULL){
      free_wish_list(lnode->value);
    }
    free(lnode);
}
/************************************/


int main(){

  Node* a = NULL;
  a = new_node("test1", 1, NULL);
  Node* b = new_node("test2", 1, a);
  Node* c = new_node("test3", 1, b);
  Node* d = new_node("test4", 1, c);
  Node* e = new_node("test5", 1, d);

  WishList* usage_test = new_wish_list("Epic_Test", e);

  Node* f = NULL;
  f = new_node("test6", 1, NULL);
  Node* g = new_node("test7", 1, f);
  Node* h = new_node("test8", 1, g);
  Node* k = new_node("test9", 1, h);

  WishList* usage_test2 = new_wish_list("Epic_Test2", k);

  Lnode* new_long_node = new_lnode(usage_test, usage_test2);
  free_lnode(new_long_node);

  return 0;
}

Answer 1

This code here:

Lnode* new_long_node = new_lnode(usage_test, usage_test2);
free_lnode(new_long_node);

is wrong and it's pretty simple to figure it out as it issues a quite-obvious warning:

test.c: In function ‘main’:
test.c:105:3: warning: passing argument 2 of ‘new_lnode’ from incompatible pointer type [enabled by default]
test.c:25:8: note: expected ‘struct Lnode *’ but argument is of type ‘struct WishList *’

So you're linking your nodes wrong. Just inferring from your API, I think you want to do something like this instead:

Lnode* new_long_node1 = new_lnode(usage_test, NULL);
Lnode* new_long_node2 = new_lnode(usage_test2, new_long_node1);
free_lnode(new_long_node2);

Oh, and that s_copy function of yours is in fact a strdup not strcopy.