C Linked list unit test returns segment fault [closed]-CodePudding

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?

CodePudding user response：

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