segmentation fault - Doubly Linked List

Question

I am implementing a doubly linked list that works similar to a queue. So when I add nodes to the list (e.g 5 nodes) and empty the list and try to add a new node to the list, it gives me a segmentation fault (core dumped). I don't know why it is doing that. Can you explain?

linkedlist.h

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

typedef struct node{
    int d;
    struct node *prev;
    struct node *next;
}node;

typedef struct linkedlist{
    int size;
    struct node *first;
    struct node *last;
}linkedlist;

linkedlist.c

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

#include "linkedlist.h"

linkedlist* createList(){
    linkedlist* myList = (linkedlist*)calloc(1,sizeof(linkedlist));
    myList->first = NULL;
    myList->last = NULL;
    myList->size =0;

    return myList;
}

static node* createNode(int n){
    node *myNode = (node*)calloc(1,sizeof(node));

    myNode->d = n;

    myNode->prev = NULL;
    myNode->next = NULL;

    return myNode;
}

void insertNode(linkedlist* l, int num){
    node *temp, *newNode;

    newNode = createNode(num);

    if (l->size == 0){
        newNode->next = NULL;
        newNode->prev = NULL;

        l->first = newNode;
        l->last = newNode;

        l->size++;
    }
    else{
        temp = l->first;
        while (temp->next != NULL){
            temp = temp->next;
        }   

        newNode->prev = temp;
        temp->next = newNode;
        newNode->next = NULL;

        l->size++;
    }
}

int deleteNode(linkedlist* l){
    node *temp = calloc(1,sizeof(node));

    if (l->first ==NULL){
        return -1;
    }
    else if (l->size ==1){

        free(l->first);
        l->first= NULL;
        l->last = NULL;

        l->size--;
    }
    else if (l->size > 1){
        temp = l->first;
        l->first = temp->next;          

        free(temp);
    }
}

void display(linkedlist *l){
    node *temp = calloc(1,sizeof(node));
    temp = l->first;

    if (temp == NULL){
        printf("The list is empty\n");
    }
    while (temp != NULL) {
        printf("-> %d ", temp->d);
        temp = temp->next;
    }
}

int main(){

    linkedlist *myList = createList();

    int choice, temp=0, numb;
    printf("(1) Insert \n (2) Delete \n");

    for (temp; temp<10; temp++){
    printf("Choice :");
    scanf ("%d", &choice);
    switch(choice) {
        case 1: {
            printf("Enter a Number: ");
            scanf("%d", &numb);
            insertNode(myList, numb);
            display(myList);
            break;
        }
        case 2:{
             deleteNode(myList);
            display(myList);
            break;

        }
    }

    }       
}

You're memory leaks in deleteNode() and display() are not good. C++ is not Java. And if you want to discover that fault, perhaps run this under a debugger like gdb. — WhozCraig
Check this. for a gdb segfault example. Then you can get closer to where the segfault arises — Suvarna Pattayil
You might as well use malloc() as calloc() in createNode() and createList() since you initialize all the members in your functions anyway. However, that has nothing to do with the presence or absence of a core dump. You should also check that the allocation succeeds before initializing the data. — Jonathan Leffler
The code in insertNode() that seeks through the list is ... well, inefficient and makes a mockery of having the last member in the list structure. You're treating it as if you've only got a singly-linked list. You should be able to go directly to l->last and add the new node immediately after it; that's the only point in having last. Since you don't set last in insertNode() with a non-empty list, you can't use it after you have more than one entry in the list. — Jonathan Leffler

Jonatan Goebel Jonatan Goebel · Accepted Answer · 2013-05-16T17:28:57

You are not decrementing the size of the list when l->size > 1:

int deleteNode(linkedlist* l){

    node *temp = calloc(1,sizeof(node));

    if (l->first ==NULL){
        return -1;
    }
    else if (l->size ==1){
        free(l->first);
        l->first= NULL;
        l->last = NULL;
        l->size--;               \\ <--- Here is OK
    }
    else if (l->size > 1){
        temp = l->first;
        l->first = temp->next;          
        free(temp);
                                \\ <--- Here should have another l->size--
    }
}

You could also just move the decrement instruction out of the if statement.

segmentation fault - Doubly Linked List

linkedlist.h

linkedlist.c

2 Answers