C++ Setting next node pointer in Linked List

Question

So I'm recreating a LinkedList in C++ and I am trying to change the pointer to the last Node in my list. Here is my overloaded += operator where all the magic and errors happen. I have two different ways of changing the pointer, but both throw Unhandled exception at 0x00ee42f3 in Lab3.exe: 0xC0000005: Access violation writing location 0xccccccec.. What is going on and how can I fix it?

void MyLinkedList::operator+=(const std::string& s)
{
    allocator<Node> a;
    Node* n = a.allocate(1);
    a.construct(n, s);


    if (first == NULL)
        first = n;
    else
    {
        (*last).next = n;    // crashes here
        (*last).SetNext(n);  //also will crash here, if the previous statement is removed
        last = n;
    }
}

To further clarify, it will go through and set the first Node, exit the method, and the next time it is called will run and enter the else statement. At this point there is a second Node, it is allocated in memory and instantiated. What I am trying to do is set the Node* next pointer in the first Node to this new Node, but it throws the exception. Sorry for being very vague initially.

We have no idea what any of those values are, whether they're valid, what state your program is in when it gets to this stage, what SetNext() does, etc. You also haven't told us exactly which line causes the issue. Have you used the debugger? — PaulMcKenzie, Feb 20 '14 at 03:03
Are you sure a.allocate returns a valid pointer ? Also if last is NULL at the beginning, you might have found one of your crash. — Eric Fortin, Feb 20 '14 at 03:04
Using a pointer from Uninitialized stack memory. http://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations — drescherjm, Feb 20 '14 at 03:10
Yes, operator += should be returning a reference to the current object (*this*), not void. Just that alone makes the rest of the code that is not shown more than likely faulty. — PaulMcKenzie, Feb 20 '14 at 03:13
@PaulMcKenzie That makes a ton of sense, this is the first time I've overloaded an operator. The same error still comes up even with that fixed, but thank you for pointing out my error. — user3280133, Feb 20 '14 at 03:19

score 2 · Accepted Answer · answered Feb 20 '14 at 03:15

2

We do not know the allocate and SetNext specific realization.

If they are no problem, please look here:

if (first == NULL) 
{
    first = n;
    last = first; // You should assign last with the head node pointer.
}
...

Maybe it help.

answered Feb 20 '14 at 03:15

Charlie

418
3
9

That fixed it! Thank you so much! Edit: I do know why it fixed it. I feel so dumb, I never set `last` to anything... stupid mistakes... – user3280133 Feb 20 '14 at 03:22
You need to know why, and not rely on "magic". Go back and understand what you did wrong. If the list is empty, you did not initialize the "last" pointer. But then later, you use the "last" pointer which is pointing to who-knows-where, and attempting to write to this address. – PaulMcKenzie Feb 20 '14 at 03:25

score 1 · Answer 2 · answered Feb 20 '14 at 03:22

Your operator += has a lot of issues.

1) The operator+= should return a reference to this, not void. Otherwise a+=b makes no sense.

MyLinkedList& MyLinkedList::operator+=(const std::string& s) 
{
    //...
    return *this;
}

2) Second, your last pointer may not have been initialized if the list is empty.

3) Style issue -- why are you doing this:

(*last).next = n;

when you should just do this:

last->next = n;

?

C++ Setting next node pointer in Linked List

2 Answers2