Search code examples
c++unit-testinglinked-listgoogletest

C++ Linked list unit test returns segment fault


My algorithm for implementing a linked-list as follows

  • Function to add a new node and returns location pointer.
  • One core function to handle adding a node front,end operations.

linkedList.hpp

#include <cstddef>

class LinkedList{
  public:
    int value {0};
    LinkedList* nextNode {NULL};
};

LinkedList* addNewNode(int nodeVal){

  LinkedList *newNode;
  newNode->value = nodeVal;
  newNode->nextNode = nullptr;

  return newNode;
}

Below googletest unit-test checks whether

  • Returned node's pointer is not null.
  • Returned node has a value.
  • Returned node's next node value is set to NULL.

linkedListTest.cpp

#include <gtest/gtest.h>
#include "../linkedList.hpp"

int main(int argc, char **argv){
    ::testing::InitGoogleTest(&argc,argv);
    return RUN_ALL_TESTS();

}

class LinkedListTest : public ::testing::Test{

    public:
        LinkedList *linkedlist = new LinkedList();
        virtual void SetUp(){
            }
        virtual void TearDown(){
            delete linkedlist;
        }
};
TEST_F(LinkedListTest,addNewNodeReturnsItsNodePointer){

    // act
    linkedlist = addNewNode(5);

    EXPECT_TRUE(linkedlist != nullptr);
    ASSERT_EQ(linkedlist->value,5);
    EXPECT_TRUE(linkedlist->nextNode != nullptr);

}

When I run this code,tests pass but I get

Segmentation fault

What did I miss here?


Solution

  • newNode in addNewNode was never initialized, so it's a pointer to nowhere:

    LinkedList* addNewNode(int nodeVal) {
      LinkedList *newNode;  // Uninitialized, so undefined
      newNode->value = nodeVal;  // `->` dereferences the pointer, but it goes nowhere!
    

    One way to initialize it is using heap allocation, i.e., operator new - but remember that you need to manage resources in C++, so you'll need to free it after your done using it.

      LinkedList* addNewNode(int nodeVal) {
          LinkedList *newNode = new LinkedList();
          newNode->value = nodeVal;
          newNode->nextNode = nullptr;
    

    Later, to free the memory, you can do something like this:

    if (myNode->nextNode != nullptr) {
        delete myNode->nextNode;
        myNode.nextNode = nullptr;
    }
    

    But if you want to delete the whole LinkedList, you'll first have to walk to its end and start deleting from there. If you delete a node before deleting its successor, you create a memory leak bc. you no longer have a pointer to free that memory.

    Also make sure to turn your compiler warnings up! Default settings are way too lenient. E.g., for GCC you can use -Wall. Which tells me:

    <source>: In function 'LinkedList* addNewNode(int)':
    <source>:12:18: warning: 'newNode' is used uninitialized [-Wuninitialized]
       12 |   newNode->value = nodeVal;
          |   ~~~~~~~~~~~~~~~^~~~~~~~~
    

    And when you get a segmentation fault (segfault), you should compile your program in debug mode and run it in a debugger. It can tell you where the error happened:

    gcc -g -Og -o myprog myprog.c
    gdb ./myprog
    run
    (segfault)
    backtrace