1

I'm pretty new to C and I can't figure out why I'm getting a segmentation fault from this code:

void delete_list(LIST *list)
{
  NODE* n = list->head;
  while(n != list->tail)
  {
    NODE* next = n->next;
    free(n);
    n = next;
  }
  free(n);
  free(list);
}

I'm pretty certain this is the part of my code that's has the error, but if not I can post the rest of my code. Both NODE and LIST are created using malloc(). The code is the barebones framework for a doubly-linked-list.

EDIT 2: Here's the code to reproduce what I'm doing. Very basic test case but it is failing. First is the header file list.h, then the main file list.c. I also eradicated the first edit to clean up the question a bit.

#ifndef __LIST_H__
#define __LIST_H__

typedef struct node {
  char *value;  /* Pointer to the string we are storing. */
  struct node *previous;  /* Pointer to the preceding node in the list. */
  struct node *next;  /* Pointer to the next node in the list. */
} NODE;

typedef struct list {
  NODE *head;  /* Pointer to the first node in the list. */
  NODE *tail;  /* Pointer to the last node in the list. */
} LIST;

/* Function prototypes the public interface. */
LIST *new_list(const char *value);
void prepend(LIST *list, const char *value);
void append(LIST *list, const char *value);
void delete_list(LIST *list);

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

#include "list.h"

NODE *new_node(const char *value);

LIST *new_list(const char *value)
{
  LIST *list = (LIST*) malloc(sizeof(LIST));
  NODE* n = new_node(value);
  list->head = n;
  list->tail = n;

  return list;
}

void prepend(LIST *list, const char *value)
{
  NODE* n = new_node(value);
  n->next = list->head;
  list->head->previous = n;
  list->head = n;
}

void append(LIST *list, const char *value)
{
  NODE* n = new_node(value);
  n->previous = list->tail;
  list->tail->next = n;
  list->tail = n;
}

void delete_list(LIST *list)
{
  NODE* n = list->head;
  while(n != list->tail)
  {
    NODE* next = n->next;
    free(n);
    n = next;
  }
  free(n);
  free(list);
}

NODE *new_node(const char *value)
{
  NODE *node = (NODE*) malloc(sizeof(NODE));

  node->value = (char*) malloc((strlen(value) + 1) * sizeof(char));
  strcpy(node->value, value);
  node->previous = NULL;
  node->next = NULL;
  
  return node;
}

void print_list(const LIST *list)
{
  /* Print the contents of a list. */
  for (NODE *node = list->head; node != NULL; node = node->next) {
    printf("%s\n", node->value);
  }
}

int main()
{
  char *middle = "middle";
  char *last = "last";
  char *first = "first";
  LIST* list = new_list(middle);
  prepend(list, first);
  append(list, last);
  print_list(list);
  delete_list(list);
  print_list(list);
}
10
  • 3
    Use a debugger to catch the crash "in action", to locate when and where in your code it happens and to examine the values of all involved variables at the time and location of the crash. Commented May 6, 2022 at 7:23
  • 1
    "'i'm pretty certain this is the part of my code that's has the error," -- nope. You may find Doubly-Linked List of Integers - Remove Rand Nodes Check useful. Commented May 6, 2022 at 7:42
  • Yes, you should edit and post a minimal reproducible example. The problem is probably elsewhere. Commented May 6, 2022 at 7:46
  • Please provide a minimal reproducible example (emphasis on reproducible). Lots of things are missing for example what is LIST, what is new_node, where is main, etc.? How do you call all these functions and with what data? Commented May 6, 2022 at 8:03
  • 1
    After delete_list(list), list points to freed memory, therefore it's pretty normal that print_list doesn't work as expected. It's just like ...; free(p); DoSomething(p);. The rest of your code seems correct to me. Commented May 6, 2022 at 8:22

1 Answer 1

1

You obviously can't

  delete_list(list);
  print_list(list);

The list is gone after delete. (you invoke Undefined Behavior at that point) The only other issue you have is you fail to free() node->value.

You can fix that with:

void delete_list(LIST *list)
{
  NODE* n = list->head;
  while(n != list->tail)
  {
    NODE* next = n->next;
    free(n->value);
    free(n);
    n = next;
  }
  free (n->value);
  free(n);
  free(list);
}

Your code works fine then.

Memory Use/Error Check

$ valgrind ./bin/list
==9065== Memcheck, a memory error detector
==9065== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9065== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==9065== Command: ./bin/list
==9065==
first
middle
last
==9065==
==9065== HEAP SUMMARY:
==9065==     in use at exit: 0 bytes in 0 blocks
==9065==   total heap usage: 8 allocs, 8 frees, 1,130 bytes allocated
==9065==
==9065== All heap blocks were freed -- no leaks are possible
==9065==
==9065== For counts of detected and suppressed errors, rerun with: -v
==9065== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Alternative Delete List

You may find the following alternative for delete_list() a bit more readable -- up to you:

void delete_list (LIST *list)
{
  NODE *n = list->head;
  
  while (n) {
    NODE *victim = n;
    n = n->next;
    free (victim->value);
    free (victim);
  }
  free (list);
}
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.