Open kunaltyagi opened 4 years ago
awk -F':| ' '{print "vim "$1" +"$2}'
That's not a Ctrl+click
@taketwo , that's a 'pipe' dream
@kunaltyagi Which profiling tool did you use to check for memory leaks?
IIRC That's cppcheck
I came leak across one in OctreeBase assignment operator https://github.com/PointCloudLibrary/pcl/blob/master/octree/include/pcl/octree/octree_base.h#L250
OctreeBase&
operator=(const OctreeBase& source)
{
leaf_count_ = source.leaf_count_;
branch_count_ = source.branch_count_;
// root_node_ has already data but it is never deleted
root_node_ = new (BranchNode)(*(source.root_node_));
depth_mask_ = source.depth_mask_;
max_key_ = source.max_key_;
octree_depth_ = source.octree_depth_;
return (*this);
}
one solution would be to delete the rootnode before we assign new:
OctreeBase&
operator=(const OctreeBase& source)
{
leaf_count_ = source.leaf_count_;
branch_count_ = source.branch_count_;
if(root_node_)
{
delete root_node_;
}
root_node_ = new (BranchNode)(*(source.root_node_));
depth_mask_ = source.depth_mask_;
max_key_ = source.max_key_;
octree_depth_ = source.octree_depth_;
return (*this);
}
Basically any time class own a resource (memory allocated with new operator) the 0/3/5 rule applies. It is lot of work to properly implements all the infrastructure to manage resource without introducing memory leaks, especially if "5" applies. I tend to avoid resources unless performance requirements warrant it. Since the OctreeBase owns the rootnode maybe it can be changed to instance variable instead of pointer. If you do not know what I am talking about here is ref: https://en.cppreference.com/w/cpp/language/rule_of_three
I run my unit tests under valgrind when I found the leak.
Thanks for finding the leak. Regarding the rules of C++, a lot of the software was written in a pre-peer-review era.
I think the problem still exists. The operator= should do the same cleanup as ~OctreeBase i.e. deleteTree()
OctreeBase& operator=(const OctreeBase& source)
{
deleteTree();//otherwise, a memory leak persists
leaf_count_ = source.leaf_count_;
branch_count_ = source.branch_count_;
delete root_node_;
root_node_ = new (BranchNode)(*(source.root_node_));
depth_mask_ = source.depth_mask_;
max_key_ = source.max_key_;
octree_depth_ = source.octree_depth_;
return (*this);
}
Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.
Your Environment
Context
Most of the leaks are in application/test code, which will not impact the core library. But they should be removed nonetheless.
Some of the leaks have been fixed, but there are still some left.
List of files with leaks
The format allows most editors to open the file using
Ctrl+click
(no, not vim :stuck_out_tongue: )