CODEWITHSUNDEEP
clinked-listfreevalgrind
user720491
user720491

Reputation: 599

Conditional jump or move depends on uninitialised value - freeing a linked list

I want to free a linked list in C. All is working fine, but Valgrind is telling me

Conditional jump or move depends on uninitialised value(s)
    at 0x401400: mtf_destroy

Here's the code:

list_elt *head;

void mtf_init() {
    list_elt *current;
    head = malloc(sizeof(list_elt));
    current = head;
    for (int i = 0; i < LIST_SIZE-1; i++) {
        current->value = (BYTE) i;
        current->next = malloc(sizeof(list_elt));
        current = current->next;
    }
    current->value = LIST_SIZE-1;
}

void mtf_destroy(list_elt *elt) {
    if (elt->next != NULL)
        mtf_destroy(elt->next);
    free(elt);
}

How can I solve this? Thanks!

Upvotes: 1

Views: 1115

Answers (1)

alk
alk

Reputation: 70951

Valgrind wants to tell you the instance of elt->next used as expression to if() has not been initialize, so the decision made by if() is random.

You might like to change:

head = malloc(sizeof(list_elt));
...
  current->next = malloc(sizeof(list_elt));

to be:

head = calloc(1, sizeof(list_elt));

..."
  current->next = calloc(1, sizeof(list_elt));

A portable approach would be to initialise all pointers explicitly using NULL:

head = calloc(1. sizeof(list_elt));
head->next = NULL;
...
  current->next = calloc(1, sizeof(list_elt));
  current->next->next = NULL;

(The background to this is, that not on all systems a NULL-pointer necessarily is represented by a sequence of NUL characters (bytes)).

Anyway I'm not sure whether you assign the right thing to nextas you did not post te definition of list_elt.

Update:

Regaring the recursive calls to mtf_destroy():

Although recursions look cool, and are indeed an elegant solution to many problems, using them tends to be error prone, for example in case of going really deep, as they consume system resources (memory).

So if your list would be really long, destroying it might break your code.

My advice: Try doing it without recursion, at least if you can not predict the recursion levels to be performed in advance.

Upvotes: 3

Related Questions