CODEWITHSUNDEEP
c++pointersstruct
Theodor Peifer
Theodor Peifer

Reputation: 3496

C++: values of pointers are wrong after return

Edit changed code to be a minimal reproducible example

So Im basically having a struct and a function that creates instances of this struct, pushes them into a std::vector<Node> and then returns it.

#include <iostream>
#include <vector>


struct Node {
    int value;
    Node *left;
    Node *right;
};

Node createNodeFamily(Node child1, Node child2) {

    Node parent;
    parent.value = child1.value + child2.value;

    parent.left = &child1;
    parent.right = &child2;

    return parent;
}

std::vector<Node> f(std::vector<Node>& nodes) {
    std::vector<Node> l;
    Node parent = createNodeFamily(nodes[0], nodes[1]);
    l.push_back(parent);

    Node r = l[0];
    std::cout << "correct value of left child node: " << r.left->value << std::endl;
    std::cout << "correct value of right child node: " << r.right->value << std::endl;

    return l;
}


int main() {

    Node child1;
    child1.value = 2;

    Node child2;
    child2.value = 1;

    std::vector<Node> children;
    children.push_back(child1);
    children.push_back(child2);

    std::vector<Node> p = f(children);

    Node parent = p[0];
    //std::cout << parent.value << std::endl;
    std::cout << "wrong value of left child node: " << parent.left->value << std::endl;
    std::cout << "wrong value of right child node: " << parent.right->value << std::endl;


    return 0;
}

The output in my case:

Printed inside the function:
>> correct value of left child node: 2
>> correct value of right child node: 1
Printed outside the function:
>> wrong value of left child node: 1875944288
>> wrong value of right child node: 16717313

So the struct has the two attributes left and right which are pointers that point to a address of a child Node. When Im reading the values of those pointera printing them inside the testFunction they are correct. But when I return them, and print them they are holding seemingly random numbers.

Upvotes: 0

Views: 232

Answers (2)

mattlangford
mattlangford

Reputation: 1260

Storing pointers to stack variables is generally a recipe for disaster as you've found. When you use new the pointers aren't to stack variables anymore (they're being stored on the heap), but still are a bit sketchy since you'll need to make sure to delete the pointers when you're done, and there will likely be performance implications since your nodes will be scattered around in memory.

Normally when dealing with vectors with elements that reference other elements, I'll try to store index values rather than pointers. Consider the following:

struct Node {
    int value;
    size_t left_index;
    size_t right_index;
};

Node createNodeFamily(size_t child1_index, size_t child2_index, std::vector<Node>& nodes) {
    Node parent;

    parent.value = nodes[child1_index].value + nodes[child2_index].value;

    parent.left_index = child1_index;
    parent.right_index = child2_index;

    return parent;
}

then anytime you need to access a node you'd use nodes[index] instead of just *node.

Upvotes: 1

Theodor Peifer
Theodor Peifer

Reputation: 3496

A solution is to not create the pointers using & but by creating Node using new (changes made inside the createNodeFamily function):

Node createNodeFamily(Node child1, Node child2) {

    Node parent;
    parent.value = child1.value + child2.value;

    parent.left = new Node;
    parent.left->value = child1.value;
    parent.right->value = child2.value;

    return parent;
}

Upvotes: 0

Related Questions