search

LinasBeres / closest-point

0 stars 0 forks source link

Memory Leaks #1

Open bradleyhenke opened 2 years ago

bradleyhenke commented 2 years ago

Hi Linas,

I took a quick glance through your code and it looks like you're leaking memory in quite a few places. In modern C++ one of the best ways to avoid these types of issues is by using smart pointers (like std::unique_ptr) in place of raw pointers. As a less safe alternative, you could also be diligent about setting your pointers to nullptr after you have freed memory, and then you would check whether or not the pointer is freed in your destructor. If it has not been freed, then you would delete resources there. For some more details try googling "Resource Acquisition Is Initialization" (RAII).

Happy coding!

LinasBeres commented 2 years ago

Ah yes good catch!

This is fairly old code that I haven't gotten the chance to upgrade.

If I was to redo it I would definitely try to use as much smart pointers as I can or alternatively try to pass everything by reference and completely avoid the pointer issue.

bradleyhenke commented 2 years ago

No worries. I suspected that this example was a bit dated.

As for returning by reference, I don't think you would be able to keep the same semantics that you have at the moment. You are lazy initializing your kd-tree (and other structures) from a method. If you were to try to update those methods to return by reference, you would probably run into the issue of the referenced object going out of scope and being destroyed. Any returned reference would be left dangling.

Anyways, food for thought should you decided to update this exercise in the future.