CODEWITHSUNDEEP
cpointerscrashlinked-listsegmentation-fault
danihodovic
danihodovic

Reputation: 1255

Segmentation fault, although the code is never executed?

I have just started learning C and was looking into simple implementations of linked lists. When I tried printing my list I realized something peculiar. I would get segmentation errors at a part of the code even if that code was never executed. How can this be?

To my knowledge the while loop is never executed as the next pointer does not point at anything. I was thinking that maybe when evaluating the while loop it may cause it to point at something that causes the segmentation error, but strangely if you remove the line root = root->next it executes fine (without any errors). Even then it never enters the while loop. So how can a line of code (root = root->next cause an error if the code is never executed? The code compiles just fine. Have I made a simple mistake somewhere?

I know this is not nearly a proper implementation for a linked list, it was made for learning purposes.

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

struct linkedlist   {
    int value;
    struct linkedlist * next;
};

typedef struct linkedlist item;

int main(int argc, char **argv) {
    item * root;
    item * current;

    root = malloc(sizeof(item));
    root->value = 500;

    int i;
    for(i = 0; i <= 20; i++)    {
        current->next = malloc(sizeof(item));
        current = current->next;
        current->value = i;
    }

    while(root->next != 0)  {
        //This is never executed
        printf("[%d]\n", root->value);
        //the line below does cause the segmentation error
        //but how can this affect anything at all if this is never executed?
        root = root->next;
    }
    return 0;
}

Upvotes: 2

Views: 99

Answers (1)

user3146587
user3146587

Reputation: 4320

The first line with current->next is dereferencing an uninitialized pointer. You probably forgot to initialize current to root before the loop. Derefencing an uninitialized pointer is undefined behavior (UB) which means anything can happen. In practice, the uninitialized variables will have values corresponding to whatever contents was stored at their memory location. So an uninitialized pointer will be be set to some semi-random address and will either point to some also semi-random contents in memory (possibly other program variables) or be invalid.

There is another uninitialized pointer being dereferenced in the condition for the while loop testing root->next. Generally, you should make sure that the next field of each of your list item (including root) is set to 0, else you won't be able to detect the end of the linked list (uninitialized pointer again, so UB again and in practice the value is likely to be different from 0).

A proposal to correct this kind of problem in your code:

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

struct linkedlist   {
    int value;
    struct linkedlist * next;
};

typedef struct linkedlist item;

int main(int argc, char **argv) {
    item * root;
    item * current;

    root = malloc(sizeof(item));
    root->value = 500;
    root->next = 0;

    current = root;

    int i;
    for(i = 0; i <= 20; i++)    {
        current->next = malloc(sizeof(item));
        current = current->next;
        current->value = i;
        current->next = 0;
    }

    while(root->next != 0)  {
        printf("[%d]\n", root->value);
        root = root->next;
    }
    return 0;
}

Upvotes: 2

Related Questions