Cloning the Linked List results in a segmentation Fault

Question

I am trying to clone one linked list to another but I am getting segmentation fault. I have a structure with with two pointers m_Next and m_Bak and I have a char pointer which stores the name. I am trying to do it in C++. I need to clone src and return a cloned linked list.

TEMPLOYEE        * cloneList    ( TEMPLOYEE       * src ) {

    TEMPLOYEE* curr = src, *temp;

    while (curr) {
        temp = curr->m_Next;
        temp->m_Name = new char[strlen(curr->m_Name) + 1];
        strcpy ( temp->m_Name, curr->m_Name);
        curr->m_Next->m_Next = temp;
        curr = temp;
    }

    curr = src;

    while (curr) {
        curr->m_Next->m_Bak = curr->m_Bak->m_Next;

        curr = curr->m_Next?curr->m_Next->m_Next:curr->m_Next;
    }

    TEMPLOYEE* original = src, *copy = src->m_Next;

    temp = copy;

    while (original && copy)
    {
        original->m_Next =
                original->m_Next? original->m_Next->m_Next : original->m_Next;

        copy->m_Next = copy->m_Next?copy->m_Next->m_Next:copy->m_Next;
        original = original->m_Next;
        copy = copy->m_Next;
    }

    return temp;
  }

EDIT:

I got where the problem was.

TEMPLOYEE        * cloneList    ( TEMPLOYEE       * src ) {
    if(!src) return NULL;
    TEmployee * current = src;

    while(current){
        TEmployee * current_next = current->m_Next;
        current->m_Next  =  newEmployee(current->m_Name,current->m_Next);
        current->m_Next->m_Next = current_next;
        current = current_next;
    }
    current = src;

    TEmployee * clone_head  = current->m_Next;

    while(current){
        TEmployee * clone = current->m_Next;
        if(current->m_Bak){
            clone->m_Bak    = current->m_Bak->m_Next;
        }
        current = clone->m_Next;
    }

    current = src;

    while(current){
        TEmployee * clone  = current->m_Next;
        current->m_Next = clone->m_Next;
        if(clone->m_Next){
            clone->m_Next = clone->m_Next->m_Next;
        }
        current = current->m_Next;
    }
    return clone_head;

}

Note that the line `temp->m_Name = new char[strlen(curr->m_Name) + 1];` means you are not using C; that is a C++ construct. — Jonathan Leffler, Dec 24 '17 at 03:58
Some debugging ideas: 1. Get the stack trace from the application (you can run it in `gdb` until it crashes and run `bt` to [see the stack](https://www.cs.cmu.edu/~gilpin/tutorial/#3.3)) 2. Run the code via [valgrind](http://valgrind.org/docs/manual/quick-start.html) to see where the invalid access may occur 3. [Step through](https://www.cs.cmu.edu/~gilpin/tutorial/#3.5) the function using `gdb` check at what point you don't see the expected behaviour. — viraptor, Dec 24 '17 at 04:02
Make sure that out there somewhere [you don't violate the Rule of Three.](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) If I'm not mistaken, properly implementing the Rule of Three will eliminate the need for the clone function entirely. — user4581301, Dec 24 '17 at 04:15
If the goal here is to clone a linked list, then there's something fundamentally wrong with what it's doing here. A typical linked list cloning operation is done in a single `while` loop. I see three `while` loops here, and the purpose of each one appears to be somewhat dubious, and it seems fairly likely that, besides everything else it's leaking memory like a sieve. Especially as a result of the first loop. You need to take a step back, and [schedule an emergency appointment with your rubber duck](https://en.wikipedia.org/wiki/Rubber_duck_debugging) in order to get to the bottom of this. — Sam Varshavchik, Dec 24 '17 at 04:30
Yet another attempt of a linked list by a beginner programmer. Has to be in the thousands racked up on Stack Overflow at this point -- seems like every tenth question is about linked lists. The easy but slow way to do this is to have your destination list start out empty, and for each node in the source list, call "destinationList.addNode(sourceList.data)" or whatever the function is on the destination list. That is of course assuming you have such functions. — PaulMcKenzie, Dec 24 '17 at 05:17

score 0 · Answer 1 · answered Dec 24 '17 at 04:29

One problem that stands out to me is that when you're cloning your list, you clone the data but not the nodes to store the data in. This means you're overwriting your existing list, and you end up with a node that has itself as a next reference (a circular link).

So when you clone your list, you need to allocate new TEMPLOYEE nodes (into temp) in addition to copying the data over. Don't forget to remember the first one you allocate (the head of the new, cloned list) to return to the caller.

Cloning the Linked List results in a segmentation Fault

1 Answers1