-1

I've started C programming yesterday and tried to implement a linked list (just really really basic).

So far everything works pretty fine except for freeing a list.

First, here's my code:

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

/*
 * Struct for elements of the linked list
 */
struct element {
    struct element* nextelement;
    int value;
};

/*
 * Struct for a list itself
 */
struct liste {
    struct element* headelement;
};

/*
 * Function to create new elements
 */
struct element* createelement(int value) {
    struct element* dummyelement = malloc(sizeof(struct element));
    dummyelement->nextelement = NULL;
    dummyelement->value = value;
    return dummyelement;
}

/*
 * Function to create new (empty) lists
 */
struct liste* createlist() {
    struct liste* dummylist = malloc(sizeof(struct liste));
    dummylist->headelement = NULL;
    return dummylist;
}

/*
 * Add an element to a given list
 */
void addelement(struct liste* liste, int value) {

    struct element* dummyelement = createelement(value);
    if (liste->headelement == NULL) {
        liste->headelement = dummyelement;
    } else {
        struct element* iterelement = liste->headelement;
        while (iterelement->nextelement != NULL) {
            iterelement = iterelement->nextelement;
        }
        iterelement->nextelement = dummyelement;
    }

}

/*
 * Plot the elements of a given list
 */
void plotlist(struct liste* liste) {
    if (liste->headelement != NULL) {
        struct element* iterelement = liste->headelement;
        printf("%d\n", iterelement->value);
        while (iterelement->nextelement != NULL) {
            iterelement = iterelement->nextelement;
            printf("%d\n", iterelement->value);
        }
    } else {
        printf("Where is my head?\n");
    }
}

/*
 * This should completely remove the list, but it fails...
 */
void removelist(struct liste* liste) {
    if (liste->headelement != NULL) {
        struct element* iterelement = liste->headelement;
        struct element* nextelement = iterelement->nextelement;
        free(iterelement);
        while (nextelement != NULL) {
            iterelement = nextelement;
            nextelement = iterelement->nextelement;
            free(iterelement);
        }
    }

    free(liste);
}

int main(void) {

    /*
     * Creates a new list
     * Plots the (empty) list
     * Adds two elements to the list
     * Plots the list again
     * Removes the list
     * Last plot shouldn't really happen, but it does.
     */
    struct liste* mylist = createlist();
    printf("First plot.\n");
    plotlist(mylist);
    addelement(mylist, 1);
    addelement(mylist, 2);
    printf("Second plot.\n");
    plotlist(mylist);
    removelist(mylist);
    printf("Third plot.\n");
    plotlist(mylist);

    return 0;

}

I get the following output:

First plot.
Where is my head?
Second plot.
1
2
Third plot.
33

Well, obviously the '33' is my problem. I really don't know how this can be the output... Furthermore I don't know why my 'removelist' isn't working properly. I am freeing everything which I've allocated with 'malloc'. What am I doing wrong?

  • 4
    Welcome to Stack Overflow! It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Paul R May 06 '16 at 12:43
  • 1
    Bear in mind that `removelist` clears out the whole list (or its supposed to). Most importantly frees the memory pointed to by the pointer you pass in. So when you call `plotlist` with that same pointer, you are using a dangling pointer !! `if (liste->headelement != NULL) {`will not protect you from this. So at the end of `removelist` simply set `liste->headelement` to NULL and do not free it. You will still need to free it later though. – Paul Rooney May 06 '16 at 13:16

1 Answers1

1

You do not check inside plotlist whether your list actually exists. You directly try to access the headelement. Better do the following:

void plotlist(struct liste* liste) {
    if(liste == NULL){
        printf("This list does not even exist.\n");
    }
    else if (liste->headelement != NULL) {
        // ...
    } else {
        printf("Where is my head?\n");
    }
}

As removelist also frees the list structure as such, you better set the pointer to NULL inside main. Otherwise you get undefined behaviour upon accessing freed memory.

int main(void) {
    // ...
    removelist(mylist);
    mylist = NULL;
    printf("Third plot.\n");
    plotlist(mylist);

    return 0;

}
jboockmann
  • 1,011
  • 2
  • 11
  • 27