[FIXED] What is causing segmentation fault in append Node function in doubly linked list?

Issue

My doubly linked list implementation is as follows that each node holds an array of four values

#define EMPTYNODE 0

struct node {
short data[4]; // pay attention
struct node* next;
struct node* prev;
};

typedef struct node nodeQ_t;

typedef enum{
   LIST_FALSE = 0,
   LIST_TRUE = 1,
} status_t;

nodeQ_t* createNode(short values[4]){

    nodeQ_t* node = (nodeQ_t*)malloc(sizeof(node));
    for(int i=0; i < 4; i++){
       node->data[i] = values[i];
     }

   node->next = EMPTYNODE;
   node->prev = EMPTYNODE;
   return node;
}

now I am trying to write append function in a way that I supply it head and a node created in createNode function so that it would append it to the list.... but it creates a segmentation fault...

status_t appendNode(nodeQ_t* head, nodeQ_t* newNode){
if(head == EMPTYNODE || newNode == EMPTYNODE){
    return LIST_FALSE;
};

nodeQ_t* currentNode = head;

while(currentNode != EMPTYNODE){
    if(currentNode->next == EMPTYNODE){ //it means that current node is tail
        currentNode->next = newNode;  //segmenttion fault arises at exactly this line 
        newNode->prev = currentNode;
    }
    currentNode = currentNode->next;
}
return LIST_TRUE;
}

please let me know what is the reason for that... for your reference my main function is

int main(){
  short array[4] = {1,2,3,4};

  nodeQ_t* head  = createNode(array);

  printList(head);


  short array2[4] = {5,6,7,8};

  nodeQ_t* newNode = createNode(array2);

  appendNode(head, newNode);


  printList(head);



  return 0;

}

if you need any further information or explanation for anything please do let me know

Solution

As mentioned in the comments, you need to break out of the loop once you've reached the end:

while(currentNode != EMPTYNODE) {
    if (currentNode->next == EMPTYNODE) {
        currentNode->next = newNode;
        newNode->prev = currentNode;
        // need a break here
    }
    currentNode = currentNode->next;
    // When at the end of the list the 1st time through, 
    // currentNode is the newly created node because you have
    //     currentNode->next = newNode
    // then
    //     currentNode = currentNode->next
    // On the next iteration, the new node next ends up getting pointed to itself 
    // since on that iteration newNode and currentNode are the same.
    // and you end up with an infinite loop.
}

Another option is to loop on currentNode->next:

while (currentNode->next) {
    currentNode = currentNode->next;
}
currentNode->next = newNode;
newNode->prev = currentNode;

I should note that this works because you previously ensured that currentNode is not NULL.

Also, your allocation here is wrong:

nodeQ_t* node = (nodeQ_t*)malloc(sizeof(node));

Because node is a pointer and sizeof(node) is the size of a pointer, not the size of struct node. Should be

nodeQ_t* node = (nodeQ_t*)malloc(sizeof(*node));

Answered By - Johnny Mopp

Answer Checked By - David Marino (PHPFixing Volunteer)