Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential lifetime issue with net(s) #13

Open
sadaszewski opened this issue Mar 8, 2023 · 2 comments
Open

Potential lifetime issue with net(s) #13

sadaszewski opened this issue Mar 8, 2023 · 2 comments

Comments

@sadaszewski
Copy link

sadaszewski commented Mar 8, 2023

net holds a pointer to pcb

In c_pcb.cpp best_pcb is copy-constructed from current_pcb. Should current_pcb go out of scope, best_pcb would become compromised and result in segfaults when calling methods which would require the net-s it contains to refer to the pcb pointers inside them, as the pcb pointers are always pointing to current_pcb. While this works the way it is used, this architecture is brittle and unintuitive. I would suggest implementing a copy-constructor for pcb which would correctly update nets to contain pointers to the new instance. Hope this is intelligible.

@sadaszewski
Copy link
Author

E.g. this should do the trick:

header:

class pcb;

class net {
...
friend class pcb;
};

class pcb {
...
pcb(const pcb&);
...
};

code:

pcb::pcb(const pcb &other):
	m_resolution(other.m_resolution),
	m_quantization(other.m_quantization),
	m_depth(other.m_depth),
	m_viascost(other.m_viascost),
	m_deform(other.m_deform),
	m_layers(other.m_layers),
	m_via_layers(other.m_via_layers),
	m_routing_flood_vectors(other.m_routing_flood_vectors),
	m_routing_path_vectors(other.m_routing_path_vectors),
    
    m_width(other.m_width),
	m_height(other.m_height),
	m_stride(other.m_stride),
	m_verbosity(other.m_verbosity),
	m_netlist(other.m_netlist),
	m_nodes(other.m_nodes)
{
    for (auto &net : m_netlist) {
        net.m_pcb = this;
    }
}

@vygr
Copy link
Owner

vygr commented Mar 9, 2023

Thanks for the heads up !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants