I might have some aspects of this wrong, this is really the first time I have dealt much with shared pointers in particular.
I am working on traversing a tree. My tree consists of a linked list, with a vector of shared pointers representing all children for each node. To traverse, I am (to begin with) trying to do this:
//--------------------------------------------------------------
void setupMesh(){
Mesh mesh;
shared_ptr<Mesh> shared_mesh(&mesh);
meshes.push_back(shared_mesh);
checkChildren(root, &temp_mesh);
}
//--------------------------------------------------------------
void checkChildren(Node * temp_node, Mesh * temp_mesh){
if(!temp_node->children.empty()){
for(int i = 0; i < temp_node->children.size(); i++){
if(i > 0){
shared_ptr<Mesh> new_mesh(new Mesh);
meshes.push_back(new_mesh);
}
temp_node = temp_node->children[0].get();
checkChildren(temp_child, temp_mesh);
}
}
}
My tree structure itself seems fine, but it's more an issue with how I'm traversing it, and how I'm keeping track of pointers. It is currently returning bad access errors. From what I can tell, it looks like I am inserting a pointer to a temporary object, temp_node, and temp_mesh.
To simplify the process I had in mind:
Loop through all children that belong to node[0] (root).
For each of the children, perform this same loop on them. If the child is child[0], continue adding it's coordinates to the same temp_mesh object, but if it is another child, create a new mesh to store it, and all first children of it.
Any new meshes should have a pointer pushed back into the meshes vector (vector>).
Does anyone have advice on how I could do this more efficiently, or where I'm going wrong with handling these pointers in memory.
It is currently returning bad access errors.
Then that's what you should worry about as your first priority. That's a serious bug.
From what I can tell, it looks like I am inserting a pointer to a temporary object, temp_node, and temp_mesh.
This aren't "temporary" objects, that means something different (why do you keep using "temp" in your variable names?), but you're right about the problem:
shared_ptr<ofMesh> shared_mesh(&temp_mesh);
This creates a shared_ptr
which owns the pointer &temp_mesh
and so will delete it when there are no more shared_ptr
objects that share ownership of that pointer.
But that pointer is the address of an automatic variable (aka stack variable) which goes out of scope at the end of the block. You don't "own" that object, the block scope of that function manages it automatically. If you don't own it then you can't give ownership of it to the shared_ptr
, because it's not yours to give away.
When the scope ends the automatic variable temp_mesh
will be destroyed automatically, but there are still shared_ptr
objects that hold that pointer, thinking they own it. When you try to access the object through those shared_ptr
objects you access a destroyed object, after its lifetime has ended. Then when there are no more shared_ptr
objects that own the pointer it will be deleted, but it wasn't created with new
so that's a serious bug. (You get this right in the other function, so I'm not sure why you've done it wrong in setupMesh
).
If you want a shared_ptr
to own an object you need to create it with new
, or preferably create it with std::make_shared
: *
shared_ptr<ofMesh> mesh = std::make_shared<ofMesh>();
mesh0->setMode(OF_PRIMITIVE_LINE_STRIP);
mesh->setupIndicesAuto();
mesh->addVertex(root->location);
mesh->addColor(ofColor(0));
meshes.push_back(shared_mesh);
checkChildren(root, mesh.get());
This creates an object that is owned by a shared_ptr
right away, so there's no problem of transferring ownership of something that can't be owned by a shared_ptr
.
* Or you can use a "null deleter" but that's way too advanced for this answer, and won't make it OK to use automatic variables like this.