std string gets corrupted inside class

Question

I am experimenting with building a simple nodegraph library for some projects i have in mind, but i'm hitting what i hope is a really simple roadblock very early on but it has me stumped.

I define objects called NodeDefinitions which are recipes from which nodes can be created in pre-defined configuraitons (number of ports/parameters etc.). NodeDefinition object contains PortDefinition objects that define each of the input/output ports. These PortDefinition objects contain their name (along with some other information in my full code, although removed below for brevity).

My Node class has a Node() constructor that creates a Node given a NodeDefition object. When I use this I create Port objects that each contain a pointer to their corresponding PortDefinition. When I try and print out the name of the port (derived from/stored in the PortDefinition object) it gets corrupted.

Through a little trial and error i have managed to find that if i just pass a std::vector directly to an alternate Node() constructor, then everything appears to work fine.

In the example code below i'm printing out the port names (only one port here), both inside the constructor and then after in the caller.

The definition classes.

class PortDefinition
{
public:
    PortDefinition(const std::string & name) : m_name(name)
    {}
    std::string m_name;
};

class NodeDefinition
{
public:    
    NodeDefinition(std::vector<PortDefinition> portDefinitions) :
        m_portDefinitions(portDefinitions) 
    {}
    std::vector<PortDefinition> m_portDefinitions;
};

The concrete object classes.

class Port
{
public:
    Port(PortDefinition * portDefinition) :
        m_portDefinition(portDefinition)
    {}
    const PortDefinition * m_portDefinition;
};

class Node
{
public:
    Node(NodeDefinition nodeDefinition) {
        std::vector<PortDefinition> portDefs = nodeDefinition.m_portDefinitions;
        for (auto & it : portDefs) {
            Port newPort = Port( &it );
            m_ports.push_back( newPort );
        }
        print();
    }

    Node(std::vector<PortDefinition> portDefs) {
        for (auto & it : portDefs) {
            Port newPort = Port( &it );
            m_ports.push_back( newPort );
        }
        print();
    }

    void print() const {
        std::cout << m_ports.size() << " : ";
        for (auto it : m_ports) {
            std::cout << "'" << it.m_portDefinition->m_name << "'" << std::endl; 
        }
    }
private:
    std::vector<Port> m_ports;
};

The test code.

int main (int argc, const char *argv[])
{    
    std::vector<PortDefinition> portDefinitions;
    portDefinitions.push_back( PortDefinition("Port_A") );
    NodeDefinition nodeDefinition = NodeDefinition(portDefinitions);

    std::cout << "constuctor N1 : ";
    Node N1 = Node(nodeDefinition);
    std::cout << "main func N1  : ";
    N1.print();

    std::cout << std::endl;

    std::cout << "constuctor N2 : ";
    Node N2 = Node(portDefinitions);
    std::cout << "main func N2  : ";
    N2.print();
    return 1;
}

All of the code can be compiled in a single file together.

When I run this get the following output.

constuctor N1 : 1 : 'Port_A'
main func N1  : 1 : ''

constuctor N2 : 1 : 'Port_A'
main func N2  : 1 : 'Port_A'

As you can see when i print out the port name after using the Node() constructor that uses the NodeDefinition object the name is empty, sometimes I get garbage there instead, which makes me think something is corrupting memory somehow, but i'm a bit lost as to why.

Answer 1

std::vector<PortDefinition> portDefs = nodeDefinition.m_portDefinitions;
for (auto & it : portDefs) {
    Port newPort = Port( &it );
    m_ports.push_back( newPort );
}

This code is the problem. portDefs is a copy of nodeDefinition.m_portDefinitions, being destroyed when the constructor is finished. But you store a pointer to the these objects with Port(&it).

The print() in the constructor should work fine, but the print() in main now accesses the destroyed copies, which is undefined behaviour.

A possible solution would be to store shared_ptr of your PortDefinition or just store a copy in Port.