user10609592
user10609592

Reputation:

Strange segfault with linked list

Had this working on visual studios most recent version, tried to sent to school server with makefile and this error on valgrind stopped at process first headPtr == nullptr when attempting to add int to empty list

segfault

It said to add some more details so I guess Ill keep on writing till it lets me submit, I don't know what else to say, if someone wants to try runnuing it I have menu and valid driver functions but testing could be done with out them.

#include "list.hpp"

int main()
{
    menu();
}
list: main.o list.o valid.o menu.o  node.o  
    g++ -std=c++0x -g main.o list.o valid.o menu.o  node.o   -o list
main.o: main.cpp
    g++ -std=c++0x -g -c main.cpp
node.o: node.cpp 
    g++ -std=c++0x -g -c node.cpp

list.o: list.cpp
    g++ -std=c++0x -g -c list.cpp


menu.o: menu.cpp 
    g++ -g -std=c++0x -c menu.cpp

valid.o: valid.cpp 
    g++ -g -c -std=c++0x valid.cpp
clean:
    rm *.o list

valgrind errorsenter image description here

  [1]: https://i.sstatic.net/MM9az.png



#include"list.hpp"


List::List()
{
    Node* headPtr = nullptr;
    Node* tailPtr = nullptr;
}
List::~List()
{

    if (headPtr != nullptr)

    {
        while (headPtr != nullptr)
        {
            Node* toDel = headPtr;
            headPtr = headPtr->getNext();
            delete toDel;
        }
    }
 }

void List::addHead(int add)
{

    Node* nodePtr = new Node;
    Node* nll = nullptr;
    nodePtr->setVal( add);//put in data
    nodePtr->setPrev(nll); // always do ->make prev( before new head) null as is now new head
    if (headPtr == nullptr)//empty list
    {
        nodePtr->setNext(nll);//nothing next prev already null

        headPtr = nodePtr;//only node set head
        tailPtr = nodePtr;//set tail
    }
    else //nodes exists already
    {
        nodePtr->setNext(headPtr);//next of new head is old head 
        (headPtr)->setPrev(nodePtr); //prev old head new haed
        headPtr = nodePtr;//change headPtr
    }

    traverse();

}

void List::addTail(int add)//takes pointer to pointer to tail node
{
    Node* nll = nullptr;

    Node* nodePtr = new Node;
    nodePtr->setNext(nll);//will be last node
    nodePtr->setVal(add);//put in data
    if (tailPtr == nullptr)
    {
        headPtr = nodePtr;//only node set head
        tailPtr = nodePtr;
        nodePtr->setPrev(nll); //nothing there
    }
    else
    {
        nodePtr->setPrev(tailPtr);
        tailPtr->setNext(nodePtr);
        tailPtr = nodePtr;
    }
    traverse();

}
    /*if (headPtr != NULL)//if this is NOT first element added

{//then stick new node to beginning of list b4 swaping head

    (headPtr)->setPrev(nodePtr);
}
headPtr = nodePtr; //reset head to new node

if (tailPtr == NULL)
{
    tailPtr = nodePtr;//if first node set tail


}*/
    /*
        tailPtr->setNext(nodePtr) ;//make link to old tail ->newtail
    }
    tailPtr = nodePtr;
    if (headPtr == NULL)
    {
        headPtr = nodePtr;//set head if first ele
    }*/






void List::delFirst()
{
    Node* nll = nullptr;

    if (headPtr == nullptr)
    {
        std::cout << "The list was already empty." << std::endl;
    }
    else if (headPtr->getNext() == nullptr || tailPtr->getPrev()== nullptr)//one node only
    {
        delete headPtr;
        headPtr = nullptr;
        tailPtr = nullptr;
    }
    else
    {
        //Node* tmp = headPtr;
        headPtr = (headPtr)->getNext();
        delete headPtr->getPrev();
        headPtr->setPrev(nll);
    }
    traverse( );

}
void List::delLast( )
{
    Node* nll = nullptr;

    if (tailPtr == nullptr)//empty
    {
        std::cout << "The list was already empty." << std::endl;
    }
    else if (headPtr->getNext() == nullptr || tailPtr->getPrev() == nullptr)//one node only?
    {
        delete tailPtr;
        headPtr = nullptr;
        tailPtr = nullptr;
    }
    else
    {   
        tailPtr = (tailPtr)->getPrev();//move back one
        delete  tailPtr->getNext();
        tailPtr->setNext(nll);

    }
    traverse();

}

void  List::reverseTrav()
{
    Node* tmp = tailPtr;
    if (tmp == nullptr)
    {
        std::cout << "The list is empty." << std::endl;

    }
    while (tmp != nullptr)
    {
        std::cout << tmp->getVal() << " " ;
        tmp = tmp->getPrev();
    }
    std::cout << std::endl;
}
void  List::traverse()
{
    Node* tmp = headPtr;
    if (tmp == nullptr)
    {
        std::cout << "The list is empty." << std::endl;

    }
    while (tmp != nullptr)
    {
        std::cout << tmp->getVal() << " " ;
        tmp = tmp->getNext();
    }

    std::cout << std::endl;
}
void menu()
{
    List list;


    Menu myMenu;

    myMenu.addItem("Enter 1 to add integer to front\nEnter 2 to add integer to back \nEnter 3 to delete from front \nEnter 4 to delete from back \nEnter 5 to traverse in reverse \n Enter 6 to quit");//itm 1
    myMenu.addItem("enter integer to add to front (limits +/-1000000)");//item 2
    myMenu.addItem("enter integer to add to back (limits +/-1000000)");//item 3;
    //myMenu.addItem("enter number:");//item 4;
    //myMenu.addItem("enter number to find triangle value from");//item 5;
    bool play = true;

    do
    {
        myMenu.displayChoice(1);
        int choice = myMenu.getIntAnswer(1, 5);
        if (choice == 1)
        {
            myMenu.displayChoice(2);
            int ans = myMenu.getIntAnswer(-1000000, 1000000);
            list.addHead(ans);
        }
        else if (choice == 2)
        {

            myMenu.displayChoice(3);
            std::cout << "upper and lower limits of numbers to be summed -100000000 and 100000000" << std::endl;
            int ans = myMenu.getIntAnswer(-1000000, 1000000);
            list.addTail(ans);
        }
        else if (choice == 3)
        {
            list.delFirst();

        }
        else if (choice == 4)
        {

            list.delLast();
        }
        else if (choice == 5)

        {
            list.reverseTrav();
        }
        else if (choice == 6)
        {
            play = false;
        }
    } while (play == true);

}


#ifndef LIST_HPP
#define LIST_HPP

#include "node.hpp"

class List
{
private:
    Node *headPtr;
    Node *tailPtr;
public:
    List();
    void addHead( int);
    void addTail( int);
    void delFirst();
    void delLast();
    void reverseTrav();
    void traverse();
    ~List();
};
void menu();

#endif
#ifndef NODE_HPP
#define NODE_HPP
#include <iostream>
#include "menu.hpp"
class Node
{
private:
    Node * next;
    int val;
    Node * prev;
public:
    Node * getPrev();
    Node * getNext();
    void setNext(Node*);
    void setPrev(Node*);
    int getVal();
    void setVal(int);
};

#endif
#include "node.hpp"
Node * Node::getPrev()
{
    return prev;
}
Node * Node::getNext()
{
    return next;
}
void Node::setNext(Node* nIn)
{
    next = nIn;
}
void Node::setPrev(Node* pIn)
{
    prev = pIn;
}

int Node::getVal()
{
    return val;
}
void Node::setVal(int vIn)
{
    val = vIn;
}

Upvotes: 1

Views: 44

Answers (1)

scohe001
scohe001

Reputation: 15446

Valgrind is trying to tell you the answer! If you ever see:

Conditional jump or move depends on uninitialized value(s)

You're almost certainly doing something wrong. In this case, Valgrind is talking about:

if (headPtr == nullptr)//empty list

But you initialize headPtr...don't you? Let's see your constructor:

List::List()
{
    Node* headPtr = nullptr;
    Node* tailPtr = nullptr;
}

Uh oh! You create two new Node* and set them to nullptr, but you never modify the headPtr and tailPtr members of your class! Remember that in C++ you can easily shadow a variable by declaring a new one with the same name in a sub-scope.

Drop the Node*'s in your constructor so you actually initialize the member variables, or even better, use a field initialization list so you don't even have to worry about this!

List::List() : headPtr(nullptr), tailPtr(nullptr) { }

Upvotes: 3

Related Questions