Search code examples
cmallocfree

C: Linked list and free() - Mistake in example?


studying the book C Primer Plus by Prata, 6th edition, I came across the linked list example of Listing 17.2 which is copied below this text. I am confused about the part where he frees the memory again. Isn't he freeing the pointer to the next list in his example? In the end, he sets current to head, which makes current point to the starting memory address of the first structure. Then he frees current and sets current=current->next, but current->next shouldn't contain any address anymore because current is freed? Running the program via CodeBlocks works fine, but debugging the free() while loop in CodeBlocks, I get a segment fault. I believe my understanding is correct that the program is faulty, right? Thank you.

/* films2.c -- using a linked list of structures */
#include <stdio.h>
#include <stdlib.h> /* has the malloc prototype */
#include <string.h> /* has the strcpy prototype */
#define TSIZE 45 /* size of array to hold title */
struct film
{
    char title[TSIZE];
    int rating;
    struct film * next; /* points to next struct in list */
};
char * s_gets(char * st, int n);
int main(void)
{
    struct film * head = NULL;
    struct film * prev, * current;
    char input[TSIZE];
    /* Gather and store information */
    puts("Enter first movie title:");
    while (s_gets(input, TSIZE) != NULL && input[0] != '\0')
    {
        current = (struct film *) malloc(sizeof(struct film));
        if (head == NULL) /* first structure */
            head = current;
        else /* subsequent structures */
            prev->next = current;
        current->next = NULL;
        strcpy(current->title, input);
        puts("Enter your rating <0-10>:");
        scanf("%d", &current->rating);
        while(getchar() != '\n')
            continue;
        puts("Enter next movie title (empty line to stop):");
        prev = current;
    }
    /* Show list of movies */
    if (head == NULL)
        printf("No data entered. ");
    else
        printf ("Here is the movie list:\n");
    current = head;
    while (current != NULL)
    {
        printf("Movie: %s Rating: %d\n",
               current->title, current->rating);
        current = current->next;
    }
    /* Program done, so free allocated memory */
    current = head;
    while (current != NULL)
    {
        free(current);
        current = current->next;
    }
    printf("Bye!\n");
    return 0;
}
char * s_gets(char * st, int n)
{
    char * ret_val;
    char * find;
    ret_val = fgets(st, n, stdin);
    if (ret_val)
    {
        find = strchr(st, '\n'); // look for newline
        if (find) // if the address is not NULL,
            *find = '\0'; // place a null character there
        else
            while (getchar() != '\n')
                continue; // dispose of rest of line
    }
    return ret_val;
}


Solution

  • current = head;
    while (current != NULL)
    {
        free(current);
        current = current->next;
    }
    

    but current->next shouldn't contain any address anymore because current is freed?

    Your assumptions are correct, you are trying to access an already deleted node and that's why your code segfaults, instead, use a temporary node, in this case you can reuse head:

    current = head;
    while (current != NULL)
    {
        head = current->next;
        free(current);
        current = head;
    }
    

    Running the program via CodeBlocks works fine ...

    Pure luck ...