SenseiHitokiri SenseiHitokiri - 17 days ago 5
C Question

C++ Pointer target returning wrong value

I am a fairly experience C# programmer and trying to help out a friend with a C++ application that creates a Stack object. It has been well over 13 years since I've even seen C++ and I am having a damn fine time trying to recall the proper way to do this. It took me a bit to get up to speed on the Header/CPP distinction again, so there may be issues in there even. Here is my problem:

//Stack.h

#ifndef __STACK_INCLUDED__
#define __STACK_INCLUDED__
#include "Node.h"

class Stack
{
private:
/// Going to be the pointer to our top node
Node* m_topNode;

/// Running count of elements
int m_count;
public:
///Constructor
Stack();

///Allows us to retrieve the top value from the stack
/// and remove it from the stack
int Pop();
.
.
.
};
#endif


Below is the CPP that matches the header. I am doing in here JUST for debugging at the moment. I am also fully qualifying everything because I was not sure if that is causing issues with the pointers and loss of references.

//Stack.cpp
#include "stdafx.h"
#include "Stack.h"
#include <iostream>
Stack::Stack(){
m_count = 0;
m_topNode = NULL;
}

void Stack::Push(int Value){

std::cout << "\nPushing Value: ";
std::cout << Value;
std::cout << "\n";
if ( Stack::m_topNode )
{
std::cout << "TopNode Value: ";
std::cout << Stack::m_topNode->data;
std::cout << "\n";
}
std::cout << "\n";

Node newNode(Value, NULL, Stack::m_topNode);
Stack::m_topNode = &newNode;
Stack::m_count++;
}


The node class is a pretty simple entity. Just needs to store a value and the pointers on either side. I know I don't need to track in both directions for a Stack but I wanted to make this something that was easily changed to a Queue or similar construct.

//Node.h

#ifndef __NODE_INCLUDED__
#define __NODE_INCLUDED__
class Node
{
private:

public:
///Constructor allows us to specify all values.
/// In a stack I expect NextNode to be NULL
Node(int Value,Node* NextNode, Node* PreviousNode);

///Pointer to the next node
Node* Next;

///Pointer to the previous node
Node* Prev;

///Value to be stored
int data;
};
#endif


Very simple implementation:
//Node.cpp
#include "stdafx.h"
#include "Node.h"

Node::Node(int Value, Node* NextNode, Node* PreviousNode){
data = Value;
Next = NextNode;
Prev = PreviousNode;
}


My main is just about sending 2 values to the stack right now via Push and seeing what the values are printing:

#include "stdafx.h"
#include "Node.h"
#include "Stack.h"
using namespace std;

int main(){
Stack s = Stack();

for ( int i = 0; i < 2; i++ ){
s.Push(i * 10);
}

int blah;
cin >> blah; //Stall screen
return 0;
}


Here is the Output:

Pushing Value: 0

<blank line>
Pushing Value: 10
TopNode Value: -858993460


When I hit Node newNode(Value, NULL, Stack::m_topNode) in the debugger I can see it tracking the proper value in the current node, but m_topNode references a really odd value. I'm hoping it's very obvious that I'm doing something dumb as I don't remember this being this tricky when I did it years ago. Appreciate any help/insight to my incorrect manners.

Answer
Node newNode(Value, NULL, Stack::m_topNode);
Stack::m_topNode = &newNode;
Stack::m_count++;

This is your problem. You allocate the new node on the current stack, and then put the pointer into the linked list of nodes. This pointer will be invalid as soon as your stack frame returns, and all hell breaks lose. ;)

You need to allocate the node with new.

Comments