3

Here is my code

Node* Node::createNode(int n)
{
    Node newNode(n);
    Node *temp = &newNode;
    return temp;
}

void Node::printNode(Node* node)
{
    cout << node->data << "\t" << node->next << endl;
    system("pause");
}

Here is the main function

int main(int argc, char* argv[])
{

    Node *head = Node::createNode(10);
    Node::printNode(head);
    return 0;
}

Problem, I was expecting to print 10. Instead I get a garbage value of -858993460. I have noticed that until the point, parameter is passed into the function, it retains correct value. But inside the function printNode, the value changes. So I guess something is going wrong in the function call. Any help would be appreciated.

R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
Sumod
  • 3,806
  • 9
  • 50
  • 69
  • Also, you don't need to code printNode that way. Use `void Node::printNode(){ cout << this->data << endl; }` or `void printNode(Node *node){ cout << node->data << endl; }` – Zhen Jul 06 '11 at 11:24

5 Answers5

8

You're returning a pointer to a local variable. When you later use that pointer in the main function, the variable it pointed to is long gone and you enter the realm of undefined behavior.

You could just return the node by value:

Node Node::createNode(int n)
{
    Node newNode(n);
    return newNode;
}

Or you could allocate one dynamically and return a smart pointer to it, enabling scope bound resource management (aka RAII):

std::unique_ptr<Node> Node::createNode(int n)
{
    return std::unique_ptr<Node>(new Node(n));
}
Community
  • 1
  • 1
R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
2

You are returning a pointer to a locally scoped Node in createNode. This is a bad thing(tm). When that function goes out of scope (i.e. returns), who knows what happens to what you are pointing to.

instead use one of the following:

  1. copy construct (and return by value)
  2. allocate on heap (new Node) and return (pref. using smart pointers)
Nim
  • 33,299
  • 2
  • 62
  • 101
0

You're mistaken in the way you create the node:

Node* Node::createNode(int n)
{
    Node newNode(n);       // create a node on stack
    Node *temp = &newNode; // get address of the node on stack
    return temp;           // return that address
}                          // and here the node is destructed!

You should have allocated it in the heap via operator new().

See also Stack, Static, and Heap in C++ and http://www.learncpp.com/cpp-tutorial/79-the-stack-and-the-heap/ for explainations.

Community
  • 1
  • 1
vines
  • 5,160
  • 1
  • 27
  • 49
0

You are returning a reference to a local

Node newNode(n);
Node *temp = &newNode;
return temp;

newNode goes out of scope and is overwritten by later stack contents.

sehe
  • 374,641
  • 47
  • 450
  • 633
0

In Node::createNode() function, you are returning the address of a local variable. This is a major gaffe. The life of the Node object goes out of scope once you return from the function, it gets ejected from the stack.

Sharath
  • 1,627
  • 2
  • 18
  • 34