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

Question

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

I think your problem is here: (nodeQ_t*)malloc(sizeof(node)). That should be sizeof(*node). node is a pointer so you're allocating bytes for the size of the pointer, not the size of the struct node points to. — sj95126
– sj95126, Commented Jul 2, 2021 at 14:51
@500-InternalServerError: I'm not saying the return from malloc isn't a pointer. I'm saying sizeof(node) is allocating just 4 or 8 bytes (whatever the size of a pointer is) when the correct thing to do is allocate *node bytes, the size of the struct. See @Johnny Mopp's answer below. — sj95126
– sj95126, Commented Jul 2, 2021 at 15:24
@sj95126: Sorry, ambiguous use of the identifier node here. — 500 - Internal Server Error
– 500 - Internal Server Error, Commented Jul 2, 2021 at 15:44

001 · Accepted Answer · 2021-07-04 14:55:44Z

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));

KPR · Accepted Answer · 2021-07-02 14:58:24Z

0

You end up in endless loop:

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 will always be different than EMPTYNODE. Add break or return after adding new element:

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;
        return LIST_TRUE;
    }

answered Jul 2, 2021 at 14:58

KPR

1018 bronze badges

Collectives™ on Stack Overflow

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

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