Debugging linked list pointers code: segmentation fault

Question

So the following code:

/*
 * For your reference:
 *
 * SinglyLinkedListNode {
 *     int data;
 *     SinglyLinkedListNode* next;
 * };
 *
 */
SinglyLinkedListNode* insertNodeAtTail(SinglyLinkedListNode* head, int data) {
  SinglyLinkedListNode* temp = head;
  while (temp != NULL) {
    temp = temp->next;
  }
  SinglyLinkedListNode* temp1;
  temp1->data = data;
  temp1->next = NULL;
  temp->next = temp1;
  return temp;
}

So, basically I want to add "data" at the end of the linked list "head" and return the updated list. So where is the fault?

Edit: Okay I got the first mistake. But even if I replace temp!=NULL with temp->next!=NULL in the loop conditional still there's this error

SinglyLinkedListNode* temp1; temp1->data=data; does not work, where does temp1 point to? — mch
– mch, Commented Apr 24, 2020 at 10:03
@mch, okay, I am new to this. What I thought was that I should initialise a blank variable like we declare integers as "int a; a=2;". So how do I resolve this? — Aditya Agarwal
– Aditya Agarwal, Commented Apr 24, 2020 at 10:05
What's the value of temp after the while(temp!=NULL) loop? — molbdnilo
– molbdnilo, Commented Apr 24, 2020 at 10:05
@molbdnilo, the pointer to the last node of the linked list? — Aditya Agarwal
– Aditya Agarwal, Commented Apr 24, 2020 at 10:06

Thomas Sablik · Accepted Answer · 2020-04-24 10:22:04Z

2

You have to allocate memory for the node. Remember to clean up the allocated memory. For each call to new you need a call delete. Therefore I prefer smart pointers.

After your loop temp contains NULL. You can't dereference a null pointer.

SinglyLinkedListNode* insertNodeAtTail(SinglyLinkedListNode* head, int data) {
    SinglyLinkedListNode* temp1 = new SinglyLinkedListNode;
    temp1->data = data;
    temp1->next = nullptr;
    if (!head) {
        head = temp1;
        return head;
    }
    SinglyLinkedListNode* temp = head;
    while(temp->next){
        temp = temp->next;
    }
    temp->next = temp1;
    return temp;
}

edited Apr 24, 2020 at 10:22

answered Apr 24, 2020 at 10:14

Thomas Sablik

16.5k7 gold badges38 silver badges69 bronze badges

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

6 Comments

Thomas Sablik Over a year ago

@AdityaAgarwal A pointer can be implicitly converted to bool and returns false for nullptr and true else. !head is equivalent to head == nullptr.

Aditya Agarwal Over a year ago

Okay, thanks! I understood all of what you said. But when I use your code, the following error occurs: error: no matching function for call to ‘SinglyLinkedListNode::SinglyLinkedListNode()’

Thomas Sablik Over a year ago

@AdityaAgarwal That means you have no default constructor. You have to call one of your constructors at the point.

LIU Qingyuan Over a year ago

@AdityaAgarwal pointer value are evaluated to true iff it is not nullptr, so '''!head''' is true iff head IS nullptr

Thomas Sablik Over a year ago

Any change on *temp will also occur on *head.

|

Vlad from Moscow · Accepted Answer · 2020-04-24 10:40:52Z

This function

SinglyLinkedListNode* insertNodeAtTail(SinglyLinkedListNode* head, int data) {
SinglyLinkedListNode* temp=head;
while(temp!=NULL){
    temp=temp->next;
}
SinglyLinkedListNode* temp1;
temp1->data=data;
temp1->next=NULL;
temp->next=temp1;
return temp;
}

does not make sense. After this loop

while(temp!=NULL){
    temp=temp->next;
}

the pointer temp is equal to NULL. So this statement

temp->next=temp1;

invokes undefined behavior.

The pointer temp1 was not initialized. So again these statements

temp1->data=data;
temp1->next=NULL;

invoke undefined behavior.

The user of the function does not know whether the returned pointer is the head pointer or the last pointer of the list. So it is unclear whether to assign the returned pointer to the head pointer or just to ignore the returned value.

The function can look the following way.

void insertNodeAtTail( SinglyLinkedListNode * &head, int data ) 
{
    SinglyLinkedListNode **current = &head;

    while ( *current != nullptr ) current = &( *current )->next;

    *current = new SinglyLinkedListNode { data, nullptr };
}

If in main you defined the pointer to the head node like

SinglyLinkedListNode *head = nullptr;

then a function call will look like

insertNodeAtTail( head, some_data );

Another definition of the function can look the following way

SinglyLinkedListNode* insertNodeAtTail( SinglyLinkedListNode *head, int data ) 
{
    SinglyLinkedListNode *new_node = new SinglyLinkedListNode { data, nullptr };

    if ( head == nullptr )
    {
        head = new_node;
    }
    else
    {
        SinglyLinkedListNode *current = head;

        while ( current->next != nullptr ) current = current->next;

        current->next = new_node;
    }

    return head;
}

In this case if in main you defined the pointer to the head node like

SinglyLinkedListNode *head = nullptr;

then the function call will look like

head = insertNodeAtTail( head, some_data );

Between these two function definitions the first function definition is preferable because there is no need to remember to assign the returned pointer to the head node.

Bear in mind that if you have a singly-linked list and want to append new nodes to the tail of the list ten it is better to define two-sided singly-linked list. In this case the list definition can look like

class SinglyLinkedList
{
private:
    struct Node
    {
        int data,
        Node *next;
    } *head = nullptr, *tail = nullptr;

public:

    SinglyLinkedList() = default;
    void insertNodeAtHead( int data );
    void insertNodeAtTail( int data );
    // other member functions;
};

David Buck · Accepted Answer · 2020-04-24 10:17:07Z

0

It crashes here also:

SinglyLinkedListNode* temp1;
temp1->data=data;

when you reach the end of the loop while(temp!=NULL){ , here temp is NULL. below you are using a statement like this temp->next=temp1; this causes crash

edited Apr 24, 2020 at 10:17

David Buck

3,87740 gold badges54 silver badges73 bronze badges

answered Apr 24, 2020 at 10:07

narayanan ms

1

1 Comment

Aditya Agarwal Over a year ago

Thanks, please look at the edits, and please tell why are they crashing?

Collectives™ on Stack Overflow

Debugging linked list pointers code: segmentation fault

3 Answers 3

6 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

6 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related