Int linked list in c throwing segmentation fault

Question

Inserting into my linked list is causing a segmentation fault and I'm not sure why. It's an int linked list. It works sometimes and then other times not. I can't see where it would be going wrong? Maybe it has something to do with when I create the new linked list. As the linked list insert last method is working during file reading but not when I create a new list.

LinkedList* createLinkedList()
{
    LinkedList* list;

    list = (LinkedList*)malloc(sizeof(LinkedList));
    list->head = NULL;
    list->tail = NULL;
    list->length = 0;

    #ifdef DEBUG
    printf("Linked list. Creation of list complete. Checking list length. %d \n", list->length);
    #endif

    return list;
}

void insertLast(LinkedList* list, int entry)
{
    Node* newNode = (Node*)malloc(sizeof(Node));
    newNode->data = entry;

    /*If list is empty*/
    if(list->length == 0)
    {
        list->head = newNode;
        list->tail = newNode;
    }
    else
    {
        newNode->prev = list->tail;
        /*if this is the second item in the list*/
        if(list->length == 1)
        {
            list->head->next = newNode;
        }
        else
        {
            list->tail->next = newNode;
        }
    }
    
    list -> tail = newNode;
    list->length++;
    #ifdef DEBUG
    printf("Linked List. Insert last complete. Checking list length. %d\n", list->length);
    #endif
}

This works

int main (void)
{
    LinkedList* list = createLinkedList();
    insertLast(list, 1);
}

but this doesn't

int shortestSeekTimeFirst(LinkedList* list)
{
    LinkedList* usedDisks = createLinkedList();

    int data = list->head->data;
    insertLast(usedDisks, data);
    insertLast(usedDisks, list->head->next->data);
}

maybe linklist in shortestSeekTimeFirst ,the head is NULL, you cannot get anything from NULL — zhj
– zhj, Commented Apr 18, 2022 at 13:32
Learn to use your environment's debugging tools and step through your code. It will be the best thing for you to do. — Lee Taylor
– Lee Taylor, Commented Apr 18, 2022 at 13:49

Willis Hershey · Accepted Answer · 2022-04-18 13:48:02Z

5

You never want to blindly dereference a pointer that could be NULL.

In shortestSeekTimeFirst() you directly access list->head->next->data without checking if any of the pointers are valid, so if any of list, list->head, or even list->head->next is NULL your program will segfault.

answered Apr 18, 2022 at 13:48

Willis Hershey

1,5827 silver badges24 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Vlad from Moscow · Accepted Answer · 2022-04-18 14:08:05Z

This function insertLast is incorrect.

void insertLast(LinkedList* list, int entry)
{
    Node* newNode = (Node*)malloc(sizeof(Node));
    newNode->data = entry;

    /*If list is empty*/
    if(list->length == 0)
    {
        list->head = newNode;
        list->tail = newNode;
    }
    else
    {
        newNode->prev = list->tail;
        /*if this is the second item in the list*/
        if(list->length == 1)
        {
            list->head->next = newNode;
        }
        else
        {
            list->tail->next = newNode;
        }
    }
    
    list -> tail = newNode;
    list->length++;
    #ifdef DEBUG
    printf("Linked List. Insert last complete. Checking list length. %d\n", list->length);
    #endif
}

In this if statement

    if(list->length == 0)
    {
        list->head = newNode;
        list->tail = newNode;
    }

neither the data member prev nor the data member next of the new node is set to NULL. They are uninitialized and have indeterminate values.

The same problem exists when the list is not empty that is neither the data member prev nor the data member next of the new node is set.

The function can be written simpler

 int insertLast( LinkedList *list, int entry )
 {
     Node* newNode = malloc( sizeof( Node ) );
     int success = newNode != NULL;

     if ( success )
     {
         newNode->data = entry;
         newNode->next = NULL;
         newNode->prev = list->tail;

         if ( list->tail == NULL )
         {
             list->head = newNode;
         }
         else
         {
             list->tail->next = newNode;
         }  

         list->tail = newNode; 
         list->length++;
     }

     return success;
 }

As for this code snippet

int shortestSeekTimeFirst(LinkedList* list)
{
    LinkedList* usedDisks = createLinkedList();

    int data = list->head->data;
    insertLast(usedDisks, data);
    insertLast(usedDisks, list->head->next->data);
}

then after this declaration

    LinkedList* usedDisks = createLinkedList();

the data members head and tail of the list are equal to NULL. So at least this statement

    int data = list->head->data;

invokes undefined behavior.

Collectives™ on Stack Overflow

Int linked list in c throwing segmentation fault

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related