Question

So, when I try to set the data of newPtr equal to the data of origPtr, I receive a seg fault within the first lopp of the for-control-structure. I'm not sure what is causing it. Is this maybe a misuse of my pointers? Or a typo? Or perhaps a range error in my for loop's idiom?

List::List(const List &aList): numNodes(aList.numNodes) {
    empty = true;
    forward = true;
    string flag;

    cout << "Copy from head to tail? (y/n): ";
    cin >> flag;
    if(flag=="n")
        forward = false;

    if(!aList.head) {
        head = NULL; //aList is empty. so is this->List.
        tail = NULL;
    } else { // copy 1st Node.
        head = new Node;
        if(forward)
            head->setData(aList.head->getData());
        else // copy in reverse.
            head->setData(aList.tail->getData());
        //copy rest of List.
        Node *newPtr = head; //newPtr points to last Node in new List.

        //origPtr points to nodes in original List.
        if(forward) {
            cout << "Copying normally...\n" << endl;
            for(Node *origPtr=aList.head->getNext(); origPtr!=NULL;
                origPtr=origPtr->getNext()) {
                newPtr = newPtr->getNext();
                newPtr->setData(origPtr->getData()); //SEG FAULT
            } // end for
            cout << "3" << endl;
        } else {
            cout << "Copying in reverse order...\n" << endl;
            for(Node *origPtr=aList.tail->getPrev(); origPtr!=NULL;
                origPtr=origPtr->getPrev()) {
                newPtr = newPtr->getNext();
                newPtr->setData(origPtr->getData()); //SEG FAULT
            } // end for
        } // end if/else
        newPtr->setNext(NULL);
    } // end if/else
    cout << "Done copying!\n" << endl;
} // end copy constructor

If more code is necessary, I will make the necessary edits.

Also, I am aware that the standard for c++11 is to use nullptr. I will not be using it for this implementation. I'm running on Ubuntu 12.04 with gcc-v4.6.3, which does not support c++11.

EDIT: Thanks guys! So I added 3 lines to my for loops before newPtr points to getNext(). I declared newNode as a Node pointer pointing to NULL on the next line after I define newPtr as pointing to head.

            newNode = new Node;
            newNode->setPrev(newPtr);
            newPtr->setNext(newNode);
Was it helpful?

Solution

Let us analyze the following code.

  head = new Node;
    if(forward)
        head->setData(aList.head->getData());
    else // copy in reverse.
        head->setData(aList.tail->getData());
    //copy rest of List.
    Node *newPtr = head; //newPtr points to last Node in new List.

Here, you creating an object of Node and assigning it to head. Then you are setting the last or first data of list in head pointer. Then assing newPtr to head. So till now newPtr is pointing to just one element of list.

Next let us analyze following code.

} else {
    cout << "Copying in reverse order...\n" << endl;
    for(Node *origPtr=aList.tail->getPrev(); origPtr!=NULL;
        origPtr=origPtr->getPrev()) {
        newPtr = newPtr->getNext();
        newPtr->setData(origPtr->getData()); //SEG FAULT
    } // end for

Here you are doing

newPtr = newPtr->getNext()

Now, since newPtr is just pointing to one element. After this operation, newPtr becomes null. Now, with newPtr being null, following is bound to crash

newPtr->setData(origPtr->getData()); //SEG FAULT

You must allocate the memory for next element before doing newPtr->getNext().

OTHER TIPS

You do this

head = new Node;
//...
Node *newPtr = head; //newPtr points to last Node in new List.

then

newPtr = newPtr->getNext();
newPtr->setData(origPtr->getData()); //SEG FAULT

but don't set the next pointer on the node, unless there is magic going on in the Node code we can't see. I suspect the getNext gives you back a NULL Node which causes the seg fault. It makes more sense to set the data first, then point at the next node, then move to it.

   for(Node *origPtr=aList.head->getNext(); origPtr!=NULL;
                origPtr=origPtr->getNext()) {
     newPtr = newPtr->getNext();
     newPtr->setData(origPtr->getData()); //SEG FAULT
   } // end for

The problem that I see is that you are not allocating new memory for the new list that you are creating. You have only created new memory for the head node.

Something like:

for(Node *origPtr=aList.head->getNext(); origPtr!=NULL;
                origPtr=origPtr->getNext()) {
     Node *temp = new Node;
     newPtr->next = temp; // assuming you have next as a member of the class Node.
     newPtr = newPtr->getNext();
     newPtr->setData(origPtr->getData());
   } // end for
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top